ctubbsii commented on a change in pull request #2178:
URL: https://github.com/apache/accumulo/pull/2178#discussion_r657327267
##########
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:
I'm not sure how it's possible for this to run after a table is deleted,
unless the lock reservations are relying on ZooCache to check table existence
and it's somehow out-of-date. See a related comment about compaction IDs in
https://github.com/apache/accumulo/pull/2175#discussion_r657292064
In any case, elsewhere in that same class, we `throw new
AcceptableThriftTableOperationException`, in response to `NoNodeException`.
Should we do that here as well? I'm not sure who is even catching and handling
that exception type. But, it's probably worth tracking down to see if they
should be made consistent, or at least adding a comment what's different here
that warrants a different reaction to the same exception.
--
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]
