fix(server): distance analytics reads a torn snapshot under concurrent ingest/eviction

[dborup]

Summary

computeAnalyticsDistance takes a snapshot of distHops/distPaths under s.mu.RLock and does all filtering outside the lock. Two paths broke that snapshot contract:

• A: area-only branch read live slices. With area set and no region, the match-set loops iterated s.distHops / s.distPaths directly after RUnlock. Ingest (updateDistanceIndexForTxs) and eviction shrink those slices under s.mu.Lock, so the loop indexes past the new length and panics in /api/analytics/distance?area=….
• B: in-place compaction rewrote the snapshot. The safety comment claimed the slices were "append-only via re-slice". They are not: updateDistanceIndexForTxs (s.distHops[n] = r; s.distHops = s.distHops[:n]) and evictStaleInternal (newDistHops := s.distHops[:0]; append…) compact in place under s.mu.Lock, over the backing array a compute is still reading. The compute then sees survivors shifted down plus stale tail records, i.e. duplicated or missing records, and returns wrong totals, averages, histograms and top paths without any error.

Both are data races on the same snapshot, so they are fixed together (see "Why one PR").

A third commit zeroes the tail that in-place compaction leaves behind, so evicted transmissions are actually released.

Reproduction (before the fix)

The deterministic tests park the compute after its RUnlock by holding one tx's decodedOnce, the same seam as TestComputeAnalyticsDistanceReleasesLockBeforeCompute. They then run the real writer under s.mu.Lock and let the compute finish.

A: the area-only compute is parked inside the area loop, and the writer removes records:

computeAnalyticsDistance panicked: runtime error: index out of range [1] with length 0   (updateDistanceIndexForTxs)
computeAnalyticsDistance panicked: runtime error: index out of range [3] with length 0   (EvictStale)

Under -race, TestDistanceSnapshot_AreaOnly_RaceStress reports the unlocked reads at store.go:7958 / store.go:7972.

B: records hash0000…hash0004 with Dist 1…5. The compute takes its snapshot, then a writer runs. A consistent answer is either the snapshot or the post-write state:

Writer after snapshot	Consistent answer	Actual answer
EvictStale (4 of 5 evicted), region+area	5 hops, avg 3.0 or 1 hop, avg 5.0	totalHops 5, avgDist 3.8, topPaths hash0001 hash0002 hash0003 hash0004 hash0004
updateDistanceIndexForTxs(tx1), region+area	5 hops, avg 3.0 or 4 hops, avg 3.25	totalHops 5, avgDist 3.6, topPaths hash0000 hash0002 hash0003 hash0004 hash0004
updateDistanceIndexForTxs (4 of 5), area-only	5 hops, avg 3.0	totalHops 5, avgDist 3.4, hash0002 twice
EvictStale (3 of 5), area-only	5 hops, avg 3.0	totalHops 5, avgDist 4.2, hash0003 and hash0004 twice

