A crash partway through an append stopped the metaserver coming back - #527
Open
bjmeetsfo wants to merge 3 commits into
Open
A crash partway through an append stopped the metaserver coming back#527bjmeetsfo wants to merge 3 commits into
bjmeetsfo wants to merge 3 commits into
Conversation
The metadata log is append-only text, one record per line. A process that dies partway through writing one leaves a partial line at the end. That is what a crash looks like, not corruption. Reading the log refused the whole file on any line that did not parse, and starting a metaserver passes that straight up, so the process failed to start. A metaserver whose metadata was entirely durable except for a fraction of the last record would not come up at all. A partial last record was never acknowledged. A writer only returns once a sync covering its bytes has completed, so nothing was ever promised to anybody about the record still being written. Dropping it loses nothing that was reported durable, and everything before it is kept. A line that does not parse with records AFTER it is a different thing. Those later records were acknowledged, and stopping at the damage would discard them silently. That is still refused, and the error says why it is not simply a crash. The partial record is not dropped quietly either: it is logged with the file, the line, how many records were recovered, and what failed to parse. Two tests: a log cut off mid-record brings every acknowledged shard and server back, and a log damaged in the middle is refused rather than half-read. Both were checked against a mutation. Refusing any bad line -- the old behaviour -- fails the first. Skipping any bad line wherever it sits fails the second. Each fails with the message that names what went wrong.
Dropping the partial record on read lets the metaserver start, but leaves the
fragment in the file. Appends open for append and write at the end -- so the
next record is written immediately after those bytes, with no newline between
them, and the fragment is spliced onto the front of it:
{"at_ms":9,"mutation":{"RegisterSha{"at_ms":178...,"kind":"register_shard"...
That line no longer parses. It is also the LAST line, so the next restart reads
it as a torn tail and drops it -- and the record it swallowed was acknowledged.
The writer had been told it was durable.
So skipping the fragment on its own turned a loud refusal into the silent loss
of a write that was promised. Measured before this change: register a shard
after recovering, restart, and the shard is gone while the metaserver starts
happily.
Recovery now truncates the file to the end of the last record that parsed, and
syncs that -- the cut has to survive the crash that follows it, or the fragment
comes back and the next append splices onto it again. The following append then
starts on a record boundary.
Test: a shard registered after recovering from a crash is still there after the
next restart, and every line of the recovered log parses, so nothing is left
waiting to swallow a record.
Checked against a mutation: leave the fragment in place -- which is what the
first version of this did -- and the test fails, saying the acknowledged write
was lost.
This file contains hidden or 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
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.
The metadata log is append-only text, one record per line. A process that dies partway through writing one leaves a partial line at the end. That is what a crash looks like, not corruption.
Reading the log refused the whole file on any line that did not parse:
and starting a metaserver passes that straight up (
with_mutation_log->log.load()?->.transpose()?in the backend). So the process failed to start. A metaserver whose metadata was entirely durable except for a fraction of the last record would not come up at all.Why dropping the last record is safe
It was never acknowledged. A writer only returns once a sync covering its bytes has completed, so nothing was ever promised to anybody about the record still being written. Dropping it loses nothing that was reported durable, and everything before it is kept.
Why the middle is different
A line that does not parse with records after it is not a torn tail. Those later records were acknowledged, and stopping at the damage would discard them silently. That is still refused, and the error says why it is not simply a crash:
The distinction is the whole point: the tail is expected, the middle is not.
Not silent either way
The dropped partial record is logged with the file, the line number, how many records were recovered, and what failed to parse. A metaserver that came back one record short says so.
Testing
Both were checked against a mutation:
Suites: 328 metadata tests, 47 metaserver binary tests,
cargo check --all-targetsclean.