sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180524406
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##########
 @@ -173,7 +175,7 @@ public void checkpoint(final Checkpoint checkpoint) throws 
IOException {
             // it means bytes might live at current active entry log, we need
             // roll current entry log and then issue checkpoint to underlying
             // interleaved ledger storage.
-            entryLogger.rollLog();
+            entryLogger.rollLogs();
 
 Review comment:
   I think `numBytesFlushed > 0` logic is a behavior that specific to 
single-log manager. `rollLogs` is a not proper term for the meaning here. I 
would suggest adding a method in the EntryLogManager:
   
   `void prepareCheckpoint(Checkpoint checkpoint, long 
numBytesFlushedBetweenCheckpoints)` -> it means prepare checkpoint in the entry 
logger. 
   
   Then logic here can be changed to:
   
   ```
   long numBytesFlushed = memTable.flush(this, checkpoint);
   entryLogger.prepareCheckpoint(checkpoint, numBytesFlushed);
   super.checkpoint(checkpoint);
   ```
   
   in single-log manager, you can implement 
   
   ```
   void prepareCheckpoint(Checkpoint checkpoint, long 
numBytesFlushedBetweenCheckpoint) {
       if (numBytesFlushedBetweenCheckpoint > 0) {
           rollLogs();
       }
   }
   ```
   
   in per-ledger-log manager, it is a no-op.
   
   so in this way, you can get rid of `rollLogs` in EntryLogManager.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to