[GitHub] keith-turner commented on a change in pull request #759: Only requeue compaction when there was activity

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

keith-turner commented on a change in pull request #759: Only requeue compaction when there was activity
URL: https://github.com/apache/accumulo/pull/759#discussion_r232081277
 
 

 ##########
 File path: server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactionRunner.java
 ##########
 @@ -41,12 +41,18 @@ public void run() {
       return;
     }
 
-    tablet.majorCompact(reason, queued);
+    CompactionStats stats = tablet.majorCompact(reason, queued);
 
-    // if there is more work to be done, queue another major compaction
-    synchronized (tablet) {
-      if (reason == MajorCompactionReason.NORMAL && tablet.needsMajorCompaction(reason))
-        tablet.initiateMajorCompaction(reason);
+    // Some compaction strategies may always return true for shouldCompact() because they need to
+    // make blocking calls to gather information. Without the following check these strategies would
+    // endlessly requeue. So only check if a subsequent compaction is needed if the previous
+    // compaction actually did something.
+    if (stats != null && stats.getEntriesRead() > 0) {
 
 Review comment:
   No, tablets are checked periodically to see if they need a compaction by the tablet server.  This code is just an optimization to re-queue the tablet for compaction if there is still work to do (without waiting for the periodic check).  If we run a compaction and it does nothing, its likely there is no follow on compaction work to be done.
   
   For background, tablets are limited in the number of files they will compact at once.  So if a tablet had 100 files to compact and the limit is 20, then when it compacts 20 files it would be nice if immediately re-queued instead of waiting for the tablet server to check all tablets again.  With these changes, it will still re-queued immediately in that case.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services