keith-turner commented on a change in pull request #2372:
URL: https://github.com/apache/accumulo/pull/2372#discussion_r765974395



##########
File path: 
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
##########
@@ -712,6 +711,8 @@ public void run() {
           compactionThread.join();
           LOG.trace("Compaction thread finished.");
 
+          checkIfCanceled();

Review comment:
       The intent of doing periodic cancelation checks is to stop long running 
external compactions that are canceled.  If lots of quick external compaction 
(like 30 s) run, then a cancelation check will now be done for each of those 
where it was not before adding this.   During the commit process atomic checks 
will be done, so doing the check here for every compaction does not offer a 
benefit over that. The compaction could still be canceled after this check 
before the atomic checks are done.  So maybe checking here places additional 
load on the metadata table and ZK w/o a clear benefit.
   
   Thinking about only checking long running compactions, we could also make 
the checkIfCanceled method ignore young compactions. We currently check every 5 
mins, if  a compaction is running and its age is <5min we could return and not 
do the check until a later call when the age is > 5min (or the check period).




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