[dborup-agent]

Relates to #492

What

New opt-in setting analytics.pauseWhenIdle, default false. With it off, nothing changes. With it on, a scheduled recompute of an analytics snapshot is skipped when nothing has read that snapshot since the previous pass. The first read after a pause gets the existing snapshot straight away and starts one coalesced refresh. Later reads get the refreshed snapshot.

Plan (done in this PR)

1. List the fork's recomputers and their dependencies.
   • The 9 analyticsRecomputer instances started by StartAnalyticsRecomputers are in scope: rf, topology, channels, distance, hash-collisions, hash-sizes, observers-clock-skew, nodes-clock-skew, roles.
   • Only one of them reads another's snapshot: computeAnalyticsRoles calls GetFleetClockSkew(""), so roles depends on nodes-clock-skew. No other compute function calls a recomputer reader (checked with grep: every other Get* caller is an HTTP handler).
   • Out of scope, unchanged: the neighbor-graph, bridge, usefulness-axes and repeater-enrich loops, and the ping-score-history worker. They are separate loops (not analyticsRecomputer) and feed /api/nodes and the graph, which the UI reads all the time. Pausing them would not save anything and would risk stale node data.
2. Config.
   • AnalyticsConfig.PauseWhenIdle and Config.AnalyticsPauseWhenIdle(), which is nil-safe.
   • The flag goes through AnalyticsRecomputeIntervals(), including the early return taken when no per-endpoint intervals are set.
   • StartAnalyticsRecomputers applies it to every recomputer in a single list, analyticsRecomputersLocked().
3. Loop.
   • When a tick may skip: only when the opt-in is on, the recomputer has settled (a successful pass that started after the startup load finished), the last pass succeeded (no panic and not nil), and there has been no read since the previous pass.
   • Unchanged: RecomputeNow always runs, so the post-load passes from fix(analytics): recompute cached snapshots after the complete startup load #116 and the distance-index refresh still happen. Warm-up gates from Analytics cards show post-restart slice with 'All data' selected until multiple manual refreshes Kpa-clawbot/CoreScope#1659 are untouched.
   • No lost wake-up: the loop publishes paused before checking readSinceCompute, and a reader stores readSinceCompute before checking paused. A read that races a tick is therefore either seen by the tick, which then computes, or sees paused and queues a wake-up.
   • Bounded wake-ups: they go on a buffered channel of capacity 1 with a non-blocking send. A wake-up that arrives after a pass has already run is dropped, so a burst of reads costs at most one pass and nothing queues without bound.
   • Dependencies: roles declares nodes-clock-skew as a dependency. Before roles computes, a paused nodes-clock-skew is refreshed through its own RecomputeNow channel; this gives up if either side stops. Roles' compute also reads the clock-skew snapshot, which counts as a read, so the dependency keeps ticking while roles is being read.
4. Freshness: a stale snapshot is never labelled fresh.
   • With the opt-in on, every default-shape response served from a recomputer (all 9 endpoints) gets X-Analytics-Snapshot: current|stale and X-Analytics-Snapshot-Age-Seconds.
   • stale means the snapshot missed a refresh that was skipped while idle. It is cleared only when a new snapshot is stored: not when the wake pass starts, and not by a pass that fails.
   • The headers are read before the snapshot, so a refresh that lands in between can only make the response newer than its label.
   • With the opt-in off, no headers are set. The JSON bodies never change.
5. Docs.
   • config.example.json gets pauseWhenIdle: false plus an operator comment on the freshness trade-off: the first post-idle result can be much older than the interval.
   • The startup log line now prints the setting.
   • Follow-up (AGENTS.md rule 8): add the setting to the customizer and the config docs in a later milestone. Not done here.

Tests (cmd/server/analytics_pause_when_idle_test.go)

The loop tests use an injected manual ticker (recomputeTicker) and a loop-event hook. A tick is a channel send, and the test waits for the loop to report tick-run, tick-skip, kick-run, kick-ignored or recompute-now. SnapshotHeaders starts the store's recomputers with manual tickers (newRecomputeTicker swapped for the start) and waits for each recomputer's post-load pass before it checks headers, so no background pass can land during the checks. No test sleeps. On origin/master the file does not compile because the API is missing (red); with this PR it is green.

Requirement	Test
nil, absent or false config leaves behaviour unchanged	ConfigDefaultsOff, DefaultOffComputesEveryUnreadTick, DefaultOffNeverStale, SnapshotHeaders (no headers when off)
true reaches every recomputer	ConfigTrueReachesIntervals, ReachesEveryRecomputer (finds every *analyticsRecomputer field by reflection, so a future recomputer without the flag fails)
unread ticks	UnreadTicksSkip
continuous reads	ContinuousReadsKeepComputing, ReadDuringPassCounts (a read while a pass computes counts for the next tick)
concurrent wake-ups, readers never wait, no storm	WakeServesSnapshotAndCoalesces (64 concurrent readers while the wake pass is blocked: all return the old snapshot, exactly 1 wake pass, ≤1 dropped wake-up, channel capacity 1, the wake pass restarts the ticker)
lost wake-up	NoLostWakeupInPauseWindow (a read injected between "pause armed" and "read checked")
first computation	FirstComputationNeverPauses
startup load	StartupLoad (no pause before a pass started after the load; post-load RecomputeNow runs while paused)
failed computation	FailedComputationRecovers (a panic and a nil result)
shutdown	Shutdown (while paused, during a wake pass, with the dependency already stopped)
dependency	DependencyNotStarved
never falsely fresh	SnapshotInfoNeverFalselyFresh, SnapshotHeaders (all 9 endpoints; exact age from an injected clock; region, area and window shapes carry no headers)

