perf(server): remove per-candidate SQL and JSON parsing from /paths and /hop_analytics

[dborup]

Summary

/api/nodes/{pk}/paths (and /hop_analytics, which shares the same code) spend almost all of their time on two avoidable costs. This PR removes both without changing what the endpoints return.

Evidence

A 60 s CPU profile (pprof) of a CoreScope v0.2.0 instance under a synthetic load (50 websocket clients, heavy endpoints at 20x production rate):

	CPU time	share of 89 s total
handleNodePaths	67.6 s	76%
confirmResolvedPathContains (SQLite)	38.1 s	43%
pathLen (json.Unmarshal)	25.9 s	29%

confirmResolvedPathContains is called by handleNodePaths (90.5%) and GetNodeHopAnalytics (9.5%). In the allocation profile, pathLen accounts for 6.8 GB of 42 GB allocated in about 12 minutes (16%), i.e. GC churn on top of the CPU cost. On a 2-vCPU host this is enough for a handful of concurrent /paths calls to saturate the box.

1. N+1 SQL. Every candidate transmission admitted by the hash index was confirmed with its own SELECT COUNT(*) ... INSTR(LOWER(resolved_path), ?), run sequentially, each scanning all observation rows of that transmission.
2. pathLen parses the whole array just to count it. fetchResolvedPathForTxBest calls it for every observation of every candidate (about 17 per transmission) and each call does a full json.Unmarshal into []interface{}.

Changes

• pathLen: allocation-free fast path for arrays of plain ASCII strings (the shape real observations have). Anything outside that grammar (escapes, non-ASCII, control bytes, nested values, numbers, null, trailing data, malformed input) falls back to the original json.Unmarshal implementation (pathLenSlow), so results are identical for every input.
• handleNodePaths: the per-candidate SQL confirmation is no longer run up front. It only guards against hash collisions and a stale index. For every candidate that has a canonical persisted resolved_path, membership is decided again later from that exact path (resolvedPK == lowerPK), which yields the same answer. The query is now issued only for candidates with no canonical path, where the legacy fallback arm still consumes confirmedBySQL.
• GetNodeHopAnalytics: drops the same pre-filter; its loop already skips rp == nil and idx < 0.
• The index membership list for the queried pubkey is converted to a set once, instead of being scanned for every candidate (was O(candidates x list length)).
• confirmResolvedPathQueries counter so tests can assert the N+1 is gone.

Why results are unchanged

If a candidate is rejected by the old SQL check, no observation of that transmission has the pubkey in its resolved_path. The canonical path is one of those observations' resolved_path, so it cannot contain the pubkey either, and the later check rejects it. If the old check accepted it, the later check decides anyway. The only behavioural difference is that a few rejected candidates now go through the canonical-path fetch before being dropped.

Test plan

• go vet ./... and go test ./... -count=1 in cmd/server (full suite passes)
• go test -race on the touched paths (NodePaths, HandleNodePaths, HopAnalytics, ResolvedIndex, PathLen)
• TestPathLenFast_MatchesReference / _RandomisedAgainstReference: fast path vs the original implementation on a fixed corpus plus 300k randomised near-valid inputs; _WellFormedPathsStayOnFastPath pins zero allocations
• TestNodePaths_NoPerCandidateSQLConfirm: 25 candidates, 0 confirmation queries (was 26 on master)
• TestNodePaths_StaleIndexEntryStillExcluded: a hash-index entry pointing at a tx whose stored path lacks the pubkey is still excluded from /paths and /hop_analytics
• TestNodePaths_NoCanonicalPathStillConfirmedBySQL: the fallback arm keeps its SQL confirmation
• Verified the three new tests fail against master's handleNodePaths

Measurements

Microbenchmark (BenchmarkPathLen, ["aa","bb","cc","dd","ee"]):

	ns/op	allocs/op
reference (json.Unmarshal)	467	18 (544 B)
fast path	15	0

End-to-end on a synthetic dataset (4,000 transmissions x 8 observations, in-memory SQLite, same machine, master vs this branch):

	master	this PR
/paths cold LRU	~52 ms	~17 ms
/paths warm	~39 ms	~3-4 ms
/hop_analytics	~38 ms	~2-3 ms

The in-memory test database has no disk I/O, so this understates the gain on a real deployment where the confirmation queries read scattered rows from a multi-GB file. I have not yet measured this on production-sized data; that is the next step.

Not in this PR (follow-ups)

