kevinrr888 commented on code in PR #4524:
URL: https://github.com/apache/accumulo/pull/4524#discussion_r1688551230
##########
core/src/main/java/org/apache/accumulo/core/fate/AbstractFateStore.java:
##########
@@ -114,38 +131,26 @@ public static Object deserialize(byte[] ser) {
}
}
- /**
- * Attempt to reserve the fate transaction.
- *
- * @param fateId The FateId
- * @return An Optional containing the FateTxStore if the transaction was
successfully reserved, or
- * an empty Optional if the transaction was already reserved.
- */
- @Override
- public Optional<FateTxStore<T>> tryReserve(FateId fateId) {
- synchronized (this) {
- if (!reserved.contains(fateId)) {
- return Optional.of(reserve(fateId));
- }
- return Optional.empty();
- }
- }
-
@Override
public FateTxStore<T> reserve(FateId fateId) {
- synchronized (AbstractFateStore.this) {
- while (reserved.contains(fateId)) {
- try {
- AbstractFateStore.this.wait(100);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new IllegalStateException(e);
- }
+ Preconditions.checkState(!_getStatus(fateId).equals(TStatus.UNKNOWN),
+ "Attempted to reserve a tx that does not exist: " + fateId);
Review Comment:
Good catch, fixed.
##########
core/src/main/java/org/apache/accumulo/core/fate/AbstractFateStore.java:
##########
@@ -361,90 +356,108 @@ public Optional<FateTxStore<T>> createAndReserve(FateKey
fateKey) {
protected abstract Optional<FateKey> getKey(FateId fateId);
- protected abstract FateTxStore<T> newFateTxStore(FateId fateId, boolean
isReserved);
-
- protected abstract FateInstanceType getInstanceType();
+ protected abstract FateTxStore<T> newUnreservedFateTxStore(FateId fateId);
protected abstract class AbstractFateTxStoreImpl<T> implements
FateTxStore<T> {
protected final FateId fateId;
- protected final boolean isReserved;
+ protected boolean deleted;
+ protected FateReservation reservation;
protected TStatus observedStatus = null;
- protected AbstractFateTxStoreImpl(FateId fateId, boolean isReserved) {
+ protected AbstractFateTxStoreImpl(FateId fateId) {
+ this.fateId = fateId;
+ this.deleted = false;
+ this.reservation = null;
+ }
+
+ protected AbstractFateTxStoreImpl(FateId fateId, FateReservation
reservation) {
this.fateId = fateId;
- this.isReserved = isReserved;
+ this.deleted = false;
+ this.reservation = Objects.requireNonNull(reservation);
+ }
+
+ protected boolean isReserved() {
+ return this.reservation != null;
}
@Override
public TStatus waitForStatusChange(EnumSet<TStatus> expected) {
- Preconditions.checkState(!isReserved,
- "Attempted to wait for status change while reserved " + fateId);
- while (true) {
+ Preconditions.checkState(!isReserved(),
+ "Attempted to wait for status change while reserved: " + fateId);
+ verifyReserved(false);
- long countBefore = unreservedNonNewCount.getCount();
+ int currNumCallers = concurrentStatusChangeCallers.incrementAndGet();
Review Comment:
Good suggestion, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
