Charan this looks like an issue to me. Do we have a defect/issue opened? On Fri, Oct 13, 2017 at 4:18 PM, Sijie Guo <guosi...@gmail.com> wrote:
> Charan, > > Didn't mean to say the logic is correct. I was just trying to point out > the points that I remembered for checkpoint. > > I am currently traveling, so I don't have code available to check the > sequence. I can check the logic when I am close to the laptop. > > Sijie > > > On Oct 14, 2017 6:11 AM, "Charan Reddy G" <reddychara...@gmail.com> wrote: > > 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 > > > > > > > > -- Jvrao --- First they ignore you, then they laugh at you, then they fight you, then you win. - Mahatma Gandhi
