sijie commented on a change in pull request #1281: Issue #570: Introducing
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r177535531
##########
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:
@ivankelly that's what I commented before. I don't think the methods in
EntryLogManager provide the right abstraction. That's what I also dislike your
approach on asking moving the class out at this PR. At this PR, we need to
figure out what are the right methods to put in an EntryLogManager before any
kind of code movement. You comments about inner classes complicate things.
----------------------------------------------------------------
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
