[GitHub] sijie commented on a change in pull request #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

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

sijie commented on a change in pull request #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#discussion_r159578472
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
 ##########
 @@ -264,12 +278,53 @@ public void safeRun() {
             // collection cycle started
             forceGarbageCollection.set(false);
         }
+
+    }
+
+    /**
+     * increase the threshold when the system in low load status.
+     */
+    private void activateMedianThreshold(){
+        majorCompactionThreshold = medianMajorCompactionThreshold;
+    }
+    /**
+     * increase the threshold when the system in a very low load status.
+     */
+    private void activateHighThreshold(){
+        majorCompactionThreshold = highMajorCompactionThreshold;
+    }
+
+    /**
+     * Check whether the configured cron is met and activate threshold.
+     */
+    private void changeMajorCompactionThreshold(){
+        // next fire time is in the interval
+        if (medianMajorCron.nextTimeAfter(ZonedDateTime.now()).toInstant().toEpochMilli()
 
 Review comment:
   > Cron is great for defining scheduled tasks/timers. It is confusing when applied to interval ranges.
   
   +1 I think one of my comments before was : not changing the threshold, only use cron for triggering compaction. I think that is the most valuable thing to have cron. The cron can also used for triggering other timer tasks, like when to schedule auditing task.
   
   > Overall yes, it does cover that one usecase but I'd love to be able to easily reuse cron functionality for other cases since we are adding code to deal with crons. I think this will push the change from "ok" to "awesome".
   
   +1
   
   I think we can simply this PR with two functionalities:
   
   1) introduce cron, and be able to schedule a task based on cron.
   2) don't adjust compaction thresholds, only use cron to schedule when to do compaction.

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