Add pair-scoped swap APIs - #277
Draft
PranaviAncha wants to merge 7 commits into
Draft
PranaviAncha wants to merge 7 commits into
PranaviAncha wants to merge 7 commits into
Conversation
The existing swap APIs take one instance and find its peer by matching logical ids at call time, so the peer is an output of the call rather than an input. A caller cannot state which instance it meant, a pairing that became ambiguous between two calls is resolved silently, and the completion writes a config that was derived from an earlier read with no check that the read still holds. prepareSwapPair and completeSwapPair take both instances, validate that they are each other's only peer, and write both configs in one transaction conditional on the config versions the decision was made from. Callers may additionally assert the identity (creation id and version) of either config, so a config that was replaced or changed since they read it is refused rather than written over. The outcome is an explicit status rather than a boolean, so a replay of a call that already took effect, a conflict with another writer, replicas not being ready, and a pair that is no longer a pair are all distinguishable. A replayed preparation or completion writes nothing. Preparation is mode specific. A coordinated swap moves only the logical id and marks the swap-in, leaving every other field it carries alone. A direct swap transfers the whole topology slot, keeping the domain keys the caller names, and sets no instance operation so the swap-in stays unassignable until completion. Force still skips only the replica readiness checks. It does not skip pair validation, identity assertions or the conditional write. A conditional write can only assert a data version, so a config that is deleted and created again is only caught because the new node restarts at version 0. That argument does not hold when the asserted version is itself 0, so a request asserting an identity on such a config is refused with IDENTITY_UNVERIFIABLE rather than granted on a guarantee that does not hold. The existing canCompleteSwap and completeSwapIfPossible commands and their response shape are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Local review pointed out that a coordinated preparation accepted a swap-in in ENABLE, which is assignable capacity. Marking such an instance SWAP_IN takes it out of the assignable set and points it at the swap-out's assignment, so anything it was serving of its own would be stranded. The native instance operation rules never allow ENABLE to reach SWAP_IN at all. Preparation now requires an ENABLE swap-in to be carrying nothing of its own, which is what makes it a joining replacement rather than an active member, and reports an explicit blocker otherwise. UNKNOWN and SWAP_IN are unchanged, since neither is assignable. Review also noted that the creation id of a config that was only checked, not written, was reported from the pre-transaction read, so a replacement of that side went unnoticed. Both creation ids are now read back after the transaction. The Javadoc says plainly that this is detection after the fact rather than prevention, and that a call asserting no identity makes no identity promise at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous commit let a coordinated preparation mark an ENABLE swap-in as SWAP_IN as long as it was carrying nothing at the moment of the check. Local re-review pointed out that the check and the write are not atomic: the instance is assignable for the whole window in between, so the controller can hand it work that the write then strands. There is no point at which marking an assignable instance SWAP_IN is known to be safe. Preparation now accepts only UNKNOWN and SWAP_IN, which are exactly the states the native instance operation rules allow to reach SWAP_IN, and reports an explicit blocker naming the state to put the swap-in in otherwise. The emptiness check is removed rather than kept as a weaker guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refuse retargeting a swap-in already assigned to another logical slot.\nDocument the non-atomic creation-identity checks and cover replacement\nafter the initial read without treating FAILED as proof of no write. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preparation refused a pair whose swap-out was already retired, while completion recognised that same shape and reported ALREADY_COMPLETED. A caller that lost the response of a completion and restarted its flow from preparation was therefore stuck on PAIR_MISMATCH forever, even though the swap had already succeeded. Both paths now decide from one isCompletedSwapPair helper, so preparation reports ALREADY_COMPLETED and writes nothing for that terminal shape, and anything short of it keeps its existing refusal and status. A coordinated preparation moves only the logical id, so the swap-in keeps its own topology position and the slot could end up in another fault zone. Preparation now requires both configs to carry the same non-empty value for the cluster's fault zone type before it writes, reading that type from the cluster config this call already loaded. A cluster that defines no fault zone type is refused the way any other unresolvable topology is. A direct swap is exempt on purpose: it transfers the whole domain, so the swap-in inherits the swap-out's fault zone by construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A direct preparation deliberately writes no instance operation marker, so a swap-in it has already committed to one swap-out's topology slot still looks free: it stays UNKNOWN, and its domain differs from the next requested swap-out's domain, so the idempotent answer does not apply either. Preparing that swap-in for a second swap-out overwrote its logical id and left the first pair impossible to complete, because completion requires the swap-in to carry the first swap-out's logical id. The pair-scoping check on the swap-out side cannot see this: it looks for other instances carrying the requested swap-out's logical id and deliberately excludes the named swap-in. Both preparation modes now refuse a swap-in whose own logical id is carried by some instance other than the requested swap-out, reusing InstanceUtil.findInstancesWithMatchingLogicalId. Excluding the requested swap-out is what keeps a replay of the same pair idempotent, since a prepared swap-in shares its logical id with its own swap-out. The coordinated path is covered too, because its existing marker check only recognises a swap-in that a coordinated preparation marked, and a swap-in left behind by a direct preparation carries no marker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PranaviAncha
force-pushed
the
99163324+PranaviAncha/ownership-checked-swaps
branch
from
September 17, 2026 05:10
da1e786 to
cf84c3c
Compare
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
1 participant
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.
Issues
No linked GitHub issue.
Description
Add pair-scoped swap APIs that name both the swap-out and swap-in instead of discovering a peer from a single instance at call time.
getInstanceConfigIdentity,prepareSwapPair, andcompleteSwapPairare exposed through the REST commandsgetSwapPairIdentities,prepareSwapPair, andcompleteSwapPair.Preparation and completion validate the named pair, reject ambiguous or retargeted pairs, and use one transaction conditioned on both observed config versions. Preparation also refuses a swap-in whose current logical ID is held by an instance other than the requested swap-out, so a second pair cannot take a topology slot from an already prepared pair. Excluding the requested swap-out keeps replay of the same pair idempotent. Explicit statuses distinguish applied work, safe replay, readiness blockers, pair mismatch, identity mismatch, unverifiable identity, concurrent conflict, invalid requests, and failures. Preparation also recognizes the exact terminal shape of an already completed pair, so a caller that lost the completion response can recover without another write.
COORDINATED preparation moves only the logical ID and marks the swap-in as SWAP_IN. It preserves the swap-in's other fields, requires both instances to have the same non-empty value for the cluster fault-zone key, and refuses an existing SWAP_IN tied to another logical slot. DIRECT preparation transfers the topology slot while preserving caller-selected swap-in domain keys, refuses preservation of the logical-ID key, and requires the swap-in to be UNKNOWN. DIRECT does not require matching fault zones because it transfers the whole topology slot. Force completion skips only replica-readiness checks, not pair validation, identity checks, or the conditional write.
Identity checks are not atomic incarnation fences. Version-zero expectations are conservatively refused. A config can still be replaced and returned to an expected nonzero version before the transaction, and post-transaction creation-ID checks can then return FAILED after a write was applied. FAILED must not be interpreted as proof that nothing changed.
Tests
TestZkHelixAdminPairScopedSwapcases.TestPairScopedSwapAPIcases.Changes that Break Backward Compatibility (Optional)
The new
HelixAdminmethods are additive default methods that report unsupported behavior for implementations that do not override them. Existing single-instance swap methods and their REST response shapes are unchanged.The new REST command values require provider-first deployment. Every serving REST instance must understand them before clients enable the pair-scoped path. An older server rejects the command rather than partially applying a swap.
Documentation (Optional)
Public API Javadocs document both swap modes, the coordinated fault-zone requirement, replay behavior, force semantics, pair validation, identity limitations, version-zero refusal, and the possibility that FAILED follows an applied write.
Commits
The seven focused commits add the pair-scoped contract, strengthen swap-in state checks, reject unsafe logical-ID preservation, protect existing pairs from retargeting or slot takeover, close terminal-replay and fault-zone validation gaps, and clarify identity guarantees.
Code Quality
The implementation keeps the existing API intact, centralizes pair validation and result reporting, and uses explicit outcomes instead of collapsing retries, blockers, and conflicts into one boolean.
Open Question for Reviewers
DIRECT preparation currently accepts any
preservedSwapInDomainKeysentry except the logical-ID key. A caller that names the cluster fault-zone key therefore keeps the swap-in's own fault zone while the swap-in takes over the swap-out's logical ID, so the slot changes fault zone. That is deliberate: the caller asked for it by name, rather than the API making a placement decision on its own, and DIRECT otherwise copies the whole swap-out domain. It is nonetheless asymmetric with COORDINATED, which refuses a pair whose fault zones differ precisely because that call must not move a slot between zones.Should DIRECT also refuse preservation of the fault-zone key (or of topology keys generally, excepting host), or is the explicit opt-in the right boundary? Flagging it because it changes what callers are allowed to express, and it is easier to restrict now than after the contract ships.
Generated with GitHub Copilot CLI