meta: do not convict the last live holder of a shard - #76
Open
bjmeetsfo wants to merge 1 commit into
Open
Conversation
Conviction is decided entirely on heartbeat evidence, which says nothing about what the convicted server is holding. So the metaserver will happily freeze the only node serving a shard. That makes the shard unroutable, and then hands auto-rebalance a shard to "recover" onto a node that has none of its data. The orphan guard asks the question conviction never did: after this round's freezes land, does every shard still have somebody serving it? It runs over the whole round rather than one server at a time, which is what catches the case a per-server check cannot. Two servers each holding one of the only two copies of a shard are individually safe to freeze - the other still has it - and freezing both loses it. When a shard would be left unserved, every candidate serving it is pulled back, which guarantees the shard keeps a holder in a single deterministic pass. "Serving" means the same thing here as it does to the divergence check: the guard reuses that classifier through a new shard_check::serving_shards, so a shard mid-load or one the node has given up on does not count as a holder, and readonly does. A server that is already frozen is not a survivor either, whatever it last reported - counting it would let the guard wave through a conviction that does orphan the shard. Safe mode still takes precedence. It is the coarser guard and runs first; the orphan guard only ever pulls back convictions that survived it. The orphaned shards are reported whether or not the guard is enabled, and the adaptive loop logs them, because a cluster one conviction away from losing a shard is worth an alert even when the policy has chosen to convict anyway. Off by default (TS_META_FORBID_ORPHANING_SHARDS). A node that is genuinely gone is not serving the shard either, so holding its conviction back keeps a dead node in the topology and stops the freeze that would let rebalancing move the shard somewhere useful. Which trade is right depends on whether the shard's data is recoverable elsewhere, so it is a deployment decision. The reference makes the same call: its equivalent flag defaults to forcing the freeze and logging. 9 new tests: the sole holder held back, a shard with another live holder still convictable, two servers holding the only copies both pulled back, a server holding one doomed and one safe shard still pulled back, an orphan reported with the guard off, an already-frozen holder not counting as a survivor, a server serving nothing convicted normally, safe mode taking precedence, and an end-to-end round reading the serving set from the heartbeat.
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 problem
Conviction is decided entirely on heartbeat evidence, which says nothing about what the convicted server is holding. So the metaserver will happily freeze the only node serving a shard.
That makes the shard unroutable — and then hands auto-rebalance a shard to "recover" onto a node that has none of its data.
The reference metaserver checks this before freezing (
CanFreezePartitionSafely): it refuses when the partition has no other healthy replica.What this adds
The orphan guard asks the question conviction never did: after this round's freezes land, does every shard still have somebody serving it?
It runs over the whole round, not one server at a time — which is what catches the case a per-server check cannot. Two servers each holding one of the only two copies of a shard are individually safe to freeze (the other still has it) and freezing both loses it. When a shard would be left unserved, every candidate serving it is pulled back, which guarantees the shard keeps a holder in a single deterministic pass.
"Serving" means the same thing here as to the divergence check. The guard reuses that classifier through a new
shard_check::serving_shards, so a shard mid-load or one the node has given up on does not count as a holder, andreadonlydoes. A server that is already frozen is not a survivor either, whatever it last reported — counting it would let the guard wave through a conviction that does orphan the shard.Safe mode still takes precedence. It is the coarser guard and runs first; the orphan guard only ever pulls back convictions that survived it.
Reported either way
The orphaned shards are recorded whether or not the guard is enabled, and the adaptive loop logs them:
A cluster one conviction away from losing a shard is worth an alert even when the policy has chosen to convict anyway.
Off by default
TS_META_FORBID_ORPHANING_SHARDS0A node that is genuinely gone is not serving the shard either, so holding its conviction back keeps a dead node in the topology and stops the freeze that would let rebalancing move the shard somewhere useful. Which trade is right depends on whether the shard's data is recoverable elsewhere, so it is a deployment decision. The reference makes the same call — its equivalent flag defaults to forcing the freeze and logging.
Tests
9 new tests: the sole holder held back, a shard with another live holder still convictable, two servers holding the only copies both pulled back, a server holding one doomed and one safe shard still pulled back, an orphan reported with the guard off, an already-frozen holder not counting as a survivor, a server serving nothing convicted normally, safe mode taking precedence, and an end-to-end round reading the serving set from the heartbeat.
Verification:
cargo test -p temporalstore-rust --lib meta::failure_detector— 37 passed, 0 failed.cargo test -p temporalstore-rust --lib meta— 197 passed, 0 failed.cargo test -p temporalstore-rust --bin metaserver— 18 passed, 0 failed.cargo build -p temporalstore-rust --bin metaserver— clean, no new warnings.Independent of #75, though both touch
failure_detector.rs.