Under -race, TestDistanceSnapshot_DefaultCompute_RaceStress (the recomputer's ("", "") call) reports store.go:7992 (filter loop) racing store.go:4539 (compaction).

Severity

• A: a user-visible, intermittent failed request for the area filter in the analytics UI. net/http recovers the handler panic, so the process survives, but the request gets no response. The window is the whole area loop, including the first ParsedDecoded of each tx, and ingest can compact every poll cycle (1 s) whenever a best path changes.
• B: silently wrong distance analytics. The result is cached for up to 60 s for region/area (rfCacheTTL) or until the next recompute for the default query (5 min), then corrects itself. The window is only the snapshot copy, so it is hit occasionally.

Both are also data races on distHopRecord / distPathRecord values (strings, slices), which Go does not define.

Why one PR

The fixes depend on each other:

• A alone stops the panic, but the area branch then reads a snapshot that compaction can still rewrite. The panic becomes silently wrong numbers, and the partial-removal tests stay red.
• B alone leaves A's panic in place (mutation M1 below).

They share the pin window and the test harness. The commits are separate so each can be reviewed and bisected.

Fix

1. A: the area-only branch iterates hopsSnap / pathsSnap, like the rest of the compute.
2. B: distSnapReaders atomic.Int64 pins the snapshot.
   • The compute increments it under s.mu.RLock, in the same critical section that captures the slice headers. It decrements it as soon as the two filter loops have copied the snapshot into filteredHops / filteredPaths, with a defer fallback on panic.
   • A new compactDistIndex(remove) replaces both compaction loops. It reads the counter under s.mu.Lock. When it is zero it compacts in place exactly as before (dst[n] = r, no allocation). When it is non-zero it copies survivors into fresh arrays, and the pinned array stays owned by the snapshot.
   • The wrong "append-only" comment is replaced with the actual contract.
3. Tail: in the in-place branch only, compactDistIndex clear()s the vacated tail ([newLen, oldLen)) of both slices. Previously that tail kept *StoreTx pointers to removed and evicted transmissions, and their observations, reachable until a later append overwrote the slot. Eviction is memory-driven, so it should release what it evicts. The pinned branch writes to fresh arrays and must not touch the old one (the snapshot's), so it clears nothing.

Why the pin is correct

• A compute increments the counter while holding RLock; a writer reads it while holding Lock. The RWMutex orders them, so every compute that captured a snapshot before the writer acquired the lock is visible to the writer as a non-zero count. The writer then leaves that backing array alone.
• A compute that captures after the writer releases the lock snapshots the post-compaction slice.
• When the writer's Load returns 0, it observes the last Add(-1). Go's atomics are sequentially consistent, so every snapshot read made before that decrement happens-before the writer's in-place writes.
• Appends (IngestNewFromDB) only write at indices ≥ the current length. That can only overlap a pinned snapshot after an in-place shrink, which the pin prevents. buildDistanceIndex always installs fresh slices.
• No extra work runs under s.mu: one atomic add per compute under RLock, and one atomic load per compaction under Lock.

Performance

Setup: Intel Xeon @ 2.80GHz, 4 vCPU, 15 GiB, Linux, Go 1.24.7, no other load. Each row is 5 runs (-count=5); ns/op is shown as median (min–max). Sizes come from the committed e2e fixture: 3.62 hop records and 0.72 path records per transmission, with distHopRecord at 152 B and distPathRecord at 80 B. At ~30K transmissions that is ~110K hops and ~21.6K paths.

updateDistanceIndexForTxs(1 tx) at 110K hops / 21.6K paths. This is the per-ingest-cycle cost when a best path changes, run -benchtime=100x. The same self-contained harness runs on all three revisions:

Revision	ns/op, median (min–max)	B/op	allocs/op
master 8a7a69e	2.83 ms (2.69–3.31)	3344	30
commit 2 (pin)	2.84 ms (2.59–2.92)	3344	30
commit 3 (pin + tail clear)	2.52 ms (2.35–3.14)	3344	30

New BenchmarkUpdateDistanceIndexForTxs in this PR (same sizes, with the test store's string fields filled in):

Revision	unpinned	pinned (a compute holds a snapshot)
commit 2	2.99 ms (2.75–3.41), 3344 B, 30 allocs	14.50 ms (12.97–15.12), 18.4 MB, 32 allocs
commit 3	3.00 ms (2.93–3.18), 3344 B, 30 allocs	14.32 ms (13.92–15.19), 18.4 MB, 32 allocs

• Unpinned (the common case): indistinguishable from master within run-to-run spread, with identical allocations. clear() costs O(removed records), a memclr of one tx's records.
• Pinned: this is the copy path, and it costs about as much as an unconditional copy-on-write would (~14 ms under s.mu.Lock and ~18 MB allocated per compaction). Doing that on every ingest cycle (up to 1 Hz) was rejected; it only happens while a compute is inside its short copy window.

Eviction. The tail clear() runs once per eviction over the removed records:

Benchmark	master	commit 2	commit 3
evict 25 % (7,500 of 30K tx, ~27.5K of 110K hops), -benchtime=5x	75.0 ms (69.0–78.9), 10.97 MB, 187,729 allocs	74.0 ms (69.7–76.2), same allocs	73.7 ms (70.5–77.9), same allocs
existing BenchmarkEviction100K (all 100K tx; 100K hops + 100K paths cleared)	1006 ms (980–1056), 135.6 MB, 2,501,580 allocs	1031 ms (970–1056), same	1013 ms (997–1058), same

All differences are within noise, and allocations are unchanged.

Tests

New file cmd/server/distance_snapshot_race_test.go. It uses makeTestStore from eviction_test.go, and every writer is the real function.

Test	Commit	Covers
TestDistanceSnapshot_AreaOnly_UpdateIndexMidLoop	1	A, updateDistanceIndexForTxs drops all records mid-loop; answer = snapshot
TestDistanceSnapshot_AreaOnly_EvictMidLoop	1	A, EvictStale evicts all mid-loop; answer = snapshot
TestDistanceSnapshot_AreaOnly_RaceStress	1	A under -race against slice swaps (buildDistanceIndex)
TestDistanceSnapshot_AreaOnly_UpdateIndexMidLoop_Partial	2	A+B, survivors moved in place mid-loop
TestDistanceSnapshot_AreaOnly_EvictMidLoop_Partial	2	A+B, same with eviction
TestDistanceSnapshot_RegionArea_EvictAfterSnapshot	2	B, eviction after snapshot
TestDistanceSnapshot_RegionArea_UpdateIndexAfterSnapshot	2	B, updateDistanceIndexForTxs after snapshot
TestCompactDistIndex_InPlaceUnlessPinned	2	no new backing array when unpinned; pinned snapshot unchanged after compaction and append
TestDistanceSnapshot_PinReleased	2	pin released for all four region/area combinations
TestDistanceSnapshot_DefaultCompute_RaceStress	2	B under -race on the default ("", "") compute
TestCompactDistIndex_ClearsVacatedTail	3	vacated tail zeroed and live records intact, via updateDistanceIndexForTxs and EvictStale
BenchmarkUpdateDistanceIndexForTxs	2	one-tx recompute at ~110K hops, unpinned vs pinned

Commit 1's tests remove every record, so the writer overwrites no snapshot element and they are race-clean with only the A fix. The partial-removal variants need B and live in commit 2.

Red → green per commit:

• Commit 1: its tests on master fail with the two panics above, and the stress test reports DATA RACE.
• Commit 2: its tests on commit 1 (pin calls stubbed so they compile) fail with the mixed summaries in the table above. InPlaceUnlessPinned fails because the pinned snapshot is rewritten to [hash0002 hash0003 hash0004 new], and the default stress test reports DATA RACE. PinReleased is a guard for the new pin and passes trivially there.
• Commit 3: its test on commit 2 fails with tail elements still holding hash0001…hash0004 and their *StoreTx.

Mutations (all fail the committed tests)

Run against the committed tests at ef04791 (non-race, deterministic):

#	Mutation	Failing tests
M1	area-only branch reads s.distHops/s.distPaths again	AreaOnly_UpdateIndexMidLoop, AreaOnly_EvictMidLoop, both _Partial
M2	ignore the pin (always in place)	all four AreaOnly_*, both RegionArea_*, InPlaceUnlessPinned
M3	always copy (perf regression)	InPlaceUnlessPinned
M4	unpin right after RUnlock	all four AreaOnly_*, both RegionArea_*
M5	never unpin	PinReleased
M6	eviction keeps its own in-place loop instead of compactDistIndex	AreaOnly_EvictMidLoop_Partial, RegionArea_EvictAfterSnapshot, ClearsVacatedTail
M7	eviction drops the compactDistIndex call	ClearsVacatedTail
M8	no tail clear()	ClearsVacatedTail
M9	clear() the old array's tail also when pinned (overwrites the snapshot)	all four AreaOnly_*, both RegionArea_*, InPlaceUnlessPinned
M10	clear() interval starts one too late (n+1)	ClearsVacatedTail
M11	clear() interval ends one too early (old-1)	ClearsVacatedTail
M12	clear() interval starts one too early (n-1, into live data)	InPlaceUnlessPinned, ClearsVacatedTail

Verification

• go vet ./... and gofmt -l on the touched files are clean.
• The full go test -race -count=1 ./... in cmd/server passes on every commit on its own: commit 1 ok 601.8s, commit 2 ok 587.4s, commit 3 ok 602.0s (local Go 1.24.7; CI uses 1.27.1).
• The read-only invariant tests (TestServerSourceHasNoCachedRWCalls, TestServerDBHasNoWriteMethods, TestServerDBConnIsReadOnly, TestPacketStoreHasNoMultibytePersistMethods) pass. The change adds no DB writes and no new map[string]interface{}.
• No frontend or API-shape change, so there is no browser validation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RqfyMm2rmAvNr27smJjyDe

---

Generated by Claude Code