On Tue, Oct 17, 2017 at 11:46 AM, Charan Reddy G <reddychara...@gmail.com> wrote:
> @Sijie..Did you get chance to go through the scenario/code path. > Just come back to US today. I will check and get back to you. > > @JV..will create a bug, once I get clarity. > > Thanks, > Charan > > On Fri, Oct 13, 2017 at 5:19 PM, Venkateswara Rao Jujjuri < > jujj...@gmail.com> wrote: > >> 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 >> > >
