sijie commented on a change in pull request #1281: Issue #570: Introducing
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r176654536
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
##########
@@ -167,14 +167,7 @@ public ByteBuf getEntry(long ledgerId, long entryId)
throws IOException {
@Override
public void checkpoint(final Checkpoint checkpoint) throws IOException {
- long numBytesFlushed = memTable.flush(this, checkpoint);
- if (numBytesFlushed > 0) {
- // if bytes are added between previous flush and this checkpoint,
- // 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();
- }
+ memTable.flush(this, checkpoint);
Review comment:
@reddycharan
> First of all I’m not sure, why this is added with this commit 81cbba3.
I believe there was whole discussion in the original PR introducing this.
And I agreed with your logic in the original PR. I am not arguing again about
the correctness.
what I am suggesting here is this change is unrelated to the introduction of
EntryLogManager. This change is more related to the subsequent change your
want to introduce. What I am suggesting is putting this change as part of your
subsequent change, that is much clearer when people looking back into the
histories. Because they are related.
----------------------------------------------------------------
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
