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