keith-turner commented on code in PR #4628:
URL: https://github.com/apache/accumulo/pull/4628#discussion_r1624958165


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java:
##########
@@ -564,11 +566,24 @@ static CompactionStats compact(Tablet tablet, 
CompactionJob job,
     AccumuloConfiguration compactionConfig = getCompactionConfig(tableConf,
         getOverrides(job.getKind(), tablet, cInfo.localHelper, 
job.getFiles()));
 
-    FileCompactor compactor = new FileCompactor(tablet.getContext(), 
tablet.getExtent(),
+    final FileCompactor compactor = new FileCompactor(tablet.getContext(), 
tablet.getExtent(),
         compactFiles, tmpFileName, cInfo.propagateDeletes, cenv, cInfo.iters, 
compactionConfig,
         tableConf.getCryptoService());
 
-    return compactor.call();
+    final Runnable compactionCancellerTask = () -> {
+      if (!cenv.isCompactionEnabled()) {
+        compactor.interrupt();
+      }
+    };
+    final ScheduledFuture<?> future = 
tablet.getContext().getScheduledExecutor()
+        .schedule(compactionCancellerTask, 10, TimeUnit.SECONDS);

Review Comment:
   This will run just once in 10 seconds, probably want it to run every 10 
seconds.



##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableUtils.java:
##########
@@ -564,11 +566,24 @@ static CompactionStats compact(Tablet tablet, 
CompactionJob job,
     AccumuloConfiguration compactionConfig = getCompactionConfig(tableConf,
         getOverrides(job.getKind(), tablet, cInfo.localHelper, 
job.getFiles()));
 
-    FileCompactor compactor = new FileCompactor(tablet.getContext(), 
tablet.getExtent(),
+    final FileCompactor compactor = new FileCompactor(tablet.getContext(), 
tablet.getExtent(),
         compactFiles, tmpFileName, cInfo.propagateDeletes, cenv, cInfo.iters, 
compactionConfig,
         tableConf.getCryptoService());
 
-    return compactor.call();
+    final Runnable compactionCancellerTask = () -> {
+      if (!cenv.isCompactionEnabled()) {
+        compactor.interrupt();
+      }
+    };
+    final ScheduledFuture<?> future = 
tablet.getContext().getScheduledExecutor()
+        .schedule(compactionCancellerTask, 10, TimeUnit.SECONDS);
+    try {
+      return compactor.call();
+    } finally {
+      future.cancel(true);
+      
tablet.getContext().getScheduledExecutor().remove(compactionCancellerTask);
+      tablet.getContext().getScheduledExecutor().purge();

Review Comment:
   Could remove these.  The next time the task tries to run the threadpool will 
see its canceled and remove it.  If there are a lot of things on the threadpool 
queue calling these frequently could be expensive.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to