[GitHub] athanatos 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>]

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

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
 ##########
 @@ -68,9 +69,16 @@
     long lastMinorCompactionTime;
 
     boolean enableMajorCompaction = false;
-    final double majorCompactionThreshold;
+    final double originalMajorCompactionThreshold;
+    double majorCompactionThreshold;
 
 Review comment:
   I don't think you want to maintain both majorCompactionThreshold and originalMajorCompactionThreshold.  majorCompactionThreshold is mainly used in two places:
   1. In the constructor, you do some validation of the config, but you don't need to change it to do that.
   2. In runWithFlags, you set it based on the current time, pass it into doCompactEntryLogs, and restore it back to original before returning.
   
   Instead, rename originalMajorCompactionThreshold to majorCompactionThreshold (and leave it final) and get rid of the mutable majorCompactionThreshold.  In runWithFlags, modify changeMajorCompactionThreshold to getCurrentMajorCompactionThreshold and let it return a value based on the current time which you can keep in a local in runWithFlags.  You don't need restoreThreshold at all.
     

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