Recover from a 421 on the evidence rather than on the cache's age - #154
Open
mpretty-cyro wants to merge 1 commit into
Open
Recover from a 421 on the evidence rather than on the cache's age#154mpretty-cyro wants to merge 1 commit into
mpretty-cyro wants to merge 1 commit into
Conversation
A 421 tells us, authoritatively, that the swarm we resolved for an account is wrong. `_handle_421_retry` answered it with `refresh_if_needed`, which decides on `cache_expiration` (2h) and so declines on any cache younger than that, and then re-read the same `_swarm_cache` entry - so the retry went to another node in the list that had just rejected us, and every later request for that account did the same until the cache aged out. Evicting the swarm cache entry alone cannot fix it: the entry is only ever a memo of `swarm::get_swarm(pubkey, _all_swarms)`, so it recomputes the identical answer. What the rejection disproves is the pool snapshot the swarms were generated from, and only a refresh replaces that. `invalidate_swarm` refreshes on that evidence, with a backoff that doubles each time another rejection arrives after a refresh we already ran for one, so a node rejecting everything costs a handful of refreshes over a couple of hours rather than one every minute for as long as it keeps it up.
mpretty-cyro
marked this pull request as ready for review
September 11, 2026 03:05
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 defect
A 421 tells us authoritatively that the swarm we resolved for an account is wrong.
_handle_421_retryanswered it withrefresh_if_needed, which decides oncache_expiration(2h)and so declines for any cache younger than that, then re-read the same
_swarm_cacheentry. The retrytherefore went to another node in the list that had just rejected us,
redirect_retry_count(1) wasexhausted, and every later request for that account did the same until the cache aged out.
User-visible result: a client can be unable to interact with a swarm for up to two hours, with no
recovery except waiting out
cache_expirationor a fullclear_cache(). The log line even said it —"refreshing swarm if stale" — staleness measured by the clock while we were holding direct proof.
Why evicting the swarm cache entry doesn't fix it
Worth stating, because it's the obvious-looking fix and it is a no-op:
_swarm_cacheis only ever written withswarm::get_swarm(swarm_pubkey, _all_swarms)— a purefunction of
_all_swarms._all_swarmsisswarm::generate_swarms(_snode_cache), grouping nodes by theswarm_ideach nodecarried in the pool fetch.
_all_swarmsreplace_swarm_cachewholesale in the same breath.So the swarm cache can never disagree with the pool; erasing an entry recomputes the identical answer.
What a 421 disproves is the pool snapshot, and only a refresh replaces that.
The 421 body does carry the correct swarm, and is deliberately not used: it would take one node's
unauthenticated word for swarm membership — the node that just rejected us — where a pool refresh
establishes it from an intersection of
cache_num_nodes_to_use_for_refresh(3) nodes. It would alsobreak the memo relationship above.
The change
SnodePool::invalidate_swarm()— evidence-driven, alongside the existing age-drivenrefresh_if_needed._handle_421_retrycalls it instead.What stops a 421 storm becoming a refresh storm. A rejection arriving after we already refreshed
on one is the refresh telling us it didn't help, so each successive one waits twice as long, capped at
cache_expiration: immediate, 1m, 2m, 4m, 8m, 16m, 32m — about 7 refreshes in the first hour, thenflattening toward today's behaviour. Quiet for twice the interval being held resets to the base delay.
The backoff is global rather than per-swarm on purpose: a refresh replaces the pool every swarm is
derived from, so a rejection for swarm B right after a refresh prompted by swarm A is equally
unhelpful to act on. A separate pool-age floor stops us refreshing a pool something else just fetched.
Worst-case outage goes from 2h to ~1 min.
Tests
New
[network][invalidate_swarm]case. It cannot fail against the pre-fix tree (it calls a functionthat doesn't exist there), so the old behaviour was reproduced by mutation instead — reducing
invalidate_swarmtorefresh_if_needed({}, cb), exactly what_handle_421_retryused to do, failsREQUIRE(debug_refresh_in_progress()). Deleting the backoff term failsCHECK_FALSE(debug_refresh_in_progress()).Full suite green (135 cases).
utils/format.sh verifyclean.Notes for review
Request::swarm_pubkeyis populated only by the client via the C API, and a request without itis refused a redirect outright — so this is inert for any caller that leaves it null. iOS sets it
for
.snodedestinations; worth confirming as the Session 2.0 client wires up its request paths.client, which carries the identical defect.