⚡ perf(ha): bound the ownership snapshot - #2029
Merged
Merged
Conversation
Every committed RecordTransfer pushed a TransferRecord onto the authority's record in replicated state and nothing ever removed one, so the snapshot grew with the number of moves the group had applied rather than with the authorities it still homed. At 200,000 authorities each moved eight times the snapshot measured 97,800,047 bytes, 73,200,000 of which was move history at 45.75 bytes per transfer. Every build serialized and cloned that whole buffer, and a follower rejoining after a disk replacement downloaded all of it, deleted repositories included, before it could apply a new entry. Nothing read the trail. OwnershipState::transfers and TransferRecord had no caller outside the crate's own tests, and no command consulted record.transfers to decide anything, so no cross-transaction reader broke by dropping them. A bounded window would have been a lossy partial copy of a record that already exists: commit_transfer persists a TransferAudit carrying the actor, reason, barrier, and commit index to the MetaStore, which the planned-transfer contract already names the record operators answer from. Keeping the last N moves in consensus would be too incomplete to audit from and still charge every snapshot transfer, so the replicated state now keeps live state only. ForgetAuthority gives a retired repository a way out of that state. It rides the existing operator command endpoint, applies in the same decision that records its receipt, commits as no_change when nothing is homed under the authority, and is refused while a write lease is live, since the lease holder still stamps work with the epoch it drops.
Merging this PR will not alter performance
Comparing Footnotes
|
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.
OwnershipStategrew with every move the group applied, not with the authorities it still homed. Each committedRecordTransferpushed aTransferRecordonto the authority's record and no command removed one, so a repository deleted a year ago still contributed its record, its epoch, and each move it ever made. At the population #1584 describes, 200,000 authorities each moved eight times during rolling restarts,OwnershipState::snapshot()measured 97,800,047 bytes. 73,200,000 of that was move history, 45.75 bytes per transfer.build_snapshotserialized and cloned the whole buffer on every build, and a follower rejoining after a disk replacement downloaded all of it before it could apply a new entry. The same population now measures 24,600,047 bytes, 123 bytes per live authority.Nothing read the trail.
OwnershipState::transfersandTransferRecordhad no caller outside this crate's own tests, and no command consultedrecord.transfersto reach a decision:assign_homereadshomeandepoch,transferandadvance_epochreadhomeandwrites. A bounded window would have held a partial copy of something that already exists, sincecommit_transferpersists aTransferAuditcarrying the authority, source, target, actor, reason, barrier, epoch, and commit index to theMetaStore, which the planned transfer contract names as the record an operator and reconciliation answer from. Keeping the last N moves in consensus would still charge every follower that downloads a snapshot while leaving the history too incomplete to audit from, so the replicated map now holds live state only: home, epoch, assignment provenance, and outstanding write leases. That is the shape #2023 took rather than the table split in #2011, because a durable record already owns the history.ControlCommand::ForgetAuthorityremoves a retired repository from the map. Operators send it to the existingPOST /availability/v1/commandsendpoint, so there is no new route, and it applies inside the same decision that records its receipt, astransfer_authorityandadvance_epochdo. It commits asno_changewhen the state homes nothing under the authority, which makes a retry after a committed forget safe, and consensus refuses it while a write lease is live, because the lease holder still stamps work with the epoch it drops. Publishing to a forgotten authority assigns it again at epoch one.One copy per snapshot build remains.
TypeConfigfixes openraft'sSnapshotDatatoCursor<Vec<u8>>, sobuild_snapshothas to hand its caller an ownedVec, and removing that copy would mean changing the openraft contract. Dropping the growth term bounds that copy, which now scales with live authorities instead of with every move the group has applied.#1584 lists #1451 as a blocker, which this change does not stand on. The durable audit table exists on
maintoday and takes a row on every committed planned transfer, while #1451 covers recovering an audit whose write failed after the ownership commit. The replicated trail could not have served that recovery, becauseTransferRecordcarried none of the actor, reason, barrier, or commit index the audit answers with.Closes #1584