Mutants: 24 total, all killed (the 19 from round 1, plus R1, R3, R4, R6 and R7, which survived the round-1 review). Each one is applied to a scratch copy, the pause tests are run, and the copy is restored.

#	Mutant	Killed by
M1a	opt-in ignored in wiring	ReachesEveryRecomputer, SnapshotHeaders
M1b	opt-in dropped in config	ConfigTrueReachesIntervals
M1c	opt-in forced on	ReachesEveryRecomputer, SnapshotHeaders
M1d	tick ignores the opt-in	DefaultOffComputesEveryUnreadTick, DefaultOffNeverStale
M2a	startup RecomputeNow skipped while paused	StartupLoad, DependencyNotStarved
M2b	pause allowed before the startup load	StartupLoad
M2c	pause before the first successful snapshot	FirstComputationNeverPauses, StartupLoad, FailedComputationRecovers
M3a	lost wake-up (no wake-up sent)	WakeServesSnapshotAndCoalesces, FailedComputationRecovers
M3b	lost wake-up (check-then-publish order)	NoLostWakeupInPauseWindow
M3c	recompute storm (every wake-up runs)	NoLostWakeupInPauseWindow
M3d	readers wait (blocking wake-up send)	test timeout
M4a	falsely fresh (stale cleared at pass start)	SnapshotInfoNeverFalselyFresh, FailedComputationRecovers
M4b	falsely fresh (header ignores stale)	SnapshotHeaders
M4c	falsely fresh (skip not recorded)	SnapshotInfoNeverFalselyFresh, FailedComputationRecovers
M4d	header missing on one endpoint	SnapshotHeaders
M5	failed pass allowed to pause	FailedComputationRecovers
M6a	dependency not refreshed	DependencyNotStarved
M6b	roles dependency not wired	ReachesEveryRecomputer
M7	stopped dependency hangs the dependent	Shutdown
R1	nil result counts as a successful pass	FailedComputationRecovers/nil
R3	wake pass does not restart the ticker	WakeServesSnapshotAndCoalesces
R4	snapshot headers on area-filtered hash-sizes	SnapshotHeaders
R6	readSinceCompute cleared after compute instead of before	ReadDuringPassCounts
R7	age header always 0	SnapshotHeaders

Measurement

PAUSE_IDLE_MEASURE=1 PAUSE_IDLE_WINDOW=10s go test -run TestPauseWhenIdle_Measure -v ./cmd/server. It uses a copy of test-fixtures/e2e-fixture.db loaded into a real PacketStore (graph before packets, as in main.go), runs all 9 recomputers at 100 ms for 10 s, and settles every recomputer first.

Traffic levels:

• idle: no reads.
• sparse: all 9 default snapshots read once every 10 intervals, so each read is a first read after idle.
• visible: all 9 read once per interval, like an open analytics page polling.

The test skips unless the variable is set, because it uses real tickers.

The Get* columns are in-process Get* latency: the time of one readAllDefaultSnapshots call, which calls all 9 default-shape store readers (GetAnalyticsRF, …, GetFleetClockSkew) once, in the test process. It is not HTTP request latency.

"Before" is origin/master 9080b6aa with the same harness (flag removed).

build	pauseWhenIdle	traffic	compute passes	alloc MB	mallocs (k)	CPU s*	Get* p50	Get* p99
before	–	idle	898	478.5	6051	1.81	–	–
before	–	sparse	898	479.6	6067	2.09	2.8µs	8.7µs
before	–	visible	898	478.6	6053	2.69	3.8µs	7.5µs
after	false	idle	896	479.2	6046	3.14	–	–
after	false	sparse	899	480.2	6063	2.70	5.3µs	45.8µs
after	false	visible	898	479.2	6064	2.31	3.4µs	10.5µs
after	true	idle	0	0.1	1.9	0.00	–	–
after	true	sparse	91	44.6	561	0.27	15.5µs	22.1µs
after	true	visible	892	477.0	6035	3.05	6.6µs	22.2µs

* CPU comes from runtime/metrics (/cpu/classes/total − idle). It is an estimate and was noisy on a shared developer machine (other agents running), so read it as an order of magnitude only.

What the numbers show:

• Off matches master: compute passes, allocations and mallocs are the same within noise. Each pass adds one small snapshot wrapper; at most 9 per interval.
• Idle: compute passes and allocations drop to about 0.
• Sparse reads: cost drops in proportion to how often snapshots are read. Each read starts one refresh, about 9 passes per 10 intervals.
• Visible traffic: the same work as master.
• In-process Get* latency (not HTTP request latency): stays in microseconds in every mode, including the first read after idle. That read never waits for a computation. With the setting on, visible-traffic reads are about 2× slower (the extra atomics), which is negligible in absolute terms.
• Scale caveat: the fixture is small (about 500 transmissions), so absolute numbers are small. The ratios are what transfers to a real deployment. Savings on a real deployment are not measured here.

Freshness trade-off for operators

With pauseWhenIdle: true, the first analytics response after a quiet period can be arbitrarily old: as old as the last read. It is labelled X-Analytics-Snapshot: stale and carries its age. The next response, about one compute later, is fresh. Startup, warm-up 503s, the distance lazy build and recovery from failed recomputes behave the same as with the setting off.

Invariants

• cmd/server stays read-only: no DB writes are added.
• No new map[string]interface{} outside tests.
• No frontend change.
• scripts/check-xss-sinks.sh --diff origin/master is clean.
• Fork guards are unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.
• Nothing is enabled on any deployment by this PR.

