Repository navigation
Recover a replica lane whose peer's hello was lost - #30
Merged
Merged
Conversation
When a lane's lease expired and the peer's replica_hello for the new probe epoch was dropped by a busy link, the lane never came back. The peer's own route was ready, so it answered every repeated probe with a constant-size ACK and no hello; this side kept the probe pending because the ACK could not certify a route it had never installed, and nothing asked for the hello again: heartbeats and heads for a lane without a view are ignored while the peer's authority is still known. Two production nodes stayed in that state for hours, each unable to reach org trackers on the other. A ready ACK that finds no route now requests the peer's authority. Shard zero sends a replica_hello_request, which the peer answers with a hello at most once per retry interval; a data lane rebuilds its view from the shared authority or escalates to shard zero. A hello refused by a busy link is no longer recorded as sent, so the next request is not silenced for a retry interval. The regression test reproduces the wedge on a two-shard group: one lane's lease expires while the node's authority stays known, the peer records the probe epoch while its hello and ACK are dropped, and repeated probes must still restore the route.
The install-once test injected ACKs with probe epoch 0. A lane whose hello outran shard zero's exact authority re-probes at once, so its current epoch can already be 1, and an ACK for epoch 0 is discarded as stale with no hello to count. Read each lane's epoch instead; the race shows up more often now that a ready ACK without a route requests the peer's authority.
Shard zero recovers a lost hello by asking the peer for it; a data lane recovers by rebuilding its view from the authority shard zero still holds and taking the route from the peer's next heartbeat. Exercise that second path with the same dropped-hello wedge on shard one.
mix format --check-formatted failed on this file alone.
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.
Problem
A replica lane whose lease expired could stay unrouted for the life of the dist connection.
In production (sprites-api, group 0.3.0, 8 shards) a transient stall between two nodes dropped heartbeats long enough for the 15 s peer lease to expire on three shards of one node. Lease expiry bumped the probe epoch and sent
peer_connect; the peer answered the new epoch with itsreplica_hello, and that hello was dropped by the same busy link (every replica message issend_nosuspend+:noconnect, which the docs describe as drop-and-repair). From then on nothing could repair the lane::duplicatewith its own route ready, so it answered with a constant-size ACK and no hello;remote_shards[peer]already matches the ACKing pid, which the lost hello never installed; its only other reaction to an ACK was to send its own hello, which the responder accepted as a lease touch;Both nodes logged
[Group ...] peer_connect from ...once a second for hours and every org tracker on the affected shards was{:unreachable, owner}between them.Fix
peer_connect_ack: a ready ACK that finds no matching route callsrequest_replica_authority/2. Shard zero sends areplica_hello_request, which the responder answers with a hello at most once per retry interval (the request token is stable); a data lane rebuilds its view from the shared authority or escalates to shard zero, and the peer's next heartbeat reinstalls the route.maybe_send_discovery_hello: a hello refused by a busy link (:busy) is no longer recorded indiscovery_hello_last_sent, so the next request is not silenced for a retry interval.Duplicate probes still get constant-size ACKs only; the catalog is only re-sent on request.
Tests
Two regression tests in
test/replica_ack_test.exsreproduce the production wedge on a dedicated two-shard group so that expiring one lane retains the peer's authority (with one shard the node is retired and re-installed from scratch, which masks the bug):a ready ACK without a route asks the receiver for its hello(shard 0): the lane's lease expires with the probe withheld; the receiver processes the probe epoch while its hello and ACK are dropped (the source shard's name is unregistered during delivery); repeated probes, answered by ACKs alone, must still restore the route. Fails on main, passes with the fix.a ready ACK without a route rebuilds a data lane from the shared authority(shard 1): same wedge on a data lane, whose repair goes through the local view rebuild and the peer's heartbeat. Fails on main, passes with the fix.full and incremental authority install once through shard zeroinjected ACKs with a hardcoded probe epoch 0; a lane whose hello outran shard zero's exact authority re-probes at once and can already be at epoch 1, and the fix makes that discovery race more likely. The test now reads each lane's current epoch (3/8 failures before, 10/10 passes after).Full
mix test.exunit: 348 passed, 1 skipped.Follow-ups (not in this PR)
ReplicaAck.tlamodels a fair successful hello in its healing step and so cannot express this wedge; a mutation case for "ready ACK, route missing, hello lost" would let the formal gate catch regressions.:busyaccounting change has no dedicated test.busy_dist_portmonitoring on nodes usingGroup.Transport.DistErl, since a busy dist port silently drops heartbeats and hellos.