Skip to content

Commit 2d35897

Browse files
committed
Fix lastMark regression in checkpoint completion
1 parent 6f966be commit 2d35897

2 files changed

Lines changed: 226 additions & 85 deletions

File tree

bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,8 @@ public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf
704704
lastMarkFileName = LAST_MARK_DEFAULT_NAME + "." + journalIndex;
705705
}
706706
lastLogMark.readLog();
707+
lastPersistedMark.setLogMark(
708+
lastLogMark.getCurMark().getLogFileId(), lastLogMark.getCurMark().getLogFileOffset());
707709
if (LOG.isDebugEnabled()) {
708710
LOG.debug("Last Log Mark : {}", lastLogMark.getCurMark());
709711
}
@@ -770,7 +772,7 @@ public Checkpoint newCheckpoint() {
770772
/**
771773
* Telling journal a checkpoint is finished.
772774
*
773-
* <p>Skips if the checkpoint is not newer than the last persisted mark,
775+
* <p>Skips if the checkpoint is older than the last persisted mark,
774776
* preventing lastMark from regressing backwards.
775777
*
776778
* @throws IOException
@@ -783,16 +785,16 @@ public void checkpointComplete(Checkpoint checkpoint, boolean compact) throws IO
783785
LogMarkCheckpoint lmcheckpoint = (LogMarkCheckpoint) checkpoint;
784786
LastLogMark mark = lmcheckpoint.mark;
785787

786-
// Skip if this mark is not newer than what was already persisted.
788+
// See #4105: keep the monotonic decision and lastMark persistence together.
787789
synchronized (checkpointLock) {
788790
if (mark.getCurMark().compare(lastPersistedMark) < 0) {
789791
return;
790792
}
793+
persistLastLogMark(mark);
791794
lastPersistedMark.setLogMark(
792795
mark.getCurMark().getLogFileId(), mark.getCurMark().getLogFileOffset());
793796
}
794797

795-
mark.rollLog(mark);
796798
if (compact) {
797799
// list the journals that have been marked
798800
List<Long> logs = listJournalIds(journalDirectory, new JournalRollingFilter(mark));
@@ -814,6 +816,16 @@ public void checkpointComplete(Checkpoint checkpoint, boolean compact) throws IO
814816
}
815817
}
816818

819+
@VisibleForTesting
820+
protected void persistLastLogMark(LastLogMark mark) throws NoWritableLedgerDirException {
821+
mark.rollLog(mark);
822+
}
823+
824+
@VisibleForTesting
825+
protected boolean isCheckpointCompleteLockHeldByCurrentThread() {
826+
return Thread.holdsLock(checkpointLock);
827+
}
828+
817829
/**
818830
* Scan the journal.
819831
*

0 commit comments

Comments
 (0)