🤖 Generated with Claude Code

[dborup-agent]

Rapport — CS-MacBook PR#554 #492 — head edfdb93

Status: Draft. All requirements are implemented and tested, all 19 mutants are killed, and every CI job is green on the first run. Waiting for owner review; not marked ready and not merged.

Evidence: [T] = automated test or measurement run, [A] = observed through the API of a local server built from this branch, [K] = code inspection.

Requirements

Requirement	Evidence	Test (red on master → green)	Mutant(s), all killed
Recomputers and dependencies listed	[K] 9 analyticsRecomputers; only roles → nodes-clock-skew (GetFleetClockSkew); other loops out of scope (reasons in the PR description). [T] reflection finds 9 fields	ReachesEveryRecomputer	M6b
nil/absent/false = unchanged	[T]	ConfigDefaultsOff, DefaultOffComputesEveryUnreadTick, DefaultOffNeverStale, SnapshotHeaders (off: no headers)	M1c, M1d
true reaches every recomputer	[T] including the config early return when no per-endpoint intervals are set	ConfigTrueReachesIntervals, ReachesEveryRecomputer	M1a, M1b
Unread ticks pause	[T]	UnreadTicksSkip	M1d
Continuous reads keep computing	[T]	ContinuousReadsKeepComputing	— (see M3a/M3b)
Concurrent wake-ups coalesce; readers never wait; no storm; bounded	[T] 64 concurrent readers while the wake pass is blocked: all get the old snapshot, 1 wake pass, ≤1 dropped wake-up, channel capacity 1	WakeServesSnapshotAndCoalesces, NoLostWakeupInPauseWindow	M3a, M3c, M3d
No lost wake-up	[T] read injected between "pause armed" and "read checked"; [K] ordering of the paused/read handshake	NoLostWakeupInPauseWindow	M3b
First computation	[T] no pause before the first successful snapshot	FirstComputationNeverPauses	M2c
Startup load / warm-up gate	[T] no pause before a pass that started after the load; post-load RecomputeNow runs while paused	StartupLoad	M2a, M2b
Failed computation recovers	[T] panic or nil → next tick computes even if unread; a failed wake keeps stale	FailedComputationRecovers	M5
Shutdown / join	[T] while paused, during a wake pass, with the dependency already stopped	Shutdown	M7
Dependency not starved	[T] paused clock-skew is refreshed before roles computes; roles' reads keep it ticking	DependencyNotStarved	M6a, M6b
Never falsely fresh	[T] all 9 endpoints; [A] local server, interval 3 s: first read after 8 s idle → X-Analytics-Snapshot: stale, age 9 s; next read → current, age 1 s	SnapshotInfoNeverFalselyFresh, SnapshotHeaders	M4a, M4b, M4c, M4d
Measured counts / allocations / CPU / latency, before and after, idle vs visible	[T] table in the PR description. Idle with pause on: 898 → 0 passes, 478 MB → 0.1 MB. Off = master within noise. CPU is a noisy runtime/metrics estimate	TestPauseWhenIdle_Measure (manual, env-gated)	n/a
Operator docs / customizer follow-up	[K] config.example.json comment on the freshness trade-off; customizer/config docs noted as a follow-up (rule 8)	—	n/a

Red on master [T]: analytics_pause_when_idle_test.go does not compile against origin/master 9080b6aa, because pauseWhenIdle, recomputeTicker, AnalyticsRecomputeIntervals.PauseWhenIdle and the rest are missing. The mutants give per-requirement behavioural red.

Invariants

• [K] cmd/server stays read-only: no DB writes added.
• [K] No new map[string]interface{} outside tests (git diff adds 0).
• [T] scripts/check-xss-sinks.sh --diff origin/master: clean (no frontend changes).
• [K] Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. No workflow files touched.
• [K] No hardcoded colours: no CSS/JS changes.

Local runs

