sijie commented on a change in pull request #1281: Issue #570: Introducing
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r176658665
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
##########
@@ -209,16 +202,12 @@ public void onSizeLimitReached(final Checkpoint cp)
throws IOException {
public void run() {
try {
LOG.info("Started flushing mem table.");
- long logIdBeforeFlush = entryLogger.getCurrentLogId();
memTable.flush(SortedLedgerStorage.this);
- long logIdAfterFlush = entryLogger.getCurrentLogId();
// in any case that an entry log reaches the limit, we
roll the log and start checkpointing.
// if a memory table is flushed spanning over two entry
log files, we also roll log. this is
// for performance consideration: since we don't wanna
checkpoint a new log file that ledger
// storage is writing to.
- if (entryLogger.reachEntryLogLimit(0) || logIdAfterFlush
!= logIdBeforeFlush) {
- LOG.info("Rolling entry logger since it reached size
limitation");
- entryLogger.rollLog();
+ if (entryLogger.rollLogsIfEntryLogLimitReached()) {
Review comment:
@reddycharan well, the existence of the code had its reason. performance is
one, I think the biggest problem is "checkpoint" logic can potentially never be
triggered.
- during memtable flushes, rollLog is disabled for appending memtable
entries.
- however background compaction can also advance entrylog, since it is
appending entries into same entrylog.
- if a memtable flush spanning over two entry log files,
`rollLogsIfEntryLogLimitReached` will most likely be false, so no checkpoint
will happen.
- in the next memtable flush, if it is spanning over two entry log files
again, then `rollLogsIfEntryLogLimitReached` still be false, no checkpoint will
happen.
This was actually a behavior we observed at Twitter 3 years ago. I added
this logic to prevent this happen. see
twitter/bookkeeper@a7a8c88eaa2659b5d6c4e29f4eb9e2d184c7cfe0
----------------------------------------------------------------
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
