[GitHub] [accumulo] milleruntime commented on a change in pull request #1863: Add check for finished to LogSorter

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

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



##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java
##########
@@ -110,7 +109,13 @@ public void sort(String name, Path srcPath, String destPath) {
       String formerThreadName = Thread.currentThread().getName();
       int part = 0;
       try {
+        // check for finished first since another thread may have already done the sort
+        if (fs.exists(SortedLogState.getFinishedMarkerPath(destPath))) {

Review comment:
       Mainly for the try/catch/finally since the `exists()` call has a checked IOException.  I wanted to add it earlier, in the process method before it gets added to the `currentWork` map but there isn't a try/catch there.  Then the more I thought about it, its probably not a bad thing it gets added to the `currentWork` map for this check since the `exists()` method is a Namenode operation.




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