Hey Sijie, I'm not questioning the semantics of checkpoint or the optimization which was added with Bookkeeper-564. But my concern is are we sure, checkpoint logic/code is correct and "marker is only updated when all the entries added before are persisted.", in the case of SortedLedgerStorage. Can you please go through the scenario I mentioned in my email. From what I understood, if entryLog is rotated because of addEntry request from GC, then we set the 'currentMark' of journal as 'lastCheckpoint' in checkpointHolder. The entries added before this 'lastCheckpoint' are still in EntryMemTable. When next checkpoint happens, we are not actually persisting entries which were in EntryMemTable but we are marking 'lastCheckpoint' as LogMarkCheckpoint (in checkpointComplete method).
Thanks, Charan On Fri, Oct 13, 2017 at 2:46 PM, Sijie Guo <guosi...@gmail.com> wrote: > The core of the checkpoint is: > > - marker is only updated when all the entries added before are persisted. > That means it doesn't affect correctness if entries added after are > flushed. > > - the flush in entry log files is just writing data to filesystem. The real > fsync happens after checkpoint. The separate is for performance > consideration. > > > > On Oct 12, 2017 11:34 PM, "Charan Reddy G" <reddychara...@gmail.com> > wrote: > > > Hey Sijie/IvanK, > > > > With > > https://github.com/apache/bookkeeper/commit/ > d175ada58dcaf78f0a70b0ebebf489 > > 255ae67b5f > > you introduced Bookkeeper-564 : Better checkpoint mechanism - Scheduling > > checkpoint only when rotating an entry log file. > > > > I'm trying to understand how it would work in the following scenario > > - using SortedLedgerStorage > > - since it is SortedLedgerStorage entries would be in EntryMemtable > > - GarbageCollectorThread.EntryLogScanner.process method calls > entryLogger > > .addEntry(ledgerId, entry) > > - in EntryLogger.addEntry method, lets say it comes to know it has > reached > > EntryLogLimit and creates NewLog > > - since current active entrylog is rotated, > > EntryLogListener.onRotateEntryLog is called > > - which sets the currentMark of journal to checkpointHolder. Point to > note, > > that all the entries added to the Bookie are not added to entryLog yet, > > there are entries still in entrymemtable > > - lets say SyncThread tries to checkpoint at this instant > > > > now the concern is, in SortedLedgerStorage.checkpoint method, before > > calling super.checkpoint(checkpoint), it does memTable.flush(this, > > checkpoint); But memTable.flush would just add entries to the current > > active entrylog (BufferedLogChannel) and it doesn't guarantee > persistence. > > super(InterLeavedLedgerStorage).checkpoint will only flushRotatedLogs > > (persists) and finally mark the checkpointcomplete with 'lastcheckpoint', > > but the 'lastCheckpoint' in the checkpointHolder would also include the > > entries which were in Entrymemtable and are not actually persisted in the > > whole process. Is there issue in SortedLedgerStorage checkpoint logic? > > > > @Override > > public Checkpoint checkpoint(final Checkpoint checkpoint) throws > > IOException { > > Checkpoint lastCheckpoint = checkpointHolder. > getLastCheckpoint(); > > // if checkpoint is less than last checkpoint, we don't need to > do > > checkpoint again. > > if (lastCheckpoint.compareTo(checkpoint) > 0) { > > return lastCheckpoint; > > } > > memTable.flush(this, checkpoint); > > return super.checkpoint(checkpoint); > > } > > > > Thanks, > > Charan > > >
