hangc0276 commented on PR #3194: URL: https://github.com/apache/bookkeeper/pull/3194#issuecomment-1189925186
> Please add tests, at least to: > > * make sure journals aren't reused when feature is disabled. > * cover recovery from journal/after crash with reused journal. is it possible to use wrong journal file in that case? or end up with corrupted one? What happens with last log mark and checkpointing? e.g. see BookieJournalTest. > * Bookie's readJournal()/replay() scan journals sequentially because Journal.listJournalIds assumes they are. With this option the assumption changes to circular with unclear stop point. @dlg99 Thanks for your review. Please refer to: https://github.com/apache/bookkeeper/pull/3194#issuecomment-1189912416 For the test, we need a special FileChannel which records the current written position when rewriting an existing journal file. I'm thinking about how to add the test. > * cover recovery from journal/after crash with reused journal. is it possible to use wrong journal file in that case? or end up with corrupted one? What happens with last log mark and checkpointing? e.g. see BookieJournalTest. We rename the old journal file to the new one and keep the journal file name increasing as the current behavior. It can avoid complex cases such as using the wrong journal file and so on. > * Bookie's readJournal()/replay() scan journals sequentially because Journal.listJournalIds assumes they are. With this option the assumption changes to circular with unclear stop point. We rename the old journal file to the new one, we can file the stop point with the largest file name on the journal replay stage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
