tlf_journal: protect against crasher during resolveBranch
#21720
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A user hit the following issue: 1. Their client completed a large conflict resolution, and called `tlfJournal.resolveBranch()` to finalize it. 2. The new commit was written to a new mdJournal directory, which was then swapped into place, making it the main MD journal. 3. Then, KBFS restarted, _before_ `resolveBranch()` had a chance to mark all the MD revision markers in the block journal as ignorable. 4. After the restart, the journal resumed flushing blocks. Concurrently, the user wrote more revisions into the MD journal, re-using some of the same revision numbers that had previously been conflict-resolved away in step 1. 5. While flushing blocks, when the TLF journal discovered a revision marker for an old MD revision (which had originally been resolved and squashed in step 1), it decided it was ok to flush a _new_ revision with that same revision number. This resulted in the revision being live to other clients _before_ the blocks associated with that revision were available, and other clients could not read the TLF. To protect against such a race, this commit adds a new notion of a "journal ID" for the MD journal. Each time the MD journal is cleared or resolved, and then a new revision is written into it, it gets a new unique ID that is persisted to a file. Also, each MD revision marker gets labeled with the current MD journal ID. When flushing, the code ignores any MD revision marker that doesn't match the current MD journal ID. That way if KBFS crashes/restarts after clearing the MD journal, but before updating the block journal, the old MD revision markers will be properly ignored when the flush resumes. Issue: HOTPOT-1553
songgao
approved these changes
Dec 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A user hit the following issue:
tlfJournal.resolveBranch()to finalize it.resolveBranch()had a chance to mark all the MD revision markers in the block journal as ignorable.To protect against such a race, this commit adds a new notion of a "journal ID" for the MD journal. Each time the MD journal is cleared or resolved, and then a new revision is written into it, it gets a new unique ID that is persisted to a file. Also, each MD revision marker gets labeled with the current MD journal ID. When flushing, the code ignores any MD revision marker that doesn't match the current MD journal ID. That way if KBFS crashes/restarts after clearing the MD journal, but before updating the block journal, the old MD revision markers will be properly ignored when the flush resumes.
Issue: HOTPOT-1553