ivankelly commented on a change in pull request #1236: Issue #570: make changes 
to SyncThread/checkpoint logic.
URL: https://github.com/apache/bookkeeper/pull/1236#discussion_r173430929
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
 ##########
 @@ -696,14 +698,29 @@ public Bookie(ServerConfiguration conf, StatsLogger 
statsLogger)
         ledgerStorage = 
LedgerStorageFactory.createLedgerStorage(ledgerStorageClass);
         syncThread = new SyncThread(conf, getLedgerDirsListener(), 
ledgerStorage, checkpointSource);
 
+        Checkpointer checkpointer;
+        /*
+         * with this change https://github.com/apache/bookkeeper/pull/677,
+         * LedgerStorage drives the checkpoint logic. But with multiple entry
+         * logs, checkpoint logic based on a entry log is not possible, hence 
it
+         * needs to be timebased recurring thing and it is driven by 
SyncThread.
+         * SyncThread.start does that and it is started in Bookie.start method.
+         */
+        if (entryLogPerLedgerEnabled) {
 
 Review comment:
   What you're basically doing here is creating two implementations of 
syncthread. One which is periodic, and one which uses checkpoint. This should 
be handled at the class level, rather than with boolean flags. 
   
   If you don't want to go to the trouble of creating a bunch of new classes, 
then do.
   ```
   if (conf.isEntryLogPerLedgerEnabled()) {
       syncThread = new SyncThread(conf, getLedgerDirsListener(), 
ledgerStorage, checkpointSource) {
           @Override
           public void checkpoint(Checkpoint checkpoint) {
               // A comment for why it's disabled
           }
   
           @Override
           public void start() {
               
syncThread.scheduleCheckpointAtFixedRate(conf.getFlushInterval());
           }
       };
   } else {
       syncThread = new SyncThread(conf, getLedgerDirsListener(), 
ledgerStorage, checkpointSource);
   }
   ```
   
   Add a start method to SyncThread and always call it in Bookie#start. There 
used to be one, and it makes sense for there to be one, since it is called 
SyncThread.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to