Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

admin fate improvements, LockIDs use for fate stores improved/fixed #5028

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

admin fate improvements, LockIDs use for fate stores improved/fixed #5028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2edec04
`admin fate` improvements, `LockID`s use for fate stores improved/fixed
kevinrr888 Oct 31, 2024
6957092
Merge branch 'main' into 4.0-feature-4904
kevinrr888 Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/src/main/java/org/apache/accumulo/core/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ public class Constants {

public static final String ZTABLE_LOCKS = "/table_locks";
public static final String ZMINI_LOCK = "/mini";
public static final String ZADMIN_LOCK = "/admin/lock";
public static final String ZTEST_LOCK = "/test/lock";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to drop this constant and have the test code create locks under the admin path when needed?


public static final String BULK_PREFIX = "b-";
public static final String BULK_RENAME_FILE = "renames.json";
Expand Down
22 changes: 5 additions & 17 deletions core/src/main/java/org/apache/accumulo/core/fate/AbstractFateStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ public FateId fromTypeAndKey(FateInstanceType instanceType, FateKey fateKey) {
// Keeps track of the number of concurrent callers to waitForStatusChange()
private final AtomicInteger concurrentStatusChangeCallers = new AtomicInteger(0);

public AbstractFateStore() {
this(createDummyLockID(), null, DEFAULT_MAX_DEFERRED, DEFAULT_FATE_ID_GENERATOR);
}

public AbstractFateStore(ZooUtil.LockID lockID, Predicate<ZooUtil.LockID> isLockHeld) {
this(lockID, isLockHeld, DEFAULT_MAX_DEFERRED, DEFAULT_FATE_ID_GENERATOR);
}
Expand All @@ -98,9 +94,7 @@ public AbstractFateStore(ZooUtil.LockID lockID, Predicate<ZooUtil.LockID> isLock
this.maxDeferred = maxDeferred;
this.fateIdGenerator = Objects.requireNonNull(fateIdGenerator);
this.deferred = Collections.synchronizedMap(new HashMap<>());
this.lockID = Objects.requireNonNull(lockID);
// If the store is used for a Fate which runs a dead reservation cleaner,
// this should be non-null, otherwise null is fine
this.lockID = lockID;
this.isLockHeld = isLockHeld;
}

Expand Down Expand Up @@ -275,6 +269,10 @@ protected void verifyFateKey(FateId fateId, Optional<FateKey> fateKeySeen,
"Collision detected for fate id " + fateId);
}

protected void verifyLock(ZooUtil.LockID lockID, FateId fateId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to ignore this comment, it was something I noticed but not sure about what if any changes to make based on this observation.

In every place where this method is called the next line is something like FateReservation.from(lockID, UUID.randomUUID()). The call to FateReservation.from will fail if the lockId is null. So this method may be redundant, however its error message is more informative because it includes the fate id.

Preconditions.checkState(lockID != null, "Tried to reserve " + fateId + " with null lockID");
}

protected abstract Stream<FateIdStatus> getTransactions(EnumSet<TStatus> statuses);

protected abstract TStatus _getStatus(FateId fateId);
Expand Down Expand Up @@ -428,14 +426,4 @@ protected Serializable deserializeTxInfo(TxInfo txInfo, byte[] data) {
throw new IllegalStateException("Bad node data " + txInfo);
}
}

/**
* this is a temporary method used to create a dummy lock when using a FateStore outside the
* context of a Manager (one example is testing) so reservations can still be made.
*
* @return a dummy {@link ZooUtil.LockID}
*/
public static ZooUtil.LockID createDummyLockID() {
return new ZooUtil.LockID("/path", "node", 123);
}
}
Loading