EdColeman commented on a change in pull request #1890:
URL: https://github.com/apache/accumulo/pull/1890#discussion_r566926865
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/zookeeper/TransactionWatcher.java
##########
@@ -104,13 +104,15 @@ public static void cleanup(ServerContext context, String
type, long tid)
final ZooReader reader = context.getZooReaderWriter();
final Set<Long> result = new HashSet<>();
final String parent = context.getZooKeeperRoot() + "/" + type;
- reader.sync(parent);
- List<String> children = reader.getChildren(parent);
- for (String child : children) {
- if (child.endsWith("-running")) {
- continue;
+ if (reader.exists(parent)) {
Review comment:
I was looking at that too - I was thinking it might be okay - the
comment in the exception handler says that it will just retry, so it could be
the same thing as processing nothing with an empty list. There does not seem to
be any difference - it looks like it builds a new list each time run is called
and nothing is signalling completion.
Also in that exception handling I noticed (well, the ide did) in
BulkImportCacheCleaner:[59]
`log.debug("Error reading bulk import live transactions {}", e);` has
mismatched arguments - either {} should be removed or something added.
Another thing is that exception handling is swallowing the interrupt
exception.
If you don't want to handle those as part of this PR i'll create an issue.
----------------------------------------------------------------
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]
