Repository navigation
Conversation
…pens within a round trip instead of at the deadline The MoveTo/Recycle paths drained by waiting for "no caller work" before replacing the connections. Under steady traffic that never happens: there is always a command in flight, so the drain waited out the entire announced window and recycled at the deadline, failing whatever was in flight at that instant and leaving no time to establish the replacement. Measured live on a Redis Enterprise endpoint rebind with ~450 ops/s on the connection: MOVING at +5.8s, MoveTo decided at +6.6s, connections replaced at +20.9s against a 15s window, 8 failed commands, and the "did not establish a replacement within the announced window" warning. The bridge now has a quiesce mode for the duration of the drain: new caller messages (single, batch, sync and async) go to the backlog instead of the socket, the backlog processor does not write while quiescing, and the drain waits only for commands already written. Held work is written to the replacement once it is established, as across any reconnect; a FailFast backlog policy no longer abandons it for a MaintenanceHandoff, which is a replacement we chose rather than a failure. Internal traffic (keep-alives) is not held. Same scenario with this change: MOVING +5.8s, handoff +6.7s, restored +6.8s, 0 failed, nothing outstanding at the recycle, no warning. The new test runs a steady load against the in-process server and asserts that the recycle follows the notification within a fraction of the window and that no command fails; it fails on main (recycle at ~10s, the whole window) and passes here.
…live caller work Review follow-ups on the quiesce-then-drain change: - The drain is capped at one second as well as by the window. A blocking or stuck command in flight would otherwise hold everyone else's work in the backlog for the rest of the window and still be failed at the end of it; past the cap the connections are replaced regardless, and whatever was still in flight fails as the server's close would have failed it. - The recycle takes the bridge's write lock and re-checks for in-flight work under it, so a writer that passed the hold check before it was set cannot slip onto the old connection between the check and the recycle. A writer that reaches the lock after a handoff recycled its connection queues for the replacement instead of writing to a dead connection; only for the handoff (an ordinary closure is still reported as one, #3167), never for a message bypassing the backlog, and never for internal traffic. - Held work goes through QueueOrFailMessage's queue branch, so QUIT is never queued and the duplication is gone; QUIT and internal traffic are not held at all, in the batch path as well as the single-message paths. - Work is held only while the connection is still live; once it has dropped, the ordinary disconnected rules and the backlog policy apply. - A held enqueue that lands just after the hold is released kicks the backlog processor itself rather than waiting for the heartbeat. - The orphaned summary is back on CheckBacklogForTimeouts, and docs/ServerMaintenanceEvent.md describes the hold. New test: slow commands in flight do not hold the drain past the cap, and traffic resumes on the replacement.
…e lock, with a bounded wait; requeue raced writers at the head Second round of review follow-ups: - Each bridge records the connection current when the hold begins and only ever retires that instance (TryRetireQuiesceTargetAsync). A replacement established meanwhile is never touched however often the drain loop retries, which the previous pass could do while the other bridge was still draining; and if the target went away by itself there is nothing to retire. - Every retirement happens under the write lock: the forced pass at the cap as well. The lock wait is bounded by a cancellation token the drain owns (the cap plus a second of grace) rather than the mutex timeout; if the lock cannot be had even then, the connection is left to the server's close rather than recycled around a writer. - A writer that lost its connection to the handoff while waiting for the lock is requeued as held work (never failed under FailFast) at the head of the backlog, via a front queue every backlog operation drains first, so it keeps its place ahead of what the hold queued after it. - The cap is half the relaxed timeout (five seconds by default) bounded by the window, and timeouts are relaxed for the hold, so a blocking command with a timeout up to the cap completes and nothing held is timed out by the hold itself. A fixed second failed commands the server's close would have let finish; the whole window held everyone else's work for one of them. - Each bridge is released in its own try/finally and the processor wake is guarded, so a bridge disposed mid-handoff cannot leave the other quiesced. - The two unused in-flight predicates are gone; the requeue check is one helper at the three lock sites. Tests: a slow command that fits inside the cap is answered and the recycle follows it; a longer one is failed at the cap and traffic resumes on the replacement; the steady-load test is unchanged.
…ly at the next hold, per-bridge release, cap floor Third round of review follow-ups: - BacklogRequeueFront takes lock(_backlog) around the head insert. The timeout sweep peeks, then dequeues and compares under that lock; an unlocked insert between the two handed it a different message, which it treats as a threading bug, and the dequeued message was dropped uncompleted. - _recycledForHandoff is cleared when the next hold begins, not when the replacement establishes: the write lock is not fair, so a writer parked on it can be bypassed by the backlog processor long enough for the replacement to come up, and clearing then made it write to the disposed connection and tear down the healthy replacement. - DrainCap has a one-second floor, so a relaxed timeout of zero (relaxation disabled) does not force the recycle with commands still in flight. - Each bridge is released as its own retirement resolves, not when both have; the interactive bridge usually drains in a round trip and must not hold work while the subscription bridge takes longer. EndQuiesce is idempotent and kicks the processor unconditionally on the transition, closing the store-load race with a concurrent enqueuer. - The drain works on snapshots of the bridge fields, and the mutex take and release tolerate disposal mid-handoff. - A batch carrying a QUIT is not held at all, so the hold decision and the per-message action agree. - The lockDeadline comment no longer claims the mutex timeout plays no part. New test: the steady-load handoff with maintRelaxedTimeout=0.
…work's timeout, budget from what is left Fourth round of review follow-ups: - A connection failure during the hold no longer abandons the held work under a FailFast backlog policy: that backlog is work diverted off a live connection, which would have been written and answered, so it is left for the replacement to flush. Previously only the handoff's own recycle was exempt. - The hold imposes its own floor on the effective timeout for its duration plus the reconnect (the cap, the lock grace and the connect timeout), independent of relaxation: with maintRelaxedTimeout=0 and a sub-second configured timeout, the one-second drain floor would otherwise have timed out exactly the work the hold protects. - The front requeue roots the multiplexer on the empty-to-non-empty transition, as the ordinary enqueue does. - The drain budget is what is left of the window after the decision has spent its share polling DNS, not the window minus jitter; with less than the floor left there is no hold, and the connections are replaced at once (still under the lock) so the replacement gets what remains. - The handoff marker is a short ring of the last four retired connections, never cleared, so a writer starved across back-to-back MOVINGs still recognises its connection. - Abandoning the backlog dequeues under the dequeuers' lock; a quiesced backlog pass returns before contending for the write lock; the processor start's wake tolerates disposal. - One hold predicate, shared by the single-message gates and the batch gate, which makes one pass over the batch. - The docs and the DrainCap remarks now agree with the clamp: the window bounds the hold because the server closes the socket when it ends.
…window, exact retired marker Fifth round of review follow-ups: - The hold cover spans the rest of the window plus one connect attempt, not one connect attempt alone: a MoveTo pins reconnection to the named successor until the window ends, and a successor that does not answer is tried until then. With relaxation disabled, held work could outlive the cover and be swept. - The drain cap is also bounded by half of what is left of the window, so a relaxed timeout of two windows or more cannot turn the hold into a full-window stall that recycles at the deadline with work in flight. - A disposed wake in KickBacklogProcessor still starts the processor, whose disposed-drain is the only thing that fails what is left once the heartbeat has stopped. - The hold is keyed on the quiesce target still being the current connection, not on IsConnected: once the target is gone, by the drain or by itself, new work and the backlog flow to whatever is current. - The retired-for-handoff marker is a flag on the connection itself, exact and holding nothing, in place of the four-slot ring. - The finalizer abandons the backlog without the dequeuers' lock; everything else keeps taking it. - The steady-load tests' resumption poll is wider, with a note that the in-process tunnel never dials the successor.
…, cover ends on establishment, teardown outside the lock Sixth round of review follow-ups: - A handoff-pending marker, set alongside the hold cover and cleared when a connection is fully established (or by the cover's deadline), keeps a FailFast policy from abandoning held work when a connect attempt on the way to the replacement fails. The hold alone covered the hold, not the reconnect it exists for. - Each bridge is retired by its own loop with its own lock deadline, run side by side, so a writer wedged on one bridge's lock cannot spend the other's time and leave its idle connection to the server's close. - The hold cover is cleared when the interactive connection establishes; the deadline is only the bound for the case where it never does. A short configured timeout with relaxation off regains hang detection the moment the replacement is up, instead of after the whole window. - Under the write lock the retire now only marks the connection; the report and dispose follow the release, so event handlers and socket shutdown no longer stall every writer and the replacement's handshake. The retired flag on the connection is what keeps writers and the backlog processor off it in the gap. - The front queue's comments say what it orders and what it does not. - The RetiredForHandoff doc sits on its own member again. - The docs say the floor wins over the half-window bound under two seconds. - No kick helper: EndQuiesce and the two held-enqueue paths pair a full fence with their flag/queue accesses, so neither side misses the other and no processor is started for an empty backlog. - The cap tests assert against jitter-aware bounds.
… a retired connection, FailFast exemption scoped, recycle path retires what is current Seventh round of review follow-ups: - The hold cover is cleared when both the interactive and the subscription connection are established (or the subscription is not required), not when the interactive one alone is: held subscribes must not be swept while the subscription bridge is still reconnecting. - A writer that reaches the lock with a retired snapshot never writes to it, internal and backlog-bypassing messages included: those fail as having no connection instead of faulting the write mid-teardown and tearing down the bridge; a caller's message is requeued as before. - The handoff-pending exemption from a FailFast abandon applies only to the handoff's own failures: the retired connection, or a replacement attempt that never got as far as a working connection. An established replacement that then fails is an ordinary failure. The marker is cleared when the handoff gives up on a bridge or finds nothing of its own to replace. - On the no-successor paths, a target that went away by itself during the hold is replaced by whatever is current, since the ordinary reconnect may have landed back on the node being retired; with a successor pinned the reconnect owns what happens next, as before. - The handoff tests gate on the slow command being observed in flight rather than on a fixed delay, and their time bounds carry more slack. - The docs say "approximately in the order they were issued". - One tick-sentinel helper (ServerEndPoint.NudgeFromZeroTicks) and one scan over both backlog queues.
…cover Eighth round of review follow-ups. The protection a handoff raises - the per-bridge shield against a FailFast policy abandoning held work, and the server's floor on the effective timeout - now shares a single lifecycle: raised in BeginQuiesce for a stated span, lowered only when a replacement connection fully establishes on that bridge, when the handoff gives up on it, or when the span runs out. The server's cover follows the bridges: it stands down when no bridge still has its protection up. - A target that went away by itself during the hold no longer stands the shield down: a replacement reconnect is in flight, and its first failure would otherwise have abandoned the held backlog. - The shield covers failures of the retired connection and of any connection that never completed its handshake, whatever the failure type; a reset mid-handshake reports as a socket failure like any other. A replacement that was fully established and then failed is ordinary. - The cover is no longer cleared by the old subscription connection still being live while the interactive replacement comes up; it waits for every bridge that still has a retire or reconnect pending. - When nothing recycles and nothing re-establishes, the cover stands down with the shield rather than waiting for its own deadline. - The orphaned sentinel doc and the doubled summary are gone; the docs say that under a second of window there is no hold at all.
…dge, leave a replacement that landed elsewhere alone Ninth round of review follow-ups: - The second backlog queue is gone. A writer that lost its connection to the handoff is appended to the backlog like the held work; the head insert bought only a best effort at ordering, at the price of a head-mutation discipline across every backlog operation. The abandon path keeps the dequeuers' lock, which closes a pre-existing race with the timeout sweep. - A bridge with no connection when the hold began lowers its protection at once, so a dead subscription bridge cannot keep the timeout floor up for the whole cover span while the interactive replacement is serving. - On the no-successor paths, a replacement that demonstrably landed on a different node than the one being retired is left alone; only one that landed back on the retiring node is retired. - The hold case speaks through the protection alone: the connection the hold is draining counts as the handoff's own, so the blanket "while quiescing" clause on the abandon path is gone and an established replacement failing during the hold is an ordinary failure, as the doc says. - The cover comments and docs say the cover stands down when every bridge's replacement is up, which is what the code does. - The drain-cap test zeroes the server latency as soon as MOVING is sent, so the replacement's handshake is not queued behind the slow replies.
… path, give-up keeps the shield for diverted work Tenth round of review follow-ups: - The landed-on address is read inside a try/catch: a socket still connecting throws on RemoteEndPoint, and a concurrent teardown throws too; either counts as unknown, and unknown is retired. Previously the throw escaped, dropped the shield and abandoned the handoff in the very race the branch exists for. - A hold that began with no connection (the old one dropped during jitter or the DNS poll) now goes through the same target-gone logic on the no-successor paths, so a reconnect that landed back on the retiring node is retired instead of surviving to the window-end close. With no current connection at all there is nothing to retire or protect, and the protection lowers. - Giving up on a bridge (the lock not obtained, or an exception) stops holding new work but keeps the protection for the work already diverted: it was queued off a live connection whatever the policy, on the promise of a replacement, and the server's close plus the reconnect after it are that replacement arriving late. The protection lowers on establishment or expiry, as always. - The bridge-disposed path lowers the protection: no replacement is coming. - The fence-then-wake idiom lives in one method opposite EndQuiesce. - The drain-cap test uses one blocking command, and zeroes the server latency only once the recycle is observed: the server reads its latency when it executes a request, and zeroing it earlier raced the command the test needs to stay slow.
… not on where the handoff's flow stands Eleventh round of review follow-ups: - A connection records that it was ever the one a hold diverted work off (WasHandoffTarget). A failure is the handoff's own if the connection has that mark or never completed its handshake, whatever the handoff's flow has done since: retired by the drain, closed by the server at window end, dropped by itself, or left un-retired after a give-up. The three remaining FailFast regressions were each a flow-based clear leaking on one path; the fact-based test closes the class. - A drain pass that finds no current connection no longer lowers the protection: work may have been diverted before the target went, and the next failed reconnect must not abandon it. Only a hold that never had a target, and so diverted nothing, lowers it. - The target itself completing a handshake it was in the middle of when the hold began is not mistaken for the replacement, so the protection and the timeout cover stay up while the hold is active. - An expired protection deadline and an expired cover are cleared on observation, as GetRelaxedRemaining does, so a tick count wrapping ~24.8 days later cannot re-arm them. - The docs scope "whatever the backlogPolicy" to the drain, and say what a FailFast policy does to new commands between the replacement and its handshake. - The in-flight check is done lock-free first; the write lock is taken only for a pass that is likely to retire. The one-line requeue alias is inlined.
…t; one tick-deadline type Twelfth round of review follow-ups: - A connection is a bona fide replacement when it is not the one the hold began on and, unless reconnection is pinned to a named successor, has demonstrably landed somewhere other than the node being retired - by address and port, since Redis Enterprise rebinds to other ports on the same host. Only such a connection lowers the protection when it establishes, ends the hold on new work and receives the held backlog. A reconnect that landed back on the retiring node (no successor, DNS still stale) is neither target nor replacement: new work is held off it, the backlog does not flush to it, and the drain retires it in its turn. - The protection deadline is monotonic: a hold beginning while an earlier handoff's protection is up extends it and remembers that older diverted work exists, so a hold with no target of its own does not clear it. - TickDeadline holds the sentinel, nudge, monotonic-extend and clear-on- expiry invariants once; the relaxed window, the hold cover and the per-bridge protection all use it. - The three handoff flags on the connection are volatile. - A requeued message keeps its write time; it only forgets the connection it was headed for.
…ement; a hold that diverted nothing protects nothing Thirteenth round of review follow-ups: - The bona fide test gains the input it was missing: once the retire has resolved, whatever connection comes up next is the replacement wherever it landed, including the same address and port - the common case after an in-place proxy restart or with stale DNS - so the protection stands down on its establishment instead of outliving its deadline. Before the retire resolves, the landed-elsewhere test still tells a replacement from a reconnect that landed back on the retiring node. The answer is latched on the connection once true, so the hot path does not re-read the socket. - A hold records whether it diverted anything (or inherited an earlier hold's diverted work); a drain pass that finds no connection lowers the protection only if nothing was diverted, whether or not the hold had a target. - TickDeadline takes TimeSpans and clamps them, and the announced MOVING window is capped at an hour on entry, so an absurd window cannot overflow a delay, a cancellation source or a deadline into the past. - The handoff's relaxation respects maintRelaxedWindowMax, as the notification path does. - The retire's report and dispose run under try/finally: a throwing user handler neither leaks the connection nor makes the retire look undone. - The hold cover extends rather than overwrites, per its documented lifecycle. - QueueOrFailMessage is back to its original shape; the gates queue held work directly, a batch with one fence for all its members.
…ded-on test applies after the retire, a lockless retire as the last resort Fourteenth round of review follow-ups: - A connection that an earlier handoff latched as its replacement and that a later handoff now retires is a target, not a replacement: marking it a target clears the latch, and the target check comes first. - With no successor pinned, the landed-on comparison applies after the retire as well as before it: a reconnect that landed back on the node being retired is not a replacement at any point. The hold keeps new work and the held backlog off it until a bona fide replacement is current or the protection expires, instead of flushing onto a socket the server is about to close; and the FailFast shield now covers any failure of a connection that is not a known, established replacement, which includes that one. The latch is taken only once a connection is established, so a still-connecting successor that fails still counts as the handoff's own. - The hold cover is bounded by maintRelaxedWindowMax, as the relaxation is, while always spanning the drain itself and one connect attempt. - When the write lock cannot be had for the whole of the drain's deadline, the target is retired without it as a last resort - what every recycle did before handoffs existed - rather than left to the server's window-end close, which would fail everything in flight to protect at most one wedged writer's message. - The guarded remote-endpoint read lives on PhysicalConnection; the inherited- protection flag is a local.
…less; the cover applies to held commands only Fifteenth round of review follow-ups: - Behind a Tunnel, or when the configured endpoint is itself a fixed address in front of the cluster, every connection reports the intermediary's address, so comparing it with the retiring one would make the genuine replacement look like a return to the retiring node forever. In those topologies the discriminator is temporal: a connection that is not the target and came up after the retire resolved is the replacement. - The hold's timeout cover applies to the commands the hold diverted (and to those it requeued), not to every command on the server: they carry a mark, and the backlog sweep, the in-flight sweep and the sync wait consult an overload that adds the cover for them alone. An application that has turned relaxation off keeps its short timeouts for everything else; a held command at the head of a sweep can delay, never lose, the one behind it. The cover still spans the drain and one connect attempt, since that is what the held commands need. - The diverted flag is set before the enqueue, so a drain pass cannot see a held message with the flag still off. - The socket's remote endpoint is cached on the connection once read. - The handoff target's expiry is a TickDeadline too; it was the last hand-rolled tick deadline. - The fence-pairing comments name WakeIfHoldReleased, which every held enqueue goes through, as the single enqueue-side site. - The steady-load test cancels its workers on any exit.
…over as a property of held commands, trust validated at use Sixteenth round of review follow-ups. The protection state machine's rules are now stated once at the top of the classification section, and the code is checked against them: - Classification has three outcomes: replacement, doomed, unknown. Unknown (an address not yet readable; or, behind a tunnel, time not yet telling) defers to the next pass and is never retired, never branded a target, and never trusted. - A successor pin is trusted only while it is still in force: the first bridge's replacement clears the pin, after which the other bridge's retry resolves the endpoint again and is classified by where it landed. - Behind a tunnel or a fixed front address, the temporal fallback also accepts "the drain has let go": after EndQuiesce nothing else will arrive, so what establishes is the replacement, rather than holding until the protection expires. - The timeout cover is a property of each held command for as long as it lives, no longer a window on the server, so it cannot be withdrawn between the replacement establishing and the held work being written to it; the FailFast shield likewise stands down only once the last held message has been flushed to the replacement. - A message that parks in a backlog that is holding (it passed the gate just before the hold rose) is held too, with the same cover. - The sync wait polls in one-second slices, so a relaxation or a cover that begins after a caller parked is noticed even when nothing pulses. - One teardown routine for the locked and lockless retire paths. The in-process test server can delay replies to one command only, so a test can slow the workload without pushing the replacement's own handshake past a short timeout; a new steady-load variant runs with relaxation off and a timeout shorter than a held command's wait.
…ection flags Rounds 13 to 17 of review each found a case where two of the handoff's flags disagreed. This replaces them with one object per handoff on each bridge, carrying a phase advanced in one direction by compare-exchange (Holding, AwaitingReplacement, Flushing, Done) and the facts fixed when it began (target, retiring address, whether an address identifies a node), plus one monotonic deadline shared by successive handoffs. Every handoff question - hold new work, flush the backlog, is this failure the handoff's own, is this enqueue covered - is answered from the phase. With that in place: - the hold cover is stamped on each held message as the handoff deadline (an absolute tick), so it cannot be withdrawn after the replacement establishes and does not ratchet across handoffs; the server-level cover and the stand-down-after-flush flag go; - everything entering the backlog during a handoff is held, in BacklogEnqueue alone, which also keeps the timeout sweeps' head-of-line rule sound without skipping; - a connection records that it was made under the successor pin, which is kept until the window expires rather than cleared when the first bridge establishes, so the second bridge's reconnect is not sent back to stale DNS; - a socket address identifies a node only when the handoff has itself seen a second address (a named successor, or DNS moving) and no tunnel is in the way; otherwise only time tells; - classification no longer latches a verdict on ordinary establishment: it is only consulted while a handoff is in progress; - sync waiters re-read their budget in slices only where maintenance notifications can change it; - TickDeadline keeps a passed deadline as the record of when it passed, replacing the hand-rolled relaxed-ended mirror, and owns the single nudge-from-zero helper.
…drain's bounds Round 18. The phase machine's transitions were driven only by the drain loop and by establishment, so one missed instant could wedge a phase until the deadline. Every heartbeat now re-judges the current connection against the handoff in progress: a bona fide replacement resolves the hold, and a reconnect that landed back on the node being retired is adopted (once, by compare-exchange on the target) and retired in its turn, which also covers the drain loop having ended on a transient null and an address that was unreadable at establishment. The retire is idempotent under the lock. Classification puts the address before the pin, so a connection dialled through a stale pin that still named the retiring node is doomed; the via-pin fact is recorded as the address dialled and matched against the successor of the handoff doing the judging; and a MOVING that names no successor drops any earlier pin. A new hold explicitly finishes an earlier handoff still flushing, so nothing is left mid-phase. A window-closed notification arriving mid-handoff can lengthen the relaxed window but not shorten it below what the drain extended; the drain cap is bounded by the relaxation ceiling it depends on (never below the floor); the head-of-line note on BacklogEnqueue states the requeue exception honestly; and the held message's cover is a TickDeadline, extended through the one ExtendTo that TryExtendBy now uses too.
…he last two wrap hazards Round 19. The handoff deadline moves onto the handoff object: each handoff stands for its own hold plus the rest of its window and one connect attempt, held work is stamped with that, and the deadline is read on every question put to a live handoff, so a passed one is noticed fresh and no stale residue is ever consulted by a later handoff. BeginHold publishes the new handoff before finishing the previous one, so there is no instant with no hold in force. The heartbeat's judgement now also retires the target itself when it is still current after the drain loop is over (the lock never came, or the retire faulted), not only an adopted reconnect; the retire outside the loop is one at a time per handoff and idempotent under the lock, so a stuck attempt is not piled on and a failed one is retried on the next beat. TickDeadline treats a deadline as in force only within one span of now, in both RemainingMilliseconds and ExtendTo, so a wrapped tick count can neither revive a long-passed deadline nor let it win against a fresh extension; the successor pin is read on every server heartbeat so its expiry is noticed while fresh. Sync waiters slice only where the server has ever accepted the opt-in, has a window open, or has a handoff in flight. The steady-load test helper's try body is indented.
…retire and stamp races, slicing only while a window is open Round 20. A successor at the very address being retired (a proxy restarted in place) no longer makes the address identify the node, and a connection dialled through this handoff's own pin is the replacement before the address is consulted - the stale-pin case the address-first order served is covered by matching the pin against the judging handoff's successor and by a no-successor MOVING dropping the pin. The written-queue sweep looks past a held head rather than stopping at it, as the backlog's ordering argument never covered the replacement's queue. BacklogEnqueue stamps from the same handoff that gated the message, and not for internal traffic. Marking a connection retired is a compare-exchange, so a lockless last-resort retire and a locked one tear it down once between them. The processor's in-hand message counts as queued until written, so a flush cannot be declared finished around it. The one-at-a-time guard on a MOVING covers the decision and the drain, not the warning wait, so a second MOVING inside the first window is acted on. Sync waiters slice only while a window is open or a handoff is in flight; an extension is honoured by the re-read after the wait regardless. TickDeadline.cs gets the repository's UTF-8 BOM.
…nd judgement needs evidence Round 21. HandoffAsync releases its one-at-a-time claim exactly once, by whichever path still holds it, so a second handoff's claim is not stomped by the first's finally. BeginHold stamps the work already queued when the hold begins, since the hold delays it as it delays diverted work; BacklogEnqueue re-reads the handoff after the enqueue, so the two sides pair and a concurrent enqueue is stamped by one of them. Sync waiters re-evaluate per wait whether their budget can shrink. The flush's termination check takes the backlog lock, under which the processor moves a message from the queue to its hand. A connection with no positive evidence either way is unknown, never a replacement by default. The drain's polls settle the no-retire cases without the write lock. The relaxation ceiling is stated once, in ClampToRelaxationCeiling.
…he phase, not the drain's claim Round 22. A reconnect that lands back on the retiring node while the held work is flushing puts the hold back in force (Flushing to AwaitingReplacement, before the connection reads as established) and is retired; the heartbeat judges a Flushing handoff too. A tunnel no longer records a successor dial that never happened. The drain's one-at-a-time claim keeps serialising the decision and the drain, but everything that asks whether a handoff exists - the relaxed window's shorten guard, the sync waiters' slicing - now asks the bridges' phases (HandoffLive); and a MOVING arriving while a drain is in flight updates the successor pin instead of being dropped. A diverted message whose hold has gone by the time it is enqueued is ordinary work rather than stamped from a dead handoff; the post-enqueue re-read rides on the total counter's interlocked increment, so a bridge that never saw a handoff pays one null read. A finished handoff lets go of its target. The docs state the cover's floor.
…pin is one generation-checked state Round 23. InheritWriteTime carries the parent's hold cover as well as its write time, so a held transaction's inner commands are not failed by the sweep on the replacement for a clock that includes the hold. An internal member of a held batch is covered with the batch, since the batch is held as a unit and the sweep must not fail one member of it. The pin - target, expiry, and the generation of the MOVING that set it - is kept consistent under the maintenance lock: a handoff's late decision cannot clear or overwrite a pin a later MOVING set, and the heartbeat's expiry check cannot null a pin set a moment ago. BacklogEnqueue loses its dead parameter and its stale contract. The docs and the drain comment say what a sub-second window does with the hold (it begins, for the instant of the replacement, because the retire is built on it). The drain-cap test's bound sits between the mechanism's worst case and the deadline it guards against.
…one; the contract; a two-MOVINGs matrix Round 24, and the focusing note that followed it. The address-identity apparatus goes: the retiring address, the per-bridge comparison, the tunnel and front-address cases, the same-address successor, the pin-dialled fact, the adoption and re-retire of a doomed reconnect. The replacement is the connection that fully establishes once the target is gone, and nothing else; a reconnect back on the retiring node, with no successor named and a stale resolver, is closed by the server at the end of the window and reconnected again, as it always was, with the relaxed window and the held work's cover meanwhile. Eight review rounds of findings were in that code, for that one corner. The pin's generation check is unconditional, a successor-less MOVING mid-drain drops the pin, and the fault-attribution type follows the window it describes. The contract is written into the docs: one handoff at a time per endpoint is fully covered; back-to-back MOVINGs in one window are best effort - nothing lost or abandoned contrary to policy, a bounded wait, resolved later rather than wrong. A six-case test matrix (second MOVING with or without a successor, arriving during the first handoff's decision, drain or flush) asserts exactly that and nothing about the mechanism.
Collaborator
Author
Exit-criteria assessment (head
|
…d finding no handoff A message stamped under a still-live handoff while a later hold begins is re-stamped with the later one's deadline (the stamp only extends; a same-handoff double stamp is a no-op). The last item from the exit assessment.
Collaborator
Author
|
Confirming: |
Collaborator
Author
|
The residual is taken in 8cddb48: the post-enqueue re-stamp no longer depends on the first read having found no handoff, so a message stamped under a still-live handoff while a later hold begins carries the later deadline. The body's follow-ups paragraph is updated accordingly. Verification on that head: Release build clean with analyzers; full net10.0 suite 6885 total, 1 failed (the known
Nothing further planned on this branch; the perf items are listed in the body for a follow-up PR. |
…n cases A per-command latency lifts the throttle from every other command, so the four PING workers hot-looped for ten seconds in the two Drain cases; on a small CI runner that starved timing-sensitive neighbours (three different tests failing once each across two runs). Every reply is slowed instead, and the throttle is restored once the second MOVING is in.
This branch has not been deployed
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
The
MOVINGhandoff drained by waiting for "no caller work" before replacing the connections. Under steady traffic that never happens: there is always a command in flight, so the drain waited out the entire announced window and recycled at the deadline, failing whatever was in flight at that instant and leaving no time to establish the replacement.Measured live on a Redis Enterprise endpoint rebind (
singleproxy policy,maintMovingEndpointType=Auto) at about 450 ops/s:The fix
While a handoff drains a connection, the bridge holds new caller work in the backlog instead of writing it to the socket the server is about to close. The drain then waits only for commands already written, so under load it finishes within a round trip; it is capped (half the relaxed timeout, bounded by what is left of the window and by
maintRelaxedWindowMax, floor one second), after which the connection is replaced regardless. The replacement happens under the bridge's write lock (a writer that reaches the lock late requeues for the replacement), on both bridges, and the held work is written to the replacement once it establishes. Held commands carry a timeout cover for the handoff's span (the hold, the rest of the window, one connect attempt) so the hold cannot time them out, whatever the relaxation setting; ordinary commands keep their configured timeouts. AFailFastbacklog policy does not abandon held work for a failure that is the handoff's own.The contract (also in
docs/ServerMaintenanceEvent.md)MOVINGs inside one window. A second notification while a handoff is in progress updates where the reconnect goes (or drops a successor that may itself be retiring) rather than starting a second drain. Held work is never lost or abandoned contrary to policy, but may wait longer than the single-handoff bound; the hold resolves later rather than wrong.Tests
MovingUnderSteadyLoadRecyclesPromptlyAndFailsNothing(fails onmain: recycle at ~10s), plus variants with relaxation off, short timeouts, a slow command inside the cap, and a blocking command past the cap.MovingTwiceInOneWindowLosesNothing: six cases, secondMOVINGwith or without a successor, arriving during the first handoff's decision, drain or flush; asserts the contract and nothing about the mechanism.FailoverTests.SubscriptionsSurvivePrimarySwitchAsyncis flaky on unmodifiedmainon the same machine).MOVING, maintenance-mode chain (MIGRATING, MIGRATED, MOVING) likewise, 0 failed commands every run.Follow-ups, post-merge
Perf tidy-ups that touch hot paths and deserve their own review: gating the per-enqueue fence further, an O(1) held-and-unwritten counter in place of the flush's locked walk, and finer sync-wait slicing. The reviewer's exit assessment (below) found the head mergeable with these deferred.
Design notes (for the next editor)
Each bridge keeps one
Handoffobject per handoff: a phase advanced one way by compare-exchange (Holdingwhile the target drains,AwaitingReplacementonce it is gone,Flushingwhile held work is written,Done), the target, and its own deadline. Whether new work is held, whether the backlog may flush, whether a failure is the handoff's own and whether an enqueue is covered are all answered from the phase. The connection records what it is (the target, retired, established); the phase says only where the flow stands.A held command is stamped with its handoff's deadline as an absolute tick at the one place messages enter the backlog, including work already queued when the hold begins and the inner commands of a held transaction or batch. The backlog and in-flight sweeps honour the cover; the in-flight sweep looks past a held head.
The retire marks the connection under the write lock (a compare-exchange, so the lockless last resort and a locked retire tear it down once between them); the teardown follows the release. Every bridge heartbeat re-judges the current connection while a handoff is live, so no phase can be wedged by a missed instant: the target still current after the drain is retired again, one attempt at a time.
The drain's one-at-a-time claim per endpoint covers the decision and the drain and is released exactly once. "A handoff exists" (for the relaxed window's shorten guard and the sync waiters' slicing) is the bridges' phases. The successor pin lasts the window, carries the generation of the
MOVINGthat set it, and lives under the maintenance lock with its expiry.Every tick-count deadline goes through
TickDeadline: zero is unset, extension is monotonic, a deadline is in force only within one span of now (so a wrapped tick count can neither revive a stale one nor beat a fresh extension), and a passed deadline is kept as the record of when it passed. The relaxation ceiling policy is stated once.Twenty-four rounds of self-review were addressed in the commits on this branch; actioned comments are removed to keep the PR readable.