kevinrr888 commented on code in PR #5028:
URL: https://github.com/apache/accumulo/pull/5028#discussion_r1852514075


##########
core/src/main/java/org/apache/accumulo/core/fate/AdminUtil.java:
##########
@@ -504,39 +502,75 @@ public boolean 
prepFail(Map<FateInstanceType,FateStore<T>> stores, ZooReaderWrit
     // determine which store to use
     FateStore<T> store = stores.get(fateId.getType());
 
-    FateTxStore<T> txStore = store.reserve(fateId);
-    try {
-      TStatus ts = txStore.getStatus();
-      switch (ts) {
-        case UNKNOWN:
-          System.out.println("Invalid fate ID: " + fateId);
-          break;
-
-        case SUBMITTED:
-        case IN_PROGRESS:
-        case NEW:
-          System.out.printf("Failing transaction: %s (%s)%n", fateId, ts);
-          txStore.setStatus(TStatus.FAILED_IN_PROGRESS);
-          state = true;
-          break;
-
-        case SUCCESSFUL:
-          System.out.printf("Transaction already completed: %s (%s)%n", 
fateId, ts);
-          break;
-
-        case FAILED:
-        case FAILED_IN_PROGRESS:
-          System.out.printf("Transaction already failed: %s (%s)%n", fateId, 
ts);
-          state = true;
-          break;
+    Optional<FateTxStore<T>> opTxStore = tryReserve(store, fateId, "fail");
+    if (opTxStore.isPresent()) {
+      var txStore = opTxStore.orElseThrow();
+
+      try {
+        TStatus ts = txStore.getStatus();
+        switch (ts) {
+          case UNKNOWN:
+            System.out.println("Invalid fate ID: " + fateId);
+            break;
+
+          case SUBMITTED:
+          case IN_PROGRESS:
+          case NEW:
+            System.out.printf("Failing transaction: %s (%s)%n", fateId, ts);
+            txStore.setStatus(TStatus.FAILED_IN_PROGRESS);
+            state = true;
+            break;
+
+          case SUCCESSFUL:
+            System.out.printf("Transaction already completed: %s (%s)%n", 
fateId, ts);
+            break;
+
+          case FAILED:
+          case FAILED_IN_PROGRESS:
+            System.out.printf("Transaction already failed: %s (%s)%n", fateId, 
ts);
+            state = true;
+            break;
+        }
+      } finally {
+        txStore.unreserve(Duration.ZERO);
       }
-    } finally {
-      txStore.unreserve(Duration.ZERO);
     }
 
     return state;
   }
 
+  /**
+   * Try to reserve the transaction for a minute. If it could not be reserved, 
return an empty
+   * optional
+   */
+  private Optional<FateTxStore<T>> tryReserve(FateStore<T> store, FateId 
fateId, String op) {
+    var retry = Retry.builder().maxRetriesWithinDuration(Duration.ofMinutes(1))
+        .retryAfter(Duration.ofMillis(25)).incrementBy(Duration.ofMillis(25))
+        
.maxWait(Duration.ofSeconds(15)).backOffFactor(1.5).logInterval(Duration.ofSeconds(15))
+        .createRetry();
+
+    Optional<FateTxStore<T>> reserveAttempt = store.tryReserve(fateId);
+    while (reserveAttempt.isEmpty() && retry.canRetry()) {
+      retry.useRetry();
+      try {
+        retry.waitForNextAttempt(log, "Attempting to reserve " + fateId);
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        throw new IllegalArgumentException(e);
+      }
+      reserveAttempt = store.tryReserve(fateId);
+    }
+    if (reserveAttempt.isPresent()) {
+      retry.logCompletion(log, "Attempting to reserve " + fateId);
+    } else {
+      log.error("Could not {} {} in a reasonable time. This indicates the 
Manager is currently "
+          + "working on {}. If {} {} is still desired, the Manager needs to be 
stopped and "
+          + "the command needs to be rerun.", op, fateId, fateId, op, fateId);

Review Comment:
   Yeah, this was a bit much. Fixed.



##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -340,11 +346,11 @@ static class FateOpsCommand {
     boolean cancel;
 
     @Parameter(names = {"-f", "--fail"},
-        description = "<FateId>... Transition FaTE transaction status to 
FAILED_IN_PROGRESS (requires Manager to be down)")
+        description = "<FateId>... Transition FaTE transaction status to 
FAILED_IN_PROGRESS")
     boolean fail;
 
     @Parameter(names = {"-d", "--delete"},
-        description = "<FateId>... Delete locks associated with transactions 
(Requires Manager to be down)")
+        description = "<FateId>... Delete locks associated with transactions")

Review Comment:
   Good catch... I missed that. This has been fixed by #5080 in 2.1+



-- 
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]

Reply via email to