[GitHub] [bookkeeper] codelipenghui commented on a change in pull request #2670: Issue 2638: Compaction Limits

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

codelipenghui commented on a change in pull request #2670:
URL: https://github.com/apache/bookkeeper/pull/2670#discussion_r605359761



##########
File path: conf/bk_server.conf
##########
@@ -500,6 +500,9 @@ ledgerDirectories=/tmp/bk-data
 # If it is set to less than zero, the minor compaction is disabled.
 # minorCompactionInterval=3600
 
+# Limit in milliseconds to run minor Compaction. Defaults to -1 run indefinitely.
+# minorCompactionLimitMs=-1

Review comment:
       ```suggestion
   # minorCompactionMaxTimeInMillis=-1
   ```

##########
File path: conf/bk_server.conf
##########
@@ -522,6 +525,9 @@ ledgerDirectories=/tmp/bk-data
 # If it is set to less than zero, the major compaction is disabled.
 # majorCompactionInterval=86400
 
+# Limit in milliseconds to run major Compaction. Defaults to -1 run indefinitely.
+# majorCompactionLimitMs=-1

Review comment:
       ```suggestion
   # majorCompactionMaxTimeInMillis=-1
   ```

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
##########
@@ -452,31 +457,48 @@ void doCompactEntryLogs(double threshold) {
 
         final int numBuckets = 10;
         int[] entryLogUsageBuckets = new int[numBuckets];
+        int[] compactedBuckets = new int[numBuckets];
+
+        long start = System.currentTimeMillis();
+        long end = start;
+        long timeDiff = 0;

Review comment:
       Do we need the `end` and the `timeDiff` here? Seems we can only break the following loop if (currentTime-start) >= limit?

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
##########
@@ -452,31 +457,48 @@ void doCompactEntryLogs(double threshold) {
 
         final int numBuckets = 10;
         int[] entryLogUsageBuckets = new int[numBuckets];
+        int[] compactedBuckets = new int[numBuckets];
+
+        long start = System.currentTimeMillis();
+        long end = start;
+        long timeDiff = 0;
 
         for (EntryLogMetadata meta : logsToCompact) {
             int bucketIndex = Math.min(
                     numBuckets - 1,
                     (int) Math.ceil(meta.getUsage() * numBuckets));
             entryLogUsageBuckets[bucketIndex]++;
 
-            if (meta.getUsage() >= threshold) {
+            if (timeDiff < limit) {
+                end = System.currentTimeMillis();
+                timeDiff = end - start;
+            }
+            if (meta.getUsage() >= threshold || (limit > 0 && timeDiff > limit) || !running) {
+                // We allow the usage limit calculation to continue so that we get a accurate
+                // report of where the usage was prior to running compaction.
                 continue;

Review comment:
       We should break the loop if exceed the max compaction time? all the remaining logs will be skipped.




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

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