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