cshannon commented on code in PR #5399:
URL: https://github.com/apache/accumulo/pull/5399#discussion_r1992184773


##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java:
##########
@@ -266,37 +271,62 @@ private long 
removeTabletServerMarkers(Map<UUID,TServerInstance> uidMap,
     return result;
   }
 
-  private long removeFile(Path path) {
-    try {
-      if (!useTrash || !fs.moveToTrash(path)) {
-        fs.deleteRecursively(path);
+  private void removeFile(ExecutorService deleteThreadPool, Path path, 
AtomicLong counter,
+      String msg) {
+    deleteThreadPool.execute(() -> {
+      try {
+        log.debug(msg);
+        if (!useTrash || !fs.moveToTrash(path)) {
+          fs.deleteRecursively(path);
+        }
+        counter.incrementAndGet();
+      } catch (FileNotFoundException ex) {

Review Comment:
   If you used futures that might be better, you could wait for them to finish 
instead of relying on the thread pool to shutdown for completion. Either way 
could work, I just figured I'd mention it as I'm not too familiar with this 
code but I noticed we were only catching those specific exceptions so was 
curious if a random runtime exception would cause issues with the new thread 
pool.



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

To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to