• [T] cmd/server: go test -race ./... ok (589 s) and go test ./... ok after the final edits.
• [T] cmd/ingestor: go test ./... ok.
• [T] sh test-all.sh: 265/265 passed.
• [T] node test-frontend-helpers.js: 719 passed, 0 failed.
• [T] Local E2E against a server built from this branch, on a CI-prepared e2e-fixture.db (freshen, CI seed SQL, migrate, the four seed files). Analytics suites 205, 236, 226, 1306 and 1657 pass with pause off and with pause on (interval 3 s).
• [T] test-e2e-playwright.js stops locally at "Version info lives on Perf dashboard" (#navStats wait). The same failure happens with a server built from origin/master, so it is environmental and not caused by this PR. In CI the suite passes (shards 1–3).
• The local server was stopped by pid; ports 13900–13902 were used and are free again.

CI (run 40087997868, first attempt, no re-runs)

Job	Result
Go Server Tests 1/3, 2/3, 3/3	pass
Go Ingestor Tests	pass
Go Libraries & CLI	pass
Go Build & Test	pass
JavaScript Unit Tests & Lint	pass
Script & Container Unit Checks	pass
Playwright E2E 1/3, 2/3, 3/3	pass
Verify E2E Shards & Coverage	pass
Verify Go Test Assignments	pass
Validate Docker Image	pass
Build & Publish Docker Image	pass (PR no-op)
Publish Badges, Release Artifacts, Deploy Staging	skipped (PR)

Known flake #271 did not appear.

Remaining items

1. The branch is based on 9080b6aa. origin/master has since moved to a4126c63 (neighbors: follow-ups from #541 (graph-age clamp, cache/publish tests, atomic.Pointer) #550), which touches config.go, routes.go and config.example.json. git merge-tree reports no conflict [T]. Not rebased, as instructed.
2. The savings are measured on the small e2e fixture only. The effect on a real deployment's allocations and GC is not proven, and the setting is not enabled anywhere.
3. Follow-up: add pauseWhenIdle to the customizer and config docs (rule 8).
4. Mutant M3d (blocking wake-up send) is killed by the 90 s test timeout rather than an assertion: the blocked readers deadlock the test.

[dborup-agent]

Review — CS-pve-agent2 PR#554 — head edfdb93

Dom: REQUEST CHANGES (test-only). The production change looks correct to me and meets the issue. The one blocker is that TestPauseWhenIdle_SnapshotHeaders is non-deterministic: I reproduced it failing, which goes against the issue's "deterministic synchronization-based tests" criterion. The fix is small.

Evidence: [T] = I ran it, [A] = observed via the HTTP API of a local server I built, [K] = code inspection.

Findings

#	Severity	Finding	Evidence
F1	Blocking (test)	TestPauseWhenIdle_SnapshotHeaders is flaky. The test calls signalStartupLoadDone() after StartAnalyticsRecomputers, so the async post-load goroutine (recomputeWhenLoaded) is still running RecomputeNow on the later recomputers while the endpoint loop runs. When that pass lands between missedRefresh.Store(true) and the GET, it stores a new snapshot, which clears missedRefresh, and the header is correctly current but the test expects stale. Seen on /api/analytics/hash-sizes and /api/observers/clock-skew (late in post-load order; endpoint order is random because it's a map). The production behaviour is right; the test is racing it.	[T] 2/50 and 2/100 failures with -count (no -race), 0/50 with -race. [T] Causal check in a scratch copy: with a 2 ms gap after missedRefresh.Store(true), 3/20 fail; with the post-load RecomputeNow disabled, 0/20 fail.
F2	Nit (test gap)	A nil compute result after settling may pause (mutant R1 survives). The PR text says a failed pass is "panic or nil", but only panic is tested. In production all 9 compute closures return typed values (typed-nil maps/slices are non-nil in an interface{}), so this branch is effectively unreachable today.	[T] mutant, [K] closures in StartAnalyticsRecomputers
F3	Nit (test gap)	Nothing pins the t.Reset(r.interval) after a kick-run (mutant R3 survives). Without it, the next scheduled tick can fire right after a wake pass and mark the brand-new snapshot stale early. manualRecomputeTicker.resets is counted but never asserted.	[T]
F4	Nit (test gap)	Nothing asserts that non-default shapes (?region=, ?area=, window) do not carry the snapshot headers (mutant R4, headers forced onto area-filtered hash-sizes, survives). The code is correct today.	[T] mutant; [A] /api/nodes/clock-skew?area=x has no headers
F5	Nit (test gap)	The "reads from here on count towards the next tick" ordering in runOnce is not pinned (mutant R6: clearing readSinceCompute after compute survives; a read during a pass would then be forgotten and the next tick would skip).	[T]
F6	Nit (test gap)	X-Analytics-Snapshot-Age-Seconds is only checked to be a non-negative integer (mutant R7: age always 0 survives). The injectable now would allow an exact assertion.	[T]
F7	Info	"Request latency" in the measurement is in-process Get* latency, not HTTP request latency. That's fine as a proxy, but worth naming as such in the description. With pause on, visible-traffic reads are about 2× slower (≈3.6 µs → ≈7.2 µs p50) because of the extra atomics; negligible in absolute terms.	[T]

Suggested fix for F1 (any of these works): drive the stale state through a real skipped tick (inject newRecomputeTicker for the test); or add a small test seam that exposes the post-load done channel and wait on it before the endpoint loop; or start the recomputers only after a barrier that guarantees the post-load pass for every recomputer has finished.

Answers to the review points

Default (absent/false) is today's behaviour. [K] With the flag off, markRead returns immediately, kick is never filled, pauseTick returns false, and no headers are set. The only differences are internal: one *recomputerSnapshot wrapper plus one time.Now() per pass, and a few atomic stores. [T] Config tests pass for nil, {}, {"analytics":{}} and explicit false. [A] Servers built from origin/master and from the merged tree, on identical copies of the CI-prepared fixture: the response bodies of rf, topology, channels, distance, hash-sizes, hash-collisions, roles and observers/clock-skew are byte-identical (sha256). nodes/clock-skew matches after sorting; the raw order differs because of the pre-existing range s.byNode map iteration. Response headers (except Date) are identical; no X-Analytics-* with the flag off.

true reaches every recomputer, including dependencies. [T] ReachesEveryRecomputer uses reflection over all 9 *analyticsRecomputer fields, plus the early-return path in AnalyticsRecomputeIntervals(). [K] The only cross-recomputer read is roles → nodes-clock-skew (GetFleetClockSkew("") inside computeAnalyticsRoles); I grepped every Load() call site and every caller of the Get* readers. On the wake path and the tick path, roles refreshes a paused clock-skew via its recomputeReq (my mutant R2 removing this on the wake path was killed). Roles' compute calls Load() on clock-skew, which counts as a read and keeps it ticking. [A] On the pause-on server, after 8 s idle: roles first read stale (age 133 s), and right afterwards /api/nodes/clock-skew is current age 0, i.e. the dependency was refreshed by the roles wake.

Concurrency. [K] Load never blocks (atomic load plus a non-blocking send on a cap-1 channel). The publish/check ordering between paused and readSinceCompute is correct, so a read cannot be lost. A wake-up queued during a pass is drained as kick-ignored, because Reset is called after the pass and the stale kick is selected before the next tick. A stale kick can at worst cause one extra pass when tick and kick are ready at the same time; that is bounded and harmless. missedRefresh is cleared only when a snapshot is stored, so the snapshot is never reported fresh while a wake pass runs or after one fails. [T] 64-reader burst → 1 wake pass, all readers served the old snapshot. Startup gate (pauseReadyGate = startup-load-done, sampled before compute), post-load RecomputeNow while paused, distance lazy-build RecomputeNow, panic recovery and shutdown (while paused, during a wake pass, dependency already stopped) are all covered and green. [T] -race -count=20 on all pause tests except SnapshotHeaders: green. Apart from F1, the tests use channel/event synchronisation and no sleeps (the env-gated measure test excepted). [K] The plain pauseWhenIdle/pauseReadyGate fields are written before Start and before the HTTP server starts in main.go, so there is no race in production (the warm-up gate uses atomic.Value; this is less defensive, but fine).

Measurements. [T] I reran PAUSE_IDLE_MEASURE=1 PAUSE_IDLE_WINDOW=10s on the merged tree (interval 100 ms, 4 CPUs):

pauseWhenIdle	traffic	passes	alloc MB	mallocs k	CPU s	read p50	read p99
false	idle	899	470.7	5873	1.51	–	–
false	sparse	898	471.9	5876	1.58	3.6µs	4.9µs
false	visible	899	473.8	5897	1.55	3.6µs	9.6µs
true	idle	0	0.0	1.8	0.00	–	–
true	sparse	91	43.7	544	0.19	12.5µs	16.1µs
true	visible	892	469.2	5848	1.78	7.2µs	18.6µs

This matches the author's table. As the PR says, it is a ~500-transmission fixture, so it shows ratios, not deployment savings.

No new SQL under PacketStore.mu. [T] TestNoSQLUnderStoreLock and TestNoSQLUnderStoreLockDetects pass. [K] The PR adds no SQL at all.

Config docs. [K] config.example.json has pauseWhenIdle: false plus a clear operator note on the freshness trade-off and the headers. The startup log prints the setting. The repo has no Markdown doc for the analytics block, so there is nothing else to update. The customizer exposure is tracked as a rule-8 follow-up in the description.

Invariants

• [T] Each issue acceptance criterion has a test that is red on origin/master (the test file does not compile there: pauseWhenIdle, recomputeTicker, AnalyticsPauseWhenIdle and others are missing) and green on the head and on the merged tree.
• [K] No behaviour change beyond the purpose. cmd/server stays read-only (no DB writes). 0 new map[string]interface{} outside tests; the only new interface{} is the recomputerSnapshot.v field that wraps the existing compute func() interface{}.
• [T] scripts/check-xss-sinks.sh --diff origin/master (run in a detached worktree of the head) is clean; no frontend/CSS files changed, so no hardcoded colours.
• [T] Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml on master, head and the merged tree. No closing keywords in the commit or PR body ("Relates to perf(analytics): opt-in pause of unread snapshot recomputers #492").
• [T] Commit author and committer are dborup <kontakt@meshview.dk> (single commit).

Tests and mutants

Merged tree c571e074 = origin/master ff1acc02 + head; clean merge:

• [T] cmd/server: go vet ./... clean; go test ./... ok (81 s); go test -race ./... ok (1457 s, 0 data races).
• [T] cmd/ingestor: go test ./... ok.
• [T] sh test-all.sh: 266 passed, 0 failed.
• [T] node test-frontend-helpers.js: 719 passed, 0 failed.
• [T] E2E against local servers built from the merged tree, on e2e-fixture.db prepared as in CI (freshen, the CI seed SQL, corescope-migrate, seeds 2073, 199, 245 and 122). Pause off (defaults) and pause on (pauseWhenIdle: true, defaultIntervalSeconds: 3): 205 (15/15), 236 (pass), 226 (15/15; known flake flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256 did not appear), 1306 (23/23), 1657 (5/5), 531 (28/28). Identical in both modes. The PR has no E2E files of its own.
• [T] test-e2e-playwright.js stops at "Version info lives on Perf dashboard" (#navStats wait) after 26 passes. The failure is the same against a server built from origin/master, so it is environmental, as the author also observed.
• [T] Master moved to 157e7831 during the review (perf(store): bounded sharing of observer metadata strings #551, paths: LRU order, NULL pinning tests and per-chunk lruMu (follow-ups to #542) #552; store.go and resolved_index.go, no overlap with this PR's files). The merge is still clean. On that merged tree (9282b6ba), go vet and -race -count=3 over the pause, NoSQLUnderStoreLock, Analytics* and Recompute* tests are green. I did not rerun the full suites there.
• Servers were stopped by port/pid.

CI run 40087997868 (attempt 1): all 15 jobs that ran passed (Go server 1–3, ingestor, libraries, build, JS, scripts, Playwright 1–3, verify jobs, Docker); publish/release/deploy were skipped on the PR. [T] The pause tests ran in server shard 3 (19 pass, Measure skipped).

My own mutants (pause tests only; all applied to a scratch copy and reverted):

#	Mutant	Result
R1	nil result counts as a successful pass	survived → F2
R2	wake path skips the dependency refresh	killed (DependencyNotStarved, SnapshotHeaders)
R3	wake path doesn't restart the ticker	survived → F3
R4	snapshot headers on area-filtered hash-sizes	survived → F4
R5	markRead doesn't record the read (kick only)	killed (ContinuousReadsKeepComputing, DependencyNotStarved)
R6	readSinceCompute cleared after compute instead of before	survived → F5
R7	age header always 0	survived → F6
R8	read counted only when pause is off (inverted opt-in)	killed (4 tests)

All of the mutation-check categories the issue names are killed: ignored opt-in, skipped startup work, lost wake-up and falsely fresh. This combines the author's 19 mutants with my R2, R5 and R8. The survivors are secondary edges.

Not verified

• Savings on a real deployment's data size, GC and allocations; only the small fixture was measured (as the PR says).
• The author's 19 mutants: I did not rerun them; I relied on my own 8.
• The full Go suites, test-all.sh and E2E on the newest merged tree (157e7831 + head); I only ran a targeted recheck there.
• No browser/visual check (no frontend change); no staging or production.

[dborup-agent]

Rapport — CS-pve-agent3 PR#554 runde-2 — head 83b70bc

Review feedback addressed (commit 83b70bcb)

1. F1 (blocking): TestPauseWhenIdle_SnapshotHeaders is now deterministic. The test starts the store's recomputers with one manual ticker per recomputer (new helper startRecomputersWithManualTickers: it swaps newRecomputeTicker only for the duration of StartAnalyticsRecomputers, because each recomputer copies the factory at construction). After signalStartupLoadDone() it waits on each ticker's new resetC for the post-load RecomputeNow pass (fix(analytics): recompute cached snapshots after the complete startup load #116). That Reset is the first one on every ticker, and the loop calls it after runOnce, so once all 9 have arrived no post-load pass is left. With manual tickers no scheduled pass can fire either, so from then on only the test runs passes. No production change and no sleeps.
2. F2: nil result. FailedComputationRecovers now runs as two subtests, panic and nil. In both, the failed wake pass keeps the old snapshot (still stale), the next unread ticks retry, and the recomputer pauses again only after recovering. I agree with the review that the branch is unreachable today (all 9 closures return typed values), but the PR text promises "panic or nil", so it is now pinned.
3. F3: t.Reset after kick-run. WakeServesSnapshotAndCoalesces asserts exactly 1 ticker reset right after the wake pass (manualRecomputeTicker.resets is now asserted).
4. F4: non-default shapes. SnapshotHeaders sets cfg.Areas = {"fyn": {}} and checks that every non-default shape returns 200 with no snapshot headers, in both modes: ?region=SJC, ?area=fyn and ?window=1h for rf, topology and channels; ?region= and ?area= for distance, hash-sizes and hash-collisions; ?area= for nodes/clock-skew. Roles and observers/clock-skew have no non-default shape.
5. F5: read during a pass. New TestPauseWhenIdle_ReadDuringPassCounts: a read while pass 2 is computing gets the previous snapshot, the next tick must compute (tick-run), and the tick after it skips.
6. F6: exact age. SnapshotHeaders injects a clock (snapshotClock, atomic) into all 9 recomputers while every loop is idle, stores new snapshots through RecomputeNow, advances 125.9 s, and requires X-Analytics-Snapshot-Age-Seconds: 125 for both current and stale on every endpoint. The clock swap is race-free: every earlier pass happens-before the post-load Reset / RecomputeNow acks the test waited for, and -race is clean.
7. F7: description. The measurement columns are now "Get* p50/p99" and are defined as in-process Get* latency (one readAllDefaultSnapshots call over the 9 default-shape store readers, not HTTP request latency). The ≈2× slower visible-traffic reads with the setting on are noted as negligible in absolute terms. The test table and mutant table in the description are updated for the changes above.

I don't disagree with any finding. The production code is unchanged in this round. The only commits are a merge of origin/master 157e7831 (e03b52b8, clean) and the one test commit, which touches only cmd/server/analytics_pause_when_idle_test.go (+183/−61).

Evidence: [T] = I ran it, [A] = observed via the HTTP API of a local server I built, [K] = code inspection.

F1 determinism

Run	Result
Old test (head edfdb93d + master), -run SnapshotHeaders$ -count=200	2/200 fail (hash-sizes, roles: current where stale expected) [T]
New test, -count=200 (no -race)	0/200 fail [T]
New test, -race -count=50	0/50 fail, no data race [T]
All pause tests, -race -count=20 / -count=200 / -cpu=1,2,8 -count=50	all green [T]
Test-mutant: the post-load wait removed from the new test	123/200 fail (roles, hash-sizes, hash-collisions, distance, channels) [T]

Mutants (red → green)

Each mutant is applied to the working tree, the pause tests run, and then git checkout restores the file. It is run first against the old test file (edfdb93d) and then against the new one. [T]

#	Mutant	Old tests	New tests
R1	ok = true before if result != nil (nil result counts as a successful pass)	survived	killed: FailedComputationRecovers/nil
R3	t.Reset(r.interval) removed from the kick case	survived	killed: WakeServesSnapshotAndCoalesces
R4	hash-sizes headers on region == "" only (area-filtered gets headers)	survived	killed: SnapshotHeaders
R6	readSinceCompute.Store(false) moved after compute()	survived	killed: ReadDuringPassCounts
R7	age header always 0	survived	killed: SnapshotHeaders
F1	test without the post-load wait	flaky 2/200 (original)	123/200 fail without the wait, 0/200 with it

I did not rerun the 19 round-1 mutants. The test changes only add assertions and a wait, and no existing assertion was removed.

Tests

On 83b70bcb (= origin/master 157e7831 + PR):

• [T] cmd/server: go vet ./... clean; go test ./... ok (64 s).
• [T] cmd/ingestor: go test ./... ok.
• [T] sh test-all.sh: 266 passed, 0 failed.
• [T] node test-frontend-helpers.js: 719 passed, 0 failed.
• [T] bash scripts/check-xss-sinks.sh --diff origin/master: no frontend files changed, nothing to scan. .github/ has no diff against master, so the fork guards are unchanged. No new map[string]interface{}.
• [T] E2E against two local servers built from this tree, on copies of e2e-fixture.db prepared as in CI (freshen, the CI seed SQL, corescope-migrate, seeds 2073, 199, 245 and 122). Pause off (no config) and pause on (pauseWhenIdle: true, defaultIntervalSeconds: 3), identical in both modes: 205 (15/15), 236 (pass), 226 (15/15), 1306 (23/23), 1657 (5/5), 531 (28/28). Servers stopped by pid afterwards.
• [A] Pause on, after ~8 s idle: /api/analytics/roles → stale, age 14; then /api/nodes/clock-skew → current, age 0 (refreshed by the roles wake); the next roles read → current, age 0; /api/analytics/rf?window=1h → no headers. Pause off: no X-Analytics-* headers.

CI

Run 40093933702 on 83b70bcb, attempt 1: all 15 jobs that ran passed. [T]

Job	Result
Go Build & Test	success
Go Server Tests 1/3, 2/3, 3/3	success (the pause tests ran in 3/3: 17 pass, Measure skipped; SnapshotHeaders passed)
Go Ingestor Tests	success
Go Libraries & CLI	success
Verify Go Test Assignments	success
JavaScript Unit Tests & Lint	success
Script & Container Unit Checks	success
Playwright E2E 1/3, 2/3, 3/3	success
Verify E2E Shards & Coverage	success
Validate Docker Image	success
Build & Publish Docker Image	success
Release Artifacts, Deploy Staging, Publish Badges & Summary	skipped (PR)

Neither known flaky test (#271, #301) failed, and no rerun was needed.

Not verified

• The measurement was not rerun. F7 is a wording change only, and the numbers are the ones already in the description.
• The 19 round-1 mutants were not rerun (see above).
• test-e2e-playwright.js (the full suite) was not run locally; CI's three Playwright shards passed.
• No browser or visual check (no frontend change). No staging or production access.

PR stays draft.

[dborup-agent]

Review — CS-pve-agent2 PR#554 — head 83b70bc

Dom: APPROVE med nits — this round resolves F1–F7 from my review of edfdb93d. Production code is byte-for-byte unchanged, and the test delta adds nothing that blocks. One optional test-hygiene nit is listed below.

Evidence: [T] = I ran it, [A] = observed via the HTTP API of a local server I built, [K] = code inspection.

Findings

#	Severity	Finding	Evidence
F1	Resolved	TestPauseWhenIdle_SnapshotHeaders is now deterministic. Each recomputer gets a manual ticker, and the test waits for the first Reset on each one, which is the post-load RecomputeNow pass (#116). After that, only the test runs passes.	[T] new test: 0/200 without -race, 0/50 with -race (no data race), 0/200 under 3 busy-loop CPU hogs. Old test (edfdb93d version) on the same merged tree: 1/200 fail (hash-sizes: current where stale expected), so it was red before and is green now.
F2	Resolved	nil result is pinned (FailedComputationRecovers/nil).	[T] R1 killed
F3	Resolved	Ticker reset after kick-run is asserted (resets == 1).	[T] R3 killed
F4	Resolved	Non-default shapes (region, area, window) are asserted to carry no headers, in both modes.	[T] R4 killed, plus my N4 (headers on nodes/clock-skew?area=) killed
F5	Resolved	New ReadDuringPassCounts pins that the read flag is cleared before compute.	[T] R6 killed
F6	Resolved	Exact age comes from an injected clock (125.9 s → "125").	[T] R7 killed, plus my N1 (round instead of truncate) and N3 (time.Now() instead of rc.now()) killed
F7	Resolved	The measurement is now named "in-process Get* latency, not HTTP request latency", and the ≈2× visible-read cost is noted.	[K] PR description
N1	Nit (test hygiene, optional)	startRecomputersWithManualTickers restores the package-level newRecomputeTicker with a plain assignment after StartAnalyticsRecomputers, not with defer/t.Cleanup. If that call ever panics, the swapped factory leaks into later tests in the same binary, and their recomputers panic with "no manual ticker". This is harmless today: there is no t.Parallel() in cmd/server, and the call does not panic.	[K]
I1	Info	The post-load wait relies on the loop calling t.Reset on the RecomputeNow path. TestRecomputeNowRunsOnLoopAndResetsTicker_116 already pins that behaviour. If it is removed, the new test fails fast with no post-load pass after 5 s and does not hang.	[T] my mutant N5

Answers to the re-review points

F1 determinism. [K] The synchronisation is sound:

• Reset runs on the loop goroutine after runOnce on the recomputeReq path, and recomputeWhenLoaded is sequential. Once the test has received 9 resetC signals, no post-load pass remains.
• With manual tickers, no scheduled tick can fire.
• No recomputer is paused (no tick ever skipped), so reads cannot queue a wake-up.
• runDistanceIndexBuild calls RecomputeNow synchronously.

The rc.now = clock.now swap happens-after each loop's last r.now() read (through the resetC send/receive and the synchronous RecomputeNow) and happens-before the next one (through the recomputeReq send). [T] -race is clean.

F2–F6. [T] Each mutant was applied to a scratch copy of the merged tree and run against the pause tests, first with the round-1 test file (edfdb93d) and then with the new one:

#	Mutant	Round-1 tests	New tests
R1	ok = true before if result != nil	survived	killed: FailedComputationRecovers/nil
R3	t.Reset removed from the kick case	survived	killed: WakeServesSnapshotAndCoalesces
R4	hash-sizes headers on region == "" only	survived	killed: SnapshotHeaders
R6	readSinceCompute.Store(false) moved after compute()	survived	killed: ReadDuringPassCounts
R7	age header always 0	survived	killed: SnapshotHeaders
N1 (mine)	age Rounded instead of truncated	survived	killed: SnapshotHeaders
N2 (mine)	missedRefresh cleared even on a nil result (falsely fresh after a failed pass)	survived	killed: FailedComputationRecovers/nil
N3 (mine)	age measured with time.Now() instead of rc.now()	survived	killed: SnapshotHeaders
N4 (mine)	nodes/clock-skew headers for any area	survived	killed: SnapshotHeaders
N5 (mine)	t.Reset removed from the RecomputeNow case	survived (pause tests only)	killed: SnapshotHeaders (fails fast); also killed by TestRecomputeNowRunsOnLoopAndResetsTicker_116

The scratch copy was restored and diffed clean afterwards.

F7. [K] The description's Measurement section names the columns "Get* p50/p99" and defines them as in-process Get* latency over the 9 default-shape readers, explicitly "not HTTP request latency".

Production code unchanged. [T] For all 7 non-test files in the PR (analytics_pause_when_idle.go, analytics_recomputer.go, config.go, main.go, role_analytics.go, routes.go, config.example.json), the PR's added and removed lines are identical between round 1 (9080b6aa..edfdb93d) and round 2 (157e7831..83b70bcb). The merge commit e03b52b8 has the same tree as the automatic git merge-tree edfdb93d 157e7831 (9282b6ba), so no hand-resolved hunks are hidden in it. 83b70bcb touches only cmd/server/analytics_pause_when_idle_test.go. The newRecomputeTicker/now seams the test uses already existed in round 1.

Delta introduces anything new? Nothing beyond the nit N1 above. [K] The new resetC (cap 16, non-blocking send) cannot block the loop. snapshotClock is atomic. cfg.Areas is set on the test server only.

Invariants

• [T] Every acceptance criterion of perf(analytics): opt-in pause of unread snapshot recomputers #492 has a test that is red before and green after. The test file does not compile against origin/master, and the new edges are red against the round-1 tests (see the mutant table).
• [K] No behaviour change beyond the purpose: production code is unchanged since round 1, which I already reviewed. cmd/server stays read-only.
• [T] 0 new map[string]interface{} outside tests.
• [T] No CSS/JS/HTML changes, so there are no hardcoded colours. scripts/check-xss-sinks.sh --diff origin/master (in a detached worktree of the head) reports nothing to scan and exits 0.
• [T] Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml on origin/master, the head and the merged tree. .github/ is identical to master.
• [T] No closing keywords in the commit messages or the PR body ("Relates to perf(analytics): opt-in pause of unread snapshot recomputers #492").
• [T] Author and committer of edfdb93d, e03b52b8 and 83b70bcb are dborup <kontakt@meshview.dk>.
• [T] git ls-remote showed the head at 83b70bcb before and after the review.

Tests

Merged tree 19787ba3 = origin/master 6aa03db3 (includes #555) + head. The merge is clean.

• [T] cmd/server: go vet ./... clean; go test ./... ok (65 s).

• [T] cmd/ingestor: go test ./... ok.

• [T] sh test-all.sh: 266 passed, 0 failed.

• [T] node test-frontend-helpers.js: 719 passed, 0 failed.

• [T] All pause tests: -race -count=20 -cpu=1,4 green; -count=100 -cpu=1,2,8 green.

• [T] E2E against two local servers built from the merged tree, each on its own copy of e2e-fixture.db prepared as in CI (freshen, the CI seed SQL, corescope-migrate, seeds 2073 and 199). One server ran with pause off (no config), the other with pause on (pauseWhenIdle: true, defaultIntervalSeconds: 3). Results were identical in both modes: 205 (15/15), 236 (pass), 226 (15/15; known flake flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256 did not appear), 1306 (23/23), 1657 (5/5), 531 (28/28). The PR has no E2E files of its own.

• [A] Pause on, after the E2E runs and ≥8 s idle:

  • /api/analytics/roles → stale, age 391;
  • then /api/nodes/clock-skew → current, age 0 (the roles wake refreshed it);
  • the next roles read → current, age 0;
  • /api/analytics/rf?window=1h → no headers.

  Pause off: no X-Analytics-* headers.

• Both servers were stopped by the pid found on their port, and the ports are free again.

CI: run 40093933702 on 83b70bcb, attempt 1, pull_request. [T] Every job that ran passed:

• Go Build & Test, Go Server 1/3–3/3, Go Ingestor, Go Libraries & CLI
• JS Unit & Lint, Script & Container
• Playwright 1/3–3/3, Verify E2E Shards & Coverage, Verify Go Test Assignments
• Validate Docker Image, Build & Publish Docker Image

Publish Badges, Release Artifacts and Deploy Staging were skipped (PR). In shard 3, all 22 pause tests and subtests passed, including SnapshotHeaders and FailedComputationRecovers/{panic,nil}, and Measure was skipped. Known flakes #256 and #267 did not appear, and nothing was re-run.

Not verified

• I did not rerun the author's 19 round-1 mutants. Production code is unchanged and no assertion was removed (the only removed assertion is the loose age check, now replaced by the exact one).
• I did not rerun the measurement (F7 is a wording change only).
• I did not run go test -race ./... over the whole cmd/server package this round, only the pause tests under -race.
• I did not run the full test-e2e-playwright.js locally, and I did not apply seeds 245 and 122 this round. CI's Playwright shards passed.
• No browser or visual check (no frontend change). No staging or production access.