fix(router): self-heal failed-path exclusion for dead-edge route setup (#22) - #4063
Closed
0pcom wants to merge 1 commit into
Closed
fix(router): self-heal failed-path exclusion for dead-edge route setup (#22)#40630pcom wants to merge 1 commit into
0pcom wants to merge 1 commit into
Conversation
When a route-group setup handshake fails to complete within handshakeAwaitTimeout (even with skycoin#4057 retransmit — e.g. the path funnels through a dead/non-forwarding intermediary), hold that route's intermediate hops on a per-destination suspect list for a bounded TTL. The next dial (an app reconnect after a wedge) excludes the suspect hops up front via DialRoutes' ExcludeIntermediatePKs, so it picks a DIFFERENT route instead of re-hammering the dead path and wedging the app in "starting". Self-healing: the suspect entry expires after the TTL (never a permanent blacklist), so a recovered hop is reused. Configurable via routing FailedHopExclusionTTL (0 = default; feature on). Bounded grace: a single transient failure is handled by skycoin#4057 retransmit; only a full-window handshake failure marks the path suspect. New: pkg/router/suspect_hops.go (+ test). Addresses the dead-edge wedge observed live where all candidate routes shared one dead second-to-last hop.
0pcom
added a commit
that referenced
this pull request
Aug 21, 2026
… fix) (#4078) Route-group setup previously walked candidate routes SEQUENTIALLY: the policy picked ONE route, saveRouteGroupRules set it up with one handshake plus the #4057 retransmit over handshakeAwaitTimeout (~10s), and only on failure did a bounded retry loop query a fresh route and try the next candidate. Each dead / non-forwarding candidate therefore cost up to the full ~10s handshake await, and a run of bad candidates burned the ~90s dial ceiling -> "Route group setup canceled ... context deadline exceeded" and the app wedged in "starting" (observed live: skysocks-client -> Frankfurt at min_hops>=2). DialRoutes now sets up the top-K candidates CONCURRENTLY and the FIRST to complete its reciprocal handshake WINS and becomes the working base (the dial returns on it immediately; mux legs grow on top later and never unseat it). Losing / failed candidates are canceled and torn down. This collapses establishment from "up to N x handshakeAwaitTimeout" to ~one RTT and makes a dead-hop candidate lose the race instead of blocking, so the app converges to a working route (single multihop at min_hops>=2, or direct at min_hops==1) instead of wedging. min-hops-agnostic. Data-model note: incoming handshake/data packets demux to a route group by RouteDescriptor, and every candidate of one dial shares one descriptor, so two reciprocal handshakes cannot run concurrently on it. The race is thus two-phase: phase 1 reserves route IDs across all candidates CONCURRENTLY (where an unreachable intermediate fails id_reservation and loses cheaply -- the dominant live failure mode); phase 2 completes the handshake on reserved candidates fastest-reservation-first, first to complete wins. Because candidates are pre-reserved, moving to the next on a handshake failure costs no re-fetch and no re-reservation, unlike the old loop. Config: routing.parallel_route_setup (Config.ParallelRouteSetup, per-dial DialOptions.ParallelRouteSetup override). Default a small N (3); 1 restores the strictly-sequential behavior for a clean rollback. Concurrency is bounded (maxParallelRouteSetup) and by the number of candidates the finder returns. Loser penalty: a candidate that loses or fails setup has its intermediate hops marked suspect for a short TTL (suspectHopCache) so the NEXT dial front-loads known-good hops. Armed on EVERY failure path -- reservation failure, handshake timeout, AND ctx-deadline / setup-canceled -- which is the arming gap the companion suspect-hop work (#4063) alone did not cover live. Composition with #4063: the router holds a single `suspects` field consulted only in candidate ranking and armed only at the race's onLoser / DialRoutes failure paths; to reconcile, point those call sites at #4063's per-destination cache and drop this global one. Tests: winner selection, handshake-failure fall-through, all-fail, single- candidate sequential-equivalent, ctx-cancel, suspect cache TTL/arming, K resolution/clamping, and candidate ranking (suspect deprioritization, hard-exclude, disjoint preference). Race-detector clean.
Collaborator
Author
|
Superseded by #4078 (parallel candidate route-group setup), which landed the suspect-hop self-heal cache (newSuspectHopCache/suspectHopCache in parallel_route_config.go) as part of the steady-connection fix. This PR's parallel suspect_hops.go + FailedHopExclusionTTL implementation is the older competing version and no longer applies cleanly. Closing. |
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.
Bounded-grace + self-heal failed-path exclusion (task #22). When a route-group setup handshake fails the full handshakeAwaitTimeout window (even with #4057 retransmit — e.g. all candidates funnel through a dead intermediary, observed live: skysocks→Frankfurt wedged 60s+, every candidate shared dead hop 038e009f, fresh route-find recovered in ~15s), the failed route's intermediate PKs are held on a per-destination TTL suspect list; the next dial excludes them via DialRoutes ExcludeIntermediatePKs → picks a different route instead of re-hammering the dead path. Self-heals on TTL expiry (no permanent blacklist); configurable via routing FailedHopExclusionTTL (0=default, on). New pkg/router/suspect_hops.go (+test). go build/vet/test ./pkg/router/ pass under -race (targeted, to avoid concurrent-golangci contention). NEEDS local-visor validation before merge (core routing); NOT auto-merged.