[GitHub] [accumulo] milleruntime commented on a change in pull request #2169: Cancel compactions when delete table is called

[GitBox <gi...@apache.org>]

milleruntime commented on a change in pull request #2169:
URL: https://github.com/apache/accumulo/pull/2169#discussion_r657032042



##########
File path: server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/cancel/CancelCompactions.java
##########
@@ -54,6 +54,19 @@ public long isReady(long tid, Manager env) throws Exception {
 
   @Override
   public Repo<Manager> call(long tid, Manager environment) throws Exception {
+
+    mutateZooKeeper(tid, tableId, environment);
+    return new FinishCancelCompaction(namespaceId, tableId);
+  }
+
+  @Override
+  public void undo(long tid, Manager env) {
+    Utils.unreserveTable(env, tableId, tid, false);
+    Utils.unreserveNamespace(env, namespaceId, tid, false);
+  }

Review comment:
       I noticed that the `undo()` of CancelCompaction doesn't actually undo the mutate of ZooKeeper. From my understanding, the `undo()` of a FATE REPO is called if the operation fails. This may not be a problem for this REPO but I think there other operations that fail to actually undo anything from the operation and just call these 2 unreserve methods. I think this is something that we should probably look into.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org