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:
[email protected]
