perf(reach): bound concurrent cold-cache scans across distinct nodes (#341)

[adminopenclaw8-sketch]

Relates to #341

Plan

1. Evaluate upstream's non-blocking two-build semaphore + 429/Retry-After against our own read pool and workloads, and record the decision in code.
2. Write the tests first (distinct keys, identical keys, cancellation, errors, saturation/recovery, warm-hit bypass, shipped defaults), confirm them red, then implement.
3. Add a per-server counting semaphore for cold Reach builds, taken inside the singleflight slot after the cache recheck and released with defer.
4. Shed with 429 + Retry-After when still saturated at the end of a bounded queue wait; cache nothing for a shed request.
5. Prove on an offline fixture that /api/nodes and /api/packets stay responsive while Reach is saturated, A/B against master.
6. Document the new status code in docs/api-spec.md.

What changes

cmd/server/node_reach.go already collapses a thundering herd on one cache key with singleflight, but distinct pubkey|days|gen keys are independent: N cold keys arriving together run N windowed observation scans at once. The server read pool is 4 connections (OpenDB), so a handful of concurrent cold Reach scans can hold every connection and leave unrelated endpoints queueing behind them.

New cmd/server/node_reach_cold_limit.go adds reachColdLimiter, a counting semaphore with a bounded queue wait, living on reachState (zero value ready to use, so every *Server — tests included — gets it without a constructor):

• Cap 2 cold builds at a time, so at least two of the four read connections stay available to everything else.
• 3s queue wait, then 429 + Retry-After: 2 with {"error":...,"retryAfter":2}. Nothing is cached for a shed request and singleflight does not cache errors, so the next request recomputes normally.
• Acquired inside the singleflight slot, after the in-flight cache recheck — so it is taken only for genuinely cold work, once per coalesced herd rather than once per waiter.
• Released with defer, which covers success, compute error, marshal error, cancellation and panic alike.

Evaluation of the upstream design

Upstream (fork PR 2127, deliberately not linked) refuses the third concurrent build instantly. Assessment against our deployment:

• The cap of 2 fits our pool as is and is kept: two cold scans leave ≥2 of 4 read connections for other endpoints, which is exactly what the issue asks for.
• Refusing instantly does not fit. A cold report normally completes in well under a second, our Reach page is one request per view, and app.js api() only auto-retries 503 + Retry-After — never 429. An instant refusal would therefore fail a user's page while the database is under no real pressure.
• We queue for a bounded wait first. Queue time costs a parked HTTP goroutine, not a database connection, so pool protection is identical while a transient burst is absorbed instead of shed. Load shedding still happens — it just requires sustained saturation. Measured below: a 6-key cold burst is fully served, a 48-key burst is shed down to the cap.
• Retry-After: 2 rather than the 5 used by the index-loading 503, because a slot frees as soon as one in-flight scan finishes.

This is a per-endpoint cap, not global database admission control: it bounds Reach's own cold work and nothing else. Any broader policy belongs with the pool-pressure evaluation in #221, which covers a different endpoint and is explicitly not touched here.

Preserved

Warm hits never reach the limiter (the handler's cache fast path is before the singleflight call). Same-key coalescing is unchanged. Visibility filtering (visibleReach), rank snapshots (applyReachRank), the blacklist-generation cache key/purge and the response body are untouched; a shed request leaves the cache exactly as it was, and the recovered response is byte-identical to the unsaturated one (asserted in the test).

Tests

New cmd/server/node_reach_cold_limit_test.go (9 tests): many distinct holders bound to the cap, shipped defaults, cancellation while queued, saturation/recovery, plus handler-level coverage for 24 distinct cold keys, 12 identical keys, warm-hit bypass under saturation, compute error, cancelled request, and 429 + Retry-After + no-cache-poison + identical report after recovery.

Six mutants were run and all were killed; see the report comment for the mapping.

Perf proof (offline fixture only, never production)

Derived offline fixture: test-fixtures/e2e-fixture.db freshened + migrated, inflated to 400k observations so a cold Reach scan costs ~1s. Two binaries — master 2dc04642 and this branch — each restarted cold, measured with the same harness (idle probes, then 48 distinct cold Reach keys in parallel while /api/nodes and /api/packets are probed for 12s).

probe	master 2dc04642	this branch
saturated /api/nodes	p50 1ms, p95 1ms, max 9145ms, 1148 samples/12s	p50 1ms, p95 2ms, max 12ms, 11838 samples/12s
saturated /api/packets	p50 39ms, p95 200ms, max 9144ms, 45 samples/12s	p50 25ms, p95 73ms, max 351ms, 378 samples/12s
48 distinct cold Reach keys	48×200, p50 7556ms, max 10400ms	15×200 + 33×429, p50 3003ms, max 3063ms
6 distinct cold Reach keys	6×200, max 3848ms	6×200, max 1990ms (none shed)
warm repeat of those keys	p50 4ms	p50 2ms

/api/packets throughput under Reach saturation goes from ~3.7 req/s to ~31 req/s, and the ~9s worst-case stall on both unrelated endpoints disappears. A moderate (6-key) cold burst is still fully served.

Gates

• go test ./... green in cmd/server and every other module.
• sh test-all.sh: 233 passed, 0 failed. node test-frontend-helpers.js: 709 passed, 0 failed.
• Reach E2E against a local Go server on the CI-prepared fixture: test-node-reach-e2e.js, test-reach-rank-e2e.js (14/14), test-issue-1630-reach-mobile-e2e.js (7/7) pass; test-node-reach-coverage-e2e.js skips (coverage disabled).
• cmd/server stays read-only; no new map[string]interface{} outside tests; no colors; scripts/check-xss-sinks.sh --diff origin/master clean (no frontend changes); fork guards unchanged at 9 in deploy.yml and 1 in release-fast-path.yml.

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#358 #341 — head e90f803

Status: Implemented and verified — cold Reach scans are capped at 2 across distinct keys with a bounded 3s queue wait and 429 + Retry-After on sustained saturation; 9 new tests, 6 mutants killed, all CI jobs green, and an offline A/B shows unrelated endpoints keep ~8x the throughput and lose a ~9s worst-case stall.

Acceptance criteria

Requirement	Implementation	Test (red before / green after)	Mutant
Evaluate upstream's non-blocking two-build semaphore and 429/Retry-After against our pool and other workloads	Decision recorded in the node_reach_cold_limit.go header: keep the cap of 2 (half the 4-connection read pool from OpenDB), replace instant refusal with a bounded 3s queue wait, Retry-After: 2. Rationale: a cold report normally finishes in ~1s, Reach is one request per page view, and app.js api() auto-retries only 503 — never 429 — so instant refusal fails a page whose database is under no real pressure. Queue time costs a parked goroutine, not a connection.	TestReachColdLimiter_DefaultsBoundToTwoWithBoundedWait [T] — pins cap 2 and a bounded positive wait, and that a zero-value limiter comes up on them	M3 default cap 2 → 64: only the defaults test fails [K]
Acquire only for genuinely cold work, after the cache recheck	s.reach.cold.acquire sits inside the singleflight.Do closure, after the in-flight reachCacheGet recheck	TestNodeReach_WarmHitSkipsColdLimit [T] — with the only slot held and a 10s wait, a warm key answers 200 immediately and the acquire counter does not move; TestNodeReach_IdenticalKeysCoalesceIntoOneSlot [T]	M2 acquire moved to handler entry, before the cache fast path: warm-hit test fails (429 after a 10s stall) and the coalescing test fails [K]
Always release on success, failure and cancellation	defer s.reach.cold.release() immediately after a successful acquire, so success, compute error, marshal error, cancellation and panic all release	TestNodeReach_ComputeErrorReleasesColdSlot [T] (DROP TABLE observations → 500, inflight back to 0, slot reusable); TestNodeReach_CancelledColdRequestReleasesSlot [T]; TestReachColdLimiter_CancellationDoesNotConsumeSlot [T] (a queued waiter that is cancelled returns context.Canceled, takes no slot; an already-dead context fails fast)	M1 release deferred only on the success path (no release on compute error): both handler release tests fail [K]
Keep warm hits responsive and retain same-key coalescing	Handler cache fast path is untouched and precedes the singleflight call; one slot per coalesced herd	TestNodeReach_WarmHitSkipsColdLimit [T]; TestNodeReach_IdenticalKeysCoalesceIntoOneSlot [T] — 12 concurrent requests on one cold key, acquiredCount == 1, rejected == 0	M2 (above) raises it to 12 [K]
Preserve visibility filtering, ranking snapshots and response/cache semantics	No change to visibleReach, applyReachRank, reachBody, the blacklist-generation cache key or the purge path; a shed request caches nothing and singleflight does not cache errors	TestNodeReach_SaturatedColdRequestReturns429AndRecovers [T] — cache length 0 after the shed, then the recovered body is byte-identical to the unsaturated baseline (window start and tie-ordered lists normalised) and cache length 1; the full existing reach suite (visibility, blacklist cache, endpoint, rank) stays green [A]	M5 429 without Retry-After: the saturation test fails [K]
Test many distinct keys	—	TestNodeReach_ColdScansBoundedAcrossDistinctKeys [T] — 3 nodes x 8 windows = 24 distinct cold keys in parallel, all 200, peak concurrent cold scans <= 2, one acquire per key, inflight 0 at the end; TestReachColdLimiter_BoundsManyDistinctHolders [T] — 32 holders, independently tracked peak is exactly 2	M6 slot channel capacity n+1: 6 tests fail including both bound tests [K]
Test identical keys	—	TestNodeReach_IdenticalKeysCoalesceIntoOneSlot [T]	M2 [K]
Test cancellation and errors	—	TestReachColdLimiter_CancellationDoesNotConsumeSlot, TestNodeReach_CancelledColdRequestReleasesSlot, TestNodeReach_ComputeErrorReleasesColdSlot [T]	M1 [K]
Test saturation and recovery	—	TestReachColdLimiter_SaturationRejectsThenRecovers [T] (sentinel on timeout, accepts again after release); TestNodeReach_SaturatedColdRequestReturns429AndRecovers [T] (429, Retry-After: 2, JSON body, nothing cached, identical 200 after release)	M4 saturated acquire returns nil instead of the busy sentinel: 3 tests fail [K]
Unrelated endpoints stay responsive on an offline fixture, never production	Cap leaves >= 2 of 4 read connections free	A/B measurement below [A]	—
No global admission control; coordinate with #221	Per-endpoint cap only; the limiter header and the PR body state that any broader policy belongs with #221, which covers a different endpoint	—	—

Red evidence [T]: the suite first failed to build (srv.reach.cold undefined); with the limiter added but not yet wired, the five handler tests failed behaviourally (peak concurrent cold scans = 0, want >= 1, acquiredCount = 0, want 1, status=200 want 429). All nine pass after wiring, including -race -count=8.

Perf proof (offline fixture only — no production load test)

Fixture: test-fixtures/e2e-fixture.db, freshened and migrated as in CI, then inflated to 400k observations in a scratch copy so one cold Reach scan costs ~1s. Two binaries, master 2dc04642 and this branch, each restarted cold on a local loopback port, same harness: idle probes, then 48 distinct cold Reach keys in parallel while /api/nodes and /api/packets are probed for 12s.

probe	master 2dc04642	this branch
idle /api/nodes	p50 1ms, p95 1ms, max 25ms	p50 1ms, p95 2ms, max 29ms
idle /api/packets	p50 29ms, p95 63ms, max 332ms	p50 30ms, p95 58ms, max 121ms
saturated /api/nodes	p50 1ms, p95 1ms, max 9145ms, 1148 samples/12s	p50 1ms, p95 2ms, max 12ms, 11838 samples/12s
saturated /api/packets	p50 39ms, p95 200ms, max 9144ms, 45 samples/12s	p50 25ms, p95 73ms, max 351ms, 378 samples/12s
48 distinct cold Reach keys	48x200, p50 7556ms, max 10400ms	15x200 + 33x429, p50 3003ms, max 3063ms
6 distinct cold Reach keys	6x200, max 3848ms	6x200, max 1990ms — none shed
warm repeat of those keys	p50 4ms	p50 2ms

/api/packets throughput under Reach saturation rises from ~3.7 to ~31 req/s, and the ~9s worst-case stall on both unrelated endpoints is gone. Every probe returned 200 on both builds. A moderate 6-key cold burst is still served in full; shedding starts only under sustained saturation. [A]

The 3s queue wait was chosen from this measurement: at 1s, the same 6-key burst shed 2 of 6 because a cold scan on this fixture takes ~1s. [A]

CI per job (run 37675768680)

job	result
Go Build & Test	pass (25m51s)
Playwright E2E Tests	pass (27m56s)
Build & Publish Docker Image	pass (54s)
Release Artifacts / Deploy Staging / Publish Badges	skipping (fork guards)

No re-runs needed; the known-unstable test (#271) did not fire. [K]

Local gates

• go test ./... green in cmd/server and in every other module (ingestor, migrate, decrypt, anomaly, regions, prunequeue, channelregistry, dbschema; geofilter has no tests). [T]
• sh test-all.sh: 233 passed, 0 failed. node test-frontend-helpers.js: 709 passed, 0 failed. [T]
• Reach E2E against a local Go server on the CI-prepared fixture: test-node-reach-e2e.js OK, test-reach-rank-e2e.js 14/14, test-issue-1630-reach-mobile-e2e.js 7/7; test-node-reach-coverage-e2e.js skipped (client-RX coverage disabled on a default deployment). [T]
• cmd/server stays read-only; no new map[string]interface{} outside tests; no hardcoded colors; scripts/check-xss-sinks.sh --diff origin/master reports no public/** changes to scan; fork guards unchanged at 9 in deploy.yml and 1 in release-fast-path.yml. [K]

Residuals / follow-ups (not filed)

1. 429 is not handled by the front end. app.js api() auto-retries only 503, so a shed cold Reach request renders Failed to load reach: API 429: …. The 3s queue wait makes this rare in dashboard use, but a friendly "busy, retrying" state plus one automatic retry on Retry-After would be the honest finish. Deliberately left out to keep this change server-side and avoid an untested front-end path.
2. Cap and wait are compile-time vars, not configuration. Tuning them on a deployment needs a rebuild. A config knob would fit naturally alongside whatever Evaluate regional node-query pool pressure and the existing cache proposal (upstream 2101) #221 lands.
3. No counter is exposed. The limiter tracks inflight/peak/acquired/rejected/cancelled, but nothing surfaces them on /api/stats or /api/healthz, so saturation is invisible to an operator. Kept out of this PR because it would change an API response shape.
4. Link ordering on full ties is non-deterministic (equal bidir, bottleneck and we_hear + they_hear come out in map-iteration order). Pre-existing, unrelated to this change, and worked around in the new test by sorting on pubkey before comparing — same family as the earlier count-sort tie work.
5. Coordination with Evaluate regional node-query pool pressure and the existing cache proposal (upstream 2101) #221: this is a per-endpoint cap on Reach's own cold work, not database admission control. It does not bound regional /api/nodes scans, and the two caps would need to be reasoned about together if Evaluate regional node-query pool pressure and the existing cache proposal (upstream 2101) #221 lands a second limiter over the same 4-connection pool.

[dborup-agent]

Independent verification — CS-MacBook PR#358 #341 — head e90f803

Independent reproduction of the PR's claims (reach E2E + perf A/B). This is a verification run, not a line-by-line correctness review, and carries no formal verdict. Ran in an isolated scratch tree (branch merged with master), ports 13900–13903; every server was stopped by PID and every port freed afterwards.

Evidence tags: [T] executed, [A] analysis/reading, [K] known/caveat.

Reach E2E (branch server on the CI-prepared fixture, port 13900)

Fixture prepared as in CI: freshen-fixture.sh → corescope-migrate → seed-2073 → seed-199. Server built from the merged tree with -public public.

Test	Result
test-node-reach-e2e.js	PASS ("node-reach E2E OK") [T]
test-reach-rank-e2e.js	PASS 14/14 [T]
test-issue-1630-reach-mobile-e2e.js	PASS 7/7 [T]
test-node-reach-coverage-e2e.js	SKIP — "clientRxCoverage disabled on this deployment" (expected; coverage off on a non-instrumented build) [T][K]

Perf A/B (offline inflated fixture, never prod)

The fixture was copied and its observations inflated to 520,192 rows; one cold Reach scan ≈ 0.29 s. A master binary (origin/master archive) was built and migrated against the same inflated copy. For each binary, 48 distinct cold Reach keys (distinct pubkeys, days=30 → genuinely cold cache keys) were fired concurrently while /api/nodes and /api/packets were probed for 12 s.

Metric	master (no limiter)	branch (cap 2)
Reach 48 keys	48×200, 0×429	18×200, 30×429
Reach p50 / max	2616 / 4072 ms	3009 / 3314 ms
/api/nodes samples / p50 / p95 / max	14570 / 1 / 1 / 2858 ms	17420 / 1 / 1 / 46 ms
/api/packets samples / p50 / p95 / max	1463 / 7 / 9 / 3584 ms	1706 / 7 / 15 / 23 ms

Moderate 6-key burst on branch: 6×200, none shed (p50 909 / max 1256 ms); unrelated endpoints max ≤ 38 ms — the queue wait absorbs a transient burst exactly as the PR claims. [T]

Conclusion. The mechanism reproduces as documented [T]: without the limiter, cold Reach saturation stalls unrelated endpoints to multiple seconds; with cap 2, the excess cold keys shed 429 + Retry-After while /api/nodes and /api/packets stay sub-50 ms.

Caveats

• The 520k rows are duplicates of the fixture's original 508 observations, so the windowed per-node scan is cheaper (~0.29 s) than the PR's ~1 s on ~400k genuine rows. The master-side stall I measured is therefore ~3 s, not the PR's ~9 s — same direction and mechanism, smaller magnitude on a lighter fixture. [K]
• The reach E2E ran against -public public (not public-instrumented), which is why the coverage E2E skips — this matches CI behaviour for a non-instrumented build. [K]
• All servers stopped by PID (not $!); ports 13900, 13901, 13902, 13903 all verified free afterwards. [T]

[dborup-agent]

Review — CS-pve-agent2 PR#358 — head e90f803

Dom: APPROVE with nits

Independent, read-only review of the head above, merged in scratch onto current origin/master ff9a501c (git merge-tree --write-tree → 74285a26, clean; the merged tree differs from master only by this PR's 4 files). Evidence tags: [T] = I ran it, [A] = my analysis of the code, [K] = checked against CI / GitHub / git metadata.

Findings

#	Severity	Finding	Evidence
1	Nit (test gap)	The "after the in-flight cache recheck" part of the acquire ordering is not pinned by any test. If acquire is moved up to the top of the singleflight closure, before the inner reachCacheGet recheck, all 9 new tests and the rest of the reach suite stay green. The author's M2 moved acquire to handler entry, which is coarser, so the warm-hit test catches it; this narrower mutant survives. Real-world impact is small: a request that misses the outer cache just as another request fills it would then queue, or get a 429, for a key that is already warm. The code is correct today. Suggestion: move the closure body into a method such as buildReachEntry(ctx, key, pubkey, days) and add a direct test. With the only slot held and the key already cached, it must return right away and leave acquiredCount unchanged.	Mutant M2a below [T]
2	Nit (test gap)	An over-release is invisible. If the handler calls release() after a failed acquire (429 or cancel), it frees a slot that another holder owns, and then the cap is no longer enforced. No test catches this, because release() is deliberately non-blocking, silently ignores an empty channel and lets inflight go negative. Suggestion: in TestNodeReach_SaturatedColdRequestReturns429AndRecovers, assert inflight == 1 right after the 429, while the test still holds the slot. Optionally, make release() log or panic in tests when inflight drops below 0.	Mutant M4 below [T]
3	Nit (test gap)	Cancellation while queued is covered only at limiter level (TestReachColdLimiter_CancellationDoesNotConsumeSlot). The handler test TestNodeReach_CancelledColdRequestReleasesSlot cancels after the slot is taken (the hook calls cancel()), and it accepts either 200 or 500. That is enough to prove release on cancel mid-scan (it killed my M5), but no handler-level test covers a request that is cancelled while it waits for a slot.	[A]
4	Follow-up (documented by the author)	429 is a new status that the only client renders as a failure. app.js api() retries only 503, so node-reach.js shows "Failed to load reach" with no retry. The margin before shedding is thin. In my run, a burst of 6 distinct cold keys was fully served, but its slowest request took 3.97 s, which is about 1 s scan plus up to 3 s of queue. So the last waiters came within a few hundred ms of the 3 s shed deadline. On data where a cold scan is slower, shedding starts earlier: once a concurrent scan takes more than about 1.5 s, a 5–6-key burst starts to shed, and once it takes more than 3 s, even the third concurrent cold view gets a 429 and shows an error. I agree with keeping this PR server-only. The front-end retry on Retry-After (the author's residual 1) should be tracked as a follow-up.	Perf table below [T]; public/app.js:225 [A]
5	Info	Pre-existing, but the queue makes it a little wider. The singleflight closure uses the leader request's context. If the leader disconnects while it is queued, every coalesced follower that is still connected gets context.Canceled and a 500. Before, that could only happen during the scan; now the window is up to 3 s longer. The existing comment at the sf.Do call already accepts this ("the next request recomputes"). Not blocking.	[A]

There are no correctness defects in the limiter or its wiring. The limiter is a buffered-channel semaphore with a non-blocking fast path, a bounded timed wait and ctx-aware cancellation. defer release() comes right after a successful acquire inside the singleflight closure, so every exit path releases, including a panic that singleflight re-raises. [A]

Answers to the review points

1. Acquire and release. [A][T]

• acquire is taken inside sf.Do, after the in-flight reachCacheGet recheck. It is released by defer on success, on a computeNodeReach error, on a visibleReach error, on a marshal error, on cancellation and on panic. A failed acquire (busy or ctx) never takes a slot, so nothing needs releasing.
• Mutant "no release on error" (M1): killed by ComputeErrorReleasesColdSlot and CancelledColdRequestReleasesSlot.
• Mutant "acquire before the cache check":
  • before the handler cache check (M2b): killed by WarmHitSkipsColdLimit (429 after 10 s) and IdenticalKeysCoalesceIntoOneSlot (acquiredCount = 12);
  • before the in-flight recheck (M2a): survives, see finding 1.

2. Behaviour.

• Warm hits are answered by the handler fast path before singleflight and never touch the limiter. Identical keys still coalesce: 12 concurrent requests on one cold key take one slot. [T]
• On saturation the response is 429 with Retry-After: 2 and a JSON body, after a bounded 3 s queue wait. This is documented in docs/api-spec.md.
• The choice is justified against the pool: OpenDB sets SetMaxOpenConns(4), and a cap of 2 keeps two read connections free. Queueing holds a goroutine, not a connection, because acquire precedes scanReachRows. [A]
• Visibility filtering (visibleReach), applyReachRank / rank view, the blacklist-generation cache key and purge, and the body and cache semantics are unchanged in the diff. A shed request caches nothing. Singleflight does not keep errors, so the next request recomputes. [A][T: saturation test asserts cache len 0, then an identical body]

3. Tests. Every case is covered:

• many distinct keys: 24 handler keys and 32 limiter holders, with peak measured independently;
• identical keys;
• cancellation: unit-level while queued, handler-level mid-scan;
• errors: DROP TABLE observations gives a 500 and the slot is reused;
• saturation and recovery: 429, nothing cached, identical 200 afterwards.

Gaps are listed in findings 1 to 3. [T]

4. Perf proof. I repeated it myself on an offline scratch copy of the CI-prepared e2e-fixture.db, inflated to 402,536 observations. One cold Reach scan takes about 1.0 s there. Setup and harness:

• master ff9a501c and the merged tree, each started cold on a loopback port;
• 4 s of idle probes;
• then 48 distinct cold Reach keys (24 nodes × days 6/29) in parallel while /api/nodes?limit=50 and /api/packets?limit=50 are probed sequentially for 12 s;
• then 6 distinct cold keys, then those 6 again warm.

No production or staging access was used. [T]

probe	master ff9a501c	PR (merged), run 1	PR (merged), run 2
idle /api/nodes	p50 2 ms, max 8 ms	p50 2 ms, max 6 ms	p50 2 ms, max 5 ms
idle /api/packets	p50 20 ms, max 30 ms	p50 20 ms, max 29 ms	p50 20 ms, max 27 ms
saturated /api/nodes	n=2 in 12 s, each ~33.3 s	n=5646, p95 3 ms, max 12 ms	n=5453, p95 4 ms, max 55 ms
saturated /api/packets	n=1 in 12 s, ~33.3 s	n=362, p95 86 ms, max 101 ms	n=323, p95 92 ms, max 106 ms
48 distinct cold Reach keys	48×200, p50 18.2 s, max 34.7 s	3×200 + 45×429, p50 3.0 s, max 4.9 s	4×200 + 44×429, p50 3.0 s, max 5.8 s
6 distinct cold Reach keys	6×200, max 4.4 s	6×200, max 4.1 s	6×200, max 4.0 s
warm repeat of those 6	p50 1 ms	p50 1 ms	p50 1 ms

This confirms the claim, and on my 4-core host the effect is stronger than the author measured: on master the unrelated endpoints stall for about 33 s, and with the PR they stay at their idle latencies. Every unrelated probe returned 200 on both builds. My PR runs served fewer of the 48 cold keys than the author's (3 to 4 vs 15). The cap is unchanged; the difference comes from slower concurrent scans on this host, and it fits finding 4.

5. Invariants. [T][K]

• cmd/server stays read-only: no write SQL in the diff, and readonly_invariant_test.go is green in the full suite.
• No new map[string]interface{}. The test file uses a named struct for the 429 body.
• No global admission control: the cap is a per-reachState field, and both the code comment and the PR body defer any broader policy to Evaluate regional node-query pool pressure and the existing cache proposal (upstream 2101) #221.
• No colors added. The 6 #xxx matches in the diff are issue references.
• check-xss-sinks.sh --diff reports no public/** changes to scan.
• Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml, and no workflow changes.
• No closing keywords: the body uses "Relates to perf(reach): bound concurrent cold-cache scans across distinct node requests #341", and closingIssuesReferences is empty.
• Single commit e90f8034, author and committer dborup <kontakt@meshview.dk>.

Tests and mutants

What	Result
New tests, merged tree, -race -count=3	pass [T]
Red before: new test file on master	build failure (undefined: reachColdBuildLimit, …) [T]
Red before: limiter added but handler unwired	4 handler tests fail behaviourally (peak … = 0, acquiredCount = 0, status=200 want 429). The two release tests pass vacuously without wiring; M1 and M5 make them red. [T]
cmd/server go test ./... (merged)	ok (2333 PASS, 0 FAIL, 1353 s, -v -timeout 60m). Reach + read-only-invariant subset (79 tests) also green with -race. Note: two earlier full runs (default 10m, then CI's -race -timeout 20m) hit the timeout with no failing test; the slowest test on this host is Test1690_BackgroundLoadHonesty (644 s), which is environmental and unrelated to this diff [T]
cmd/ingestor go test ./... (merged)	ok (892 PASS, 0 FAIL, 1077 s, -timeout 60m). Untouched by the PR; it also timed out at 10m/20m on this host, which confirms the timeouts are environmental [T]
sh test-all.sh (merged)	233 passed, 0 failed [T]
node test-frontend-helpers.js (merged)	709 passed, 0 failed [T]
Reach E2Es against a local Go server on e2e-fixture.db, prepared as in CI (freshen, the grouped-packet and GRP_DATA seed SQL, corescope-migrate, seeds 2073, 199 and 245)	test-node-reach-e2e.js OK; test-reach-rank-e2e.js 14/14; test-issue-1630-reach-mobile-e2e.js 7/7; test-node-reach-coverage-e2e.js SKIP (client-RX coverage disabled on a default deployment). The server was stopped by port. [T]
CI run 37675768680 on e90f8034	Go Build & Test pass; Playwright E2E pass; Docker build pass; Release / Staging / Badges skipped (fork guards). Single attempt; the known flakes #256 and #267 did not fire. [K]

Own mutants (all applied to node_reach.go in the merged scratch tree; tests: -run 'ReachCold\|NodeReach_'):

Mutant	Result
M1: no defer release, release only on the success / not-found paths (i.e. no release on error)	killed: ComputeErrorReleasesColdSlot (inflight 1), CancelledColdRequestReleasesSlot
M2a: acquire before the in-flight cache recheck inside sf.Do	survived, finding 1
M2b: acquire before the handler cache check (fast path)	killed: WarmHitSkipsColdLimit (429 after 10 s), IdenticalKeysCoalesceIntoOneSlot (acquired 12)
M3: errReachColdBusy mapped to 500	killed: SaturatedColdRequestReturns429AndRecovers (status 500)
M4: release() also called after a failed acquire (over-release)	survived, finding 2
M5: release skipped when the request context is cancelled	killed: CancelledColdRequestReleasesSlot (inflight 1)

Not verified

• No browser validation beyond the Reach E2Es above. The 429 UI state ("Failed to load reach") follows from reading app.js; I did not render it.
• No measurement on production-sized data, and none on staging or production. The perf proof is a single 4-core host on an inflated fixture, with the scan cost and the 6-key headroom as measured there.
• I did not re-run the author's six mutants. I did not run the full Playwright suite (test-e2e-playwright.js), only the Reach-related E2Es.
• The full cmd/server suite under -race did not finish within CI's 20m on this host. Locally I ran it without -race, plus the 79 Reach and read-only tests with -race. CI's own -race run on this head passed. [K]
• I did not run any modules other than cmd/server and cmd/ingestor; the PR does not touch them.
• Head checked with git ls-remote before and after this review: unchanged at e90f8034. [K]