• fetchResolvedPathForObs still issues one single-row query per candidate on a cold LRU (about 4 s of the 89 s profile).
• Startup load: loadChunk / resolvePathForObsColdLoad allocate about 31 GB to build a 3 GB live heap (separate optimisation).

🤖 Generated with Claude Code

[dborup]

Note for the review — Playwright failure on the re-run

The re-run (run 37324912879, merge with master 0572e7f9, which already contains the #244 fix from #252) failed only in test-issue-1122-details-row-clamp-e2e.js, at 1200 and 900 px:

advert link in Details is not hit-testable: {"found":true,"hitIsLink":false,"text":"R5-D4 300D Rak"}

Every other PR run since #252 passed this test, and so did master. Please establish whether this PR causes it (for example through changed data or timing in the packets view), or whether it is a remainder of the flake. Run the test at least 5 times each on the merged tree and on master.

[dborup-agent]

Review — CS-pve-agent3 PR#246 paths-cpu — head 45dc081

Dom: APPROVE med nits

This is an independent, read-only review. Evidence tags: [T] means a test, probe, benchmark or command I ran in this review. [A] means analysis or reading of code. [K] means taken from the PR description or comments, and not re-checked here.

The change does what it says. On the CI-prepared fixture and on a 20× scaled copy of it, /paths and /hop_analytics return byte-identical output before and after for all 202 nodes, both cold and warm. Both endpoints get much faster (about 6–10× on the scaled copy). Every mutant I ran was caught. The red Playwright run is not caused by this PR: CI checked out a merge with an old master, and the failure is the pre-#244 version of the test, which I reproduced on that base without this PR. There is one narrow state where the old and new results differ (finding 1). It does not block the merge, but the description's "identical for every input" claim should be softened.

Findings

#	Severity	Finding	Evidence
1	Minor (claim precision)	Counterexample to "results unchanged": a stale resolved-path LRU. The old pre-filter read the current DB row. The canonical path comes from fetchResolvedPathForTxBest, which serves from apiResolvedPathLRU first, and nothing ever invalidates that LRU when the stored path changes (lruDelete has no non-test callers). The ingestor's observation upsert can replace a stored resolved_path (ON CONFLICT … resolved_path = COALESCE(excluded.resolved_path, resolved_path), cmd/ingestor/db.go), and the in-memory hash index is not rebuilt for that. In that state, master excludes the tx and head includes it, in both /paths and /hop_analytics. Head's answer matches what the packets page shows for the tx, because that page uses the same LRU. So this arguably makes the endpoints more consistent, and the root cause, a never-invalidated LRU, already existed. It does contradict "results are identical for every input" in the description. Suggest softening that sentence, or pinning the behaviour in a test.	Probe TestReviewProbe246_StaleLRUAfterResolvedPathRewrite: seed a tx whose path contains the target, warm the LRU via /paths, rewrite the row's resolved_path to a path without the target. Then: origin/master in /paths=false in /hop_analytics=false; merged head in /paths=true in /hop_analytics=true [T]
2	Info (CI)	The red Playwright run tested an old base. The run 37324912879 log shows HEAD is now at b1c081dc Merge 45dc081b… into 2a7877ac…, which is the branch's merge-base and not 0572e7f9. 2a7877ac does not contain the #244 fix 0e8f6569 (git merge-base --is-ancestor → no) [T]. The failure text (advert link in Details is not hit-testable: {"found":true,…}) is the old test's message; master's test now reports N/M advert links … are not hit-testable. In CI the test ran about 14 min after freshen-fixture.sh, which is the #244 condition. To get a CI result on current master, the branch needs master merged in (your call); re-running the same run will not change the base.	Local reproduction in the next section [T]
3	Nit (efficiency)	Candidates that only the old SQL check rejected (hash collisions, stale index entries) now go through fetchResolvedPathForTxBest. That is one PK lookup each on a cold LRU, plus the multi-row fallback when the longest observation has no stored path. Each such lookup also adds an entry to the 10k-entry FIFO LRU for a tx that does not belong to the node. The description mentions the first part. The measured net effect is still clearly positive (below), so this is just worth knowing for very collision-heavy prefixes.	[A] + timings [T]
4	Nit (tests)	TestPathLenFast_MatchesReference/_RandomisedAgainstReference are equivalence guards and pass trivially on a harness without a fast path. TestNodePaths_StaleIndexEntryStillExcluded/_NoCanonicalPathStillConfirmedBySQL fail on master only because of their query-count assertions; their exclusion assertions pass on master. That is the right design (regression guards), but "the three new tests fail against master" is mostly about the counter.	Red/green table below [T]

1. Correctness: does the "no change" argument hold?

• Argument [A]:

  • old result = {candidate c : index pre-check ∧ (hasReverse ⇒ SQL(c)) ∧ later(c)};
  • new result = {c : index pre-check ∧ (hasReverse ∧ no canonical ⇒ SQL(c)) ∧ later(c)}.

  They can differ only for a c that has a canonical path, where SQL(c) = false and later(c) = true. In the canonical arm, later(c) needs resolvedPK == lowerPK within the first len(hops) entries of the canonical path (/paths), or EqualFold anywhere in it (/hop_analytics). The canonical path is one observation's resolved_path, read from SQLite or the LRU. If it comes from the DB, SQL(c) is true, which is a contradiction, so the argument holds. If it comes from a stale LRU entry, it can differ (finding 1). JSON escapes such as a inside a stored pubkey would also break INSTR(LOWER(…), '"pk"') against json.Unmarshal, but the ingestor writes plain hex through json.Marshal, so I consider that unreachable.

• Test on the fixture [T]: master and the merged head ran side by side on separate copies of the CI-prepared e2e-fixture.db (500 tx, 399 observations with resolved_path). I fetched /api/nodes/{pk}/paths and /hop_analytics?days=3650 for all 202 nodes, normalised with jq -S and sorted paths by sampleHash (ties in the handler's sort fall back to map order). Cold pass: 404/404 identical. Warm pass: 404/404 identical. 146 nodes have non-empty paths and hop packets, so the hash-index arm is exercised.

• Same comparison on a 20× scaled copy (9,981 tx, 79,843 observations, 63,844 with resolved_path; synthetic copies of every fixture tx with distinct raw_hex, 8 observers each): cold 404/404, warm 404/404 identical [T].

2. pathLen fast path and the new tests: red on master, green on head

Master harness: origin/master plus only the counter field and its increment, pathLenSlow = pathLen and a pathLenFast stub that always falls back. No behaviour change [T].

Test	master harness	merged head
TestPathLenFast_MatchesReference	pass (trivial)	pass
TestPathLenFast_RandomisedAgainstReference	pass (trivial)	pass
TestPathLenFast_WellFormedPathsStayOnFastPath	FAIL (11 allocs/call)	pass
TestNodePaths_NoPerCandidateSQLConfirm	FAIL (26 queries for 25 candidates)	pass (0)
TestNodePaths_StaleIndexEntryStillExcluded	FAIL (count 4, exclusion holds)	pass
TestNodePaths_NoCanonicalPathStillConfirmedBySQL	FAIL (count 2, exclusion holds)	pass

Fast-path grammar [A]:

• whitespace is exactly JSON's four characters;
• strings must be printable ASCII without a backslash;
• empty input, null, a BOM, numbers, nesting and trailing data all fall back.

On top of the PR's corpus and its 300k randomised inputs, I ran a 60 s native Go fuzz of pathLen against pathLenSlow: about 450k executions and no divergence [T].

3. Performance

BenchmarkPathLen (["aa","bb","cc","dd","ee"], -count 5, 4-vCPU box, idle) [T]:

	ns/op	B/op	allocs/op
before (master pathLen, json.Unmarshal)	980–1024	456	16
after (fast path)	24.3–26.0	0	0

That is about 40× faster. My absolute numbers are about twice the PR's (467 → 15 ns), which is the machine; the ratio matches.

Endpoint timings against a local server (one server at a time, each started fresh on its own DB copy). Cold means the first request per node after start (empty LRU); warm means 3 more passes; hop is 3 passes of /hop_analytics?days=3650. All 202 nodes, sequential curl [T]:

Dataset	Endpoint	master mean / p50 / p95 / max (ms)	PR mean / p50 / p95 / max (ms)
fixture (500 tx)	/paths cold	0.92 / 0.72 / 1.97 / 7.8	0.67 / 0.56 / 1.08 / 4.4
fixture	/paths warm	0.92 / 0.72 / 2.06 / 5.1	0.63 / 0.57 / 1.06 / 2.2
fixture	/hop_analytics	0.83 / 0.70 / 1.64 / 8.9	0.51 / 0.50 / 0.69 / 0.85
scaled (9,981 tx / 79,843 obs), run 1	/paths cold	10.08 / 4.45 / 31.2 / 213	1.70 / 0.86 / 3.86 / 53
scaled, run 1	/paths warm	9.32 / 3.93 / 30.4 / 164	1.23 / 0.77 / 3.18 / 12.6
scaled, run 1	/hop_analytics	8.76 / 3.72 / 27.7 / 147	0.83 / 0.63 / 1.85 / 6.8
scaled, run 2	/paths cold	9.26 / 3.71 / 28.1 / 186	1.79 / 0.86 / 4.01 / 63
scaled, run 2	/paths warm	9.75 / 4.32 / 32.2 / 179	1.42 / 0.95 / 3.76 / 13.6
scaled, run 2	/hop_analytics	9.38 / 3.93 / 32.8 / 149	0.86 / 0.66 / 1.88 / 11.8

The scaled data is synthetic and local-disk, with no production-sized file. As in the PR, this probably understates the I/O saving on a multi-GB DB.

4. Rules

• cmd/server stays read-only. The only SQL change is a counter around an existing SELECT, and readonly_invariant_test.go is in the green suite [T][A].
• No new map[string]interface{}: the non-test diff adds 0 [T].
• Locks:
  • handleNodePaths builds indexedForTarget under s.store.mu.RLock. That is O(len(index list)) once, replacing the O(candidates × list) scan that ran under the same lock.
  • The deferred SQL confirmation and the canonical fetches run between RUnlock and the second RLock, and the lock order is unchanged (lruMu never under mu).
  • GetNodeHopAnalytics follows the same pattern.
  • The in-place filters (kept := candidates[:0]) only alias the handler's local slice, not byPathHop [A].
  • go test -race on the touched tests and the full race suite are green [T].
• Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml on the merged tree, with no .github/ change [T].
• The single commit is authored and committed by dborup <kontakt@meshview.dk> [T].
• check-xss-sinks.sh --diff: no frontend files changed [T].

Playwright 1122 failure (requested in the PR comments)

Every run below is on its own server with a CI-prepared fixture [T]:

Tree	Test version	Fixture age	Runs	Result
origin/master 0572e7f	fixed (#244)	~45 min	5	5 × 18/18
merged tree (0572e7f + head)	fixed (#244)	~45 min	5	5 × 18/18
both of the above	fixed	~55 min	1 each	18/18
PR head as-is (= CI's merge, base 2a7877a)	old	< 13 min	7	7 × 18/18
PR head as-is	old	~22 min	2	2 × 16/18, the exact CI failure: [desktop-1200]/[tablet-900] {"found":true,"hitIsLink":false,"text":"R5-D4 300D Rak"}
base 2a7877a without this PR	old	~22 min	2	2 × 16/18, same steps

The PR changes no frontend file, and public/ plus the test are identical between master and the merged tree [T]. Verdict: a remainder of #244, caused by the stale CI base and not by this PR.

Tests (merged tree = origin/master 0572e7f + head, tree 49cb9bf3, git archive copies)

• cmd/server: go test -race -count=1 ./... ok (ran 16 min on an idle box) [T].
• sh test-all.sh: 219 passed, 0 failed. node test-frontend-helpers.js: 707 passed, 0 failed [T].
• The Go jobs in CI run 37324912879 also used the old base (finding 2).

Mutants (mine, each applied to a fresh copy and reverted; targeted PathLen|NodePaths|HopAnalytics|ResolvedIndex|Paths tests) [T]:

Mutant	Result
B1 fast path accepts a backslash inside strings	caught: _RandomisedAgainstReference
B3 trailing data after ] ignored	caught: _MatchesReference, _Randomised…
B4 no SQL confirmation for no-canonical candidates (trust the index)	caught: _NoCanonicalPathStillConfirmedBySQL
B6 /hop_analytics keeps a candidate whose path lacks the target	caught: _StaleIndexEntryStillExcluded
B7 /paths canonical arm trusts the hash index	caught: _StaleIndexEntryStillExcluded, …AnchorBiasInconsistency_Issue1278
B8 index membership set built for the wrong key	caught: 8 tests

Not verified

• Behaviour and timings on production-sized data, or on a multi-GB DB file. Only the fixture and a synthetic 20× copy were measured.
• The pprof profile in the description [K].
• Concurrency under load (many parallel /paths calls); I measured sequential requests only.
• A CI run on the current master base (finding 2).