[GitHub] [accumulo] ctubbsii commented on a change in pull request #1923: Update ServerBulkImportStatus for new Bulk import. Closes #1510

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

ctubbsii commented on a change in pull request #1923:
URL: https://github.com/apache/accumulo/pull/1923#discussion_r574806504



##########
File path: server/base/src/main/java/org/apache/accumulo/server/util/ServerBulkImportStatus.java
##########
@@ -37,12 +39,22 @@
 
   public void updateBulkImportStatus(List<String> files, BulkImportState state) {
     for (String file : files) {
-      BulkImportStatus initial = new BulkImportStatus(System.currentTimeMillis(), file, state);
-      status.putIfAbsent(file, initial);
-      initial = status.get(file);
-      if (initial != null) {
-        initial.state = state;
-      }
+      updateFile(file, state);
+    }
+  }
+
+  private void updateFile(String file, BulkImportState state) {
+    BulkImportStatus initial = new BulkImportStatus(System.currentTimeMillis(), file, state);
+    status.putIfAbsent(file, initial);
+    initial = status.get(file);
+    if (initial != null) {
+      initial.state = state;
+    }
+  }

Review comment:
       This could be made to update the concurrent map atomically:
   
   ```suggestion
     private void updateFile(String file, BulkImportState state) {
       status.compute(file, (key, currentStatus) -> {
         if (currentStatus == null) {
           return new BulkImportStatus(System.currentTimeMillis(), file, state);
         }
         currentStatus.state = state;
         return currentStatus;
       });
     }
   ```

##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java
##########
@@ -162,6 +164,7 @@
   private final TabletServer server;
   private final WriteTracker writeTracker = new WriteTracker();
   private final RowLocks rowLocks = new RowLocks();
+  private final ServerBulkImportStatus bulkImportStatus = new ServerBulkImportStatus();

Review comment:
       I'm not sure I understand the point of this addition here. I see the three places where you update/remove entries being tracked in this object, but I don't see anywhere where the statuses being tracked in this ever get read and used.




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