milleruntime commented on a change in pull request #2178:
URL: https://github.com/apache/accumulo/pull/2178#discussion_r657382125
##########
File path:
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactRange.java
##########
@@ -149,24 +149,27 @@ static void removeIterators(Manager environment, final
long txid, TableId tableI
ZooReaderWriter zoo = environment.getContext().getZooReaderWriter();
- zoo.mutateExisting(zTablePath, currentValue -> {
- String cvs = new String(currentValue, UTF_8);
- String[] tokens = cvs.split(",");
- long flushID = Long.parseLong(tokens[0]);
-
- String txidString = String.format("%016x", txid);
+ try {
+ zoo.mutateExisting(zTablePath, currentValue -> {
+ String cvs = new String(currentValue, UTF_8);
+ String[] tokens = cvs.split(",");
+ long flushID = Long.parseLong(tokens[0]);
- StringBuilder encodedIterators = new StringBuilder();
- for (int i = 1; i < tokens.length; i++) {
- if (tokens[i].startsWith(txidString))
- continue;
- encodedIterators.append(",");
- encodedIterators.append(tokens[i]);
- }
+ String txidString = String.format("%016x", txid);
- return (Long.toString(flushID) + encodedIterators).getBytes(UTF_8);
- });
+ StringBuilder encodedIterators = new StringBuilder();
+ for (int i = 1; i < tokens.length; i++) {
+ if (tokens[i].startsWith(txidString))
+ continue;
+ encodedIterators.append(",");
+ encodedIterators.append(tokens[i]);
+ }
+ return (Long.toString(flushID) + encodedIterators).getBytes(UTF_8);
+ });
+ } catch (NoNodeException ke) {
+ log.debug("Node for {} no longer exists.", tableId, ke);
+ }
Review comment:
> In any case, elsewhere in that same class, we `throw new
AcceptableThriftTableOperationException`, in response to `NoNodeException`.
Should we do that here as well?
I don't think that is necessary. The `undo()` will only be called when the
operation failed, which usually is when an Exception occurs. Throwing another
exception on a failure in the `undo()` seems excessive. I think this is where
it prints the original Exception which set the REPO to `FAILED_IN_PROGRESS`.
https://github.com/apache/accumulo/blob/16a3cec36981a694ecdfbefb839ecf9ddd70a535/core/src/main/java/org/apache/accumulo/fate/Fate.java#L162-L174
I assume in this case the exception was acceptable so wasn't printed.
--
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]
