[adminopenclaw8-sketch]

Relates to #489

Summary

Every observation the store holds carries its observer's ID (a 64-hex pubkey), name, IATA code and direction. All four come out of a SQL join, so each row scan allocated a fresh copy of each. The store now hands out one canonical copy of each value from a bounded, sharded pool, so equal values share immutable storage.

On the measured 2.5M-observation shape of the large instance that is 206.9 MiB of logical string content against 5.4 KiB of distinct values: 140 distinct values that were held in 9,460,013 separate copies. Heap after load drops from 1562.8 to 1321.4 MiB (-241 MiB, -15.4 %) for +1.2 % on the load.

trackedBytes now holds the actual retention rather than one logical copy per observation, and the pool's own bounded retention is accounted separately, so the memory budget stays honest.

Plan (four commits)

1. Measure first (below), on origin/master e08072be.
2. Red commit test(store): … (20aef204): the tests, the measurement harness, the benchmarks and the full plumbing, with intern() stubbed to return every value as the row scan handed it over — exactly what master does. That commit produces the "before" numbers and is red on every sharing, GC, saturation and accounting assertion.
3. Green commits perf(store): … (25472d3d, 5fdcf595): the real lookup, then the sharding the parallel benchmark asked for.
4. Test fix test(store): … (53682d4f): the lock-discipline test, after two mutants survived its first version.

#411 (PR #527) and #413 (PR #529) are both merged and were read first: the dedup helpers in obs_dedup.go, the five StoreObs constructors and the exact trackedBytes/txChargedBytes accounting are all honoured, and none of them is reimplemented here.

No schema, API, deployment or workflow change. signatureValid and the decoded-cache contract are untouched.

Measurement first

Measured on this branch with intern() stubbed out (the red commit) against the real pool, same generator, same data, one run each. copies is the number of distinct string data pointers the observations hold for the four fields — i.e. how many separate copies of those ~140 values are actually in memory.

test-fixtures/e2e-fixture.db (499 transmissions, 499 observations, 22 observers; its direction column is NULL throughout):

	before	after
observer metadata, logical	41,425 B	41,425 B
distinct values / their bytes	48 / 1,770 B (95.7 % duplicate)	same
copies held	1,497	48
heap after load	11.78 MiB	11.74 MiB
trackedBytes	1.54 MiB	1.50 MiB

The fixture is far too small to choose a capacity from (41 KB of metadata in total), so the synthetic set below is the basis. obs489Data builds the shape of the large instance: 65 observers (64-hex IDs, 18–26 byte names, 3-letter IATA codes with every seventh observer having none, RX/TX/none directions) and the production observations-per-transmission distribution (log-normal, mean 20.6, p50 15, p90 42, p99 96).

observations	250,000	1,000,000	2,500,000
transmissions	12,152	48,519	121,115
observer metadata, logical	20.69 MiB	82.76 MiB	206.90 MiB
distinct values / their bytes	140 / 5,381 B	140 / 5,381 B	140 / 5,381 B
duplicate share	99.98 %	99.99 %	100.00 %
copies held, before → after	945,889 → 140	3,783,971 → 140	9,460,013 → 140
heap after load, before → after	166.1 → 141.9 MiB (−14.6 %)	642.2 → 545.3 MiB (−15.1 %)	1562.8 → 1321.4 MiB (−15.4 %, −241 MiB)
trackedBytes, before → after	158.2 → 137.6 MiB	633.2 → 550.4 MiB	1583.5 → 1376.6 MiB
Load wall time, before → after	1360 → 1390 ms	6802 → 6720 ms	23025 → 23062 ms
pool's own retention	5,381 B + 140 × 208 B = 34.5 KB	same	same

The heap drop is larger than the 207 MiB of logical content because Go rounds each allocation to a size class: a 64-byte ID costs 64 B, a 18–26 byte name 32 B, a 3-byte IATA and a 2-byte direction 8 B each. Startup time is unchanged within run-to-run noise (the controlled before/after is the benchmark below: +1.2 %).

trackedBytes/heap moved from 1.013 to 1.042 at 2.5M observations, i.e. the accounting still tracks the heap and still over-charges slightly, so eviction triggers earlier rather than later. TestTrackedBytesTracksTheHeap_113's 0.85–1.15 bound passes.

Change

cmd/server/obs_strings_pool.go (new):

• obsStringPool is a bounded canonical table owned by the PacketStore, not a package global, so every store (and every test) is independent and a store built as &PacketStore{} simply gets a nil pool, which shares nothing and breaks nothing.
• intern(s) returns the canonical copy and whether it is shared. A hit takes only its shard's read lock and allocates nothing. A miss clones the value, so a short string scanned out of a large buffer cannot pin that buffer.
• The copies are held for the life of the store. That is what makes the sharing survive a garbage collection — a weak table (unique.Make) hands back a second copy for a value interned again after a GC.
• The table is bounded by obsStringPoolMax entries, across 16 independently locked shards, with the count reserved in one shared counter before a slot is taken so the cap is exact. Past the cap a value is returned exactly as it came in: unshared, correct, never rejected.
• Nothing is ever replaced in place. A renamed observer is a different value with its own entry, so observations heard before the rename keep the name they were heard with (Go strings are immutable and no path writes an observation's observer strings after construction).

All five StoreObs constructors intern:

Constructor	Where it interns	Under s.mu?
Load (hot cold load)	the row scan	yes — Load holds the write lock for its whole scan and nothing else runs
scanAndMergeChunk (LoadChunked)	the row scan	yes — a chunk brings at most a handful of values the pool does not hold
loadChunk / loadChunkWhere (background fill)	the row scan, into the local batch	no
IngestNewFromDB	the row scan, into tempRow.strs	no
IngestNewObservations	the row scan, into obsRow.strs	no

The #215 hash merge moves existing observations, so it creates no strings: they keep their canonical copies and their charges, and a dropped duplicate is credited exactly as before.

Accounting: logical charge vs. actual shared bytes

• StoreObs gains sharedStrs uint8 in the padding wireRoute already sits in, so StoreObs stays 200 bytes (TestStoreObsLayoutFitsSharedStrsInPadding_489). It records which of the four fields the pool holds.
• estimateStoreObsBytes charges an observation only the metadata bytes it owns (ownedObsStringBytes). A shared field costs it just the string header, which storeObsBaseBytes already covers. The charge stays a pure function of the observation, so eviction credits exactly what ingest charged and trackedBytes still returns to exactly 0 when the store empties.
• The pool's own retention is charged once, separately: accountedBytes() = trackedBytes + obsStrPool.retainedBytes(). That is the figure trackedMemoryMB() reports and the figure the two memory watermarks in evictionCandidateTxIDs / evictStaleInternal compare against. The pool term is never credited by eviction — the canonical copies outlive the observations that share them — so it behaves as a floor the budget must respect rather than as something an eviction pass can free.
• Intentionally conservative: the per-entry charge is obsStringEntryBytes = 208 plus the value's own bytes. Measured on Go swiss maps, a table filled to capacity costs 80 B per entry and the steady state (140 entries in tables sized for fewer) costs 208 B per entry, because a table's footprint is fixed below its hint. The higher figure is charged, so the charge is an upper bound at every fill: at the cap it holds 2.2 MB of charge for about 0.7 MB of real retention. Over-charging can only make eviction trigger earlier, never later. TestObsStringPoolRetainedBytes_489 pins both directions against a measured pool.
• estimateStoreTxBytesTypical (the cold-load budget estimate) no longer carries the per-observation observer-ID and name terms, for the same reason.
• GetStoreMemoryBreakdown's ObsStringsMB keeps its meaning and its shape — it was already documented as a deliberate upper bound — with a note that it is now the loosest of the components, since it counts the same few hundred values once per observation. TrackedMB is the figure that reflects the real retention. No response field was added or changed.

Capacity policy, from the numbers

The steady state is one entry per distinct value: 140 on the large instance (65 observer IDs + 65 names + 8 IATA codes + 2 directions) and 48 on test-fixtures/e2e-fixture.db (22 IDs + 22 names + 4 IATA codes; its direction is NULL throughout). obsStringPoolMax = 8192 is 58x that, which leaves room for observer growth and for renames — a rename adds one entry and the old name stays, so observations heard before it keep sharing their own — and bounds the pool's own retention to 2.2 MB of charge (about 0.7 MB in fact) even if something floods it with distinct values. Past the cap nothing breaks: the values come back unshared and the observations are charged for the bytes they then own (TestObsStringsSaturatedPoolStillIngests_489).

16 shards comes from BenchmarkObsStringPoolHitParallel, below.

Tests

New: cmd/server/obs_strings_pool_489_test.go (pool properties), obs_strings_paths_489_test.go (store paths), obs_strings_489_data_test.go (generator + measurement harness, compiles on master), obs_strings_489_bench_test.go.

Requirement (#489)	Test	Red on the red commit
One copy of each value, shared	ObsStringPoolSharesOneCopy, ObsStringsSharedOnEveryConstructionPath	yes
Clone, no substring pinning (oversized backing buffer)	ObsStringPoolClonesNewValues (2 bytes out of a 1 MiB buffer)	yes
Sharing survives GC	ObsStringPoolSurvivesGC, ObsStringsSharingSurvivesGC (ingest after a GC shares the cold load's copies)	yes
Pool saturation falls back to normal strings, no rejection	ObsStringPoolSaturationFallsBackToPlainStrings, ObsStringsSaturatedPoolStillIngests, ObsStringPoolChurnStaysBounded (2x the cap in distinct values)	yes
Concurrent calls get the same copy (-race)	ObsStringPoolConcurrentCallsShareOneCopy (16 goroutines), ObsStringsConcurrentIngestAndReaders	yes
All construction paths	ObsStringsSharedOnEveryConstructionPath over hot Load, LoadChunked, background fill, ingest and upsert — same observations, same totalObs, same trackedBytes, same pool retention on all five	yes
Merge keeps sharing and charges	ObsStringsHashMergeKeepsSharingAndCharges (#215 migration, batch 2)	yes
A metadata change does not touch older observations	ObsStringsMetadataChangeLeavesOlderObservations	yes
Logical charge separated from shared bytes; pool retention accounted	ObsStringsTrackedBytesHoldsSharedBytesOnce	yes
Eviction credits exactly; trackedBytes back to 0	ObsStringsEvictionCreditsExactly (all five paths), plus acct113Check everywhere	yes
Entry accounting is an upper bound on real retention	ObsStringPoolRetainedBytes	yes
A hit allocates nothing	ObsStringPoolHitDoesNotAllocate (AllocsPerRun == 0)	yes (mask half)
Nil pool and empty strings	ObsStringPoolEmptyAndNil	yes (mask half)
StoreObs does not grow	StoreObsLayoutFitsSharedStrsInPadding (200 bytes)	green (preservation guard)
No pool contention under PacketStore.mu (#495)	ObsStringPoolNeverBlocksStoreWriters: every shard locked, then an ingest that must intern a fresh value runs — a store writer must still get s.mu	green (master never touches the pool); its teeth come from mutants M15 and M16

22 of the 26 _489 test and subtest results are red on the red commit. The four that pass are the lock-discipline test (plus its two subtests) and the StoreObs layout guard, both marked above as preservation guards.

Mutants

18 run, 18 killed. Each was applied to the green tree, go test -run _489 was run, and the tree restored.

Mutant	Killed by
M1 intern reports shared without consulting the table	15 tests, incl. SharesOneCopy, SharedOnEveryConstructionPath, TrackedBytesHoldsSharedBytesOnce
M2 the value is retained as it came, no clone	ClonesNewValues, RetainedBytes
M3 the canonical copy is not kept in the table	SurvivesGC, SharesOneCopy, ClonesNewValues, SharedOnEveryConstructionPath, +9
M4 no cap: the table grows without bound	ChurnStaysBounded, SaturationFallsBackToPlainStrings, SaturatedPoolStillIngests
M5 a full pool drops the value (returns "")	ChurnStaysBounded, SaturationFallsBackToPlainStrings, SaturatedPoolStillIngests
M6 the shared mask misses the name	EmptyAndNil, HitDoesNotAllocate, MetadataChangeLeavesOlderObservations, HashMergeKeepsSharingAndCharges, SharedOnEveryConstructionPath, TrackedBytesHoldsSharedBytesOnce
M7 the per-observation charge ignores the mask	EmptyAndNil, HashMergeKeepsSharingAndCharges, SharedOnEveryConstructionPath, TrackedBytesHoldsSharedBytesOnce
M8 the pool's own retention is not accounted (accountedBytes drops it)	EvictionCreditsExactly, TrackedBytesHoldsSharedBytesOnce
M9 the entry charge forgets the value's own bytes	RetainedBytes
M10 hot Load does not intern	SharedOnEveryConstructionPath, EvictionCreditsExactly, SharingSurvivesGC, MetadataChange…, HashMerge…, TrackedBytes…
M11 scanAndMergeChunk (LoadChunked) does not intern	SharedOnEveryConstructionPath, EvictionCreditsExactly
M12 the background fill (loadChunk) does not intern	SharedOnEveryConstructionPath
M13 IngestNewFromDB does not intern	SharedOnEveryConstructionPath, EvictionCreditsExactly, ConcurrentIngestAndReaders
M14 IngestNewObservations does not intern	SharedOnEveryConstructionPath, SharingSurvivesGC, MetadataChange…, ConcurrentIngestAndReaders
M15 IngestNewObservations interns under s.mu instead of at the row scan	NeverBlocksStoreWriters/IngestNewObservations (probe 0)
M16 IngestNewFromDB interns under s.mu instead of at the row scan	NeverBlocksStoreWriters/IngestNewFromDB (probe 0)
M17 the table matches values by their first 4 bytes, not exactly	MetadataChangeLeavesOlderObservations (the rename shares a prefix with the old name on purpose), SharedOnEveryConstructionPath, +6
M18 sharedStrs does not use existing padding (StoreObs grows to 208 bytes)	StoreObsLayoutFitsSharedStrsInPadding (and the pre-existing #76 layout test)

M15 and M16 are what the lock-discipline test exists for, and finding that they survived the first version of that test is why it now asserts the ingest is really parked before probing s.mu, and probes twenty times (commit 53682d4f).

Benchmarks (before = the red commit, which runs master's store code)

BenchmarkLoad_Obs489, a hot Load of 250,000 observations over 12,152 transmissions (production distribution: mean 20.6 observations per transmission, p50 15, p90 41, p99 95), -benchtime 3x -count 6, Go 1.27.0, darwin/arm64, 10 cores. Medians of 6:

	before (20aef204)	after	delta
sec/op	1.369 s	1.386 s	+1.2 %
heap after load	166.1 MiB	141.9 MiB	-14.6 %
trackedBytes	158.2 MiB	137.6 MiB	-13.0 %
B/op	547.24 MB	547.27 MB	~
allocs/op	13.056 M	13.057 M	~

The +1.2 % on the load is the four pool lookups per observation (1 M of them here): BenchmarkObsStringInternObs measures them at 39 ns per observation, i.e. 39 ms over the 1 M observations of this load, which is the whole difference.

B/op and allocs/op do not move, on purpose — see the perf justification above.

Pool micro-benchmarks

-benchtime 2s -count 3, medians, 140 held values (the steady state):

benchmark	-cpu 1	-cpu 2	-cpu 10
ObsStringPoolHit (one hit)	11.9 ns, 0 allocs	11.4 ns	11.4 ns
ObsStringPoolHitParallel (hits from N goroutines)	11.8 ns	10.7 ns	22.4 ns
ObsStringInternObs (one observation, four fields)	39.1 ns, 0 allocs	39.1 ns	39.0 ns
ObsStringPoolMiss (filling an empty pool to the 8192 cap)	751 µs / 8192 values = 92 ns per miss, 1.50 MB	606 µs	600 µs

Why 16 shards: with a single RWMutex the same parallel hit cost 9 ns alone but 34 ns with two goroutines and 84 ns with ten — the read lock's own cache line, not the map. A per-hit atomic counter made it worse still and was dropped. Sharded and counter-free it is flat to 4 cores and 22 ns at 10, for 2 ns more on a serial hit (24 ms over the 10 M calls a 2.5M-observation load makes).

Perf justification (AGENTS.md rule 0)

• Interning is O(1) per observation: four map lookups on a table bounded at 8192 entries. There is no work proportional to the number of transmissions or observations held, and no new unbounded structure — the pool is capped and the per-observation cost is one byte of existing padding.
• A hit allocates nothing. A miss allocates one clone, and there can be at most obsStringPoolMax misses for the life of the store.
• Nothing new runs under PacketStore.mu that waits on another goroutine: the two live paths intern while scanning rows, before they take the store lock, which the lock-discipline test pins. The two cold paths hold s.mu for their whole scan anyway; sharding keeps them from waiting on the background fill or on live ingest.
• The allocation count of a load does not change (547.3 MB/op and 13.06 M allocs/op before and after): the SQLite driver still allocates one string per row per column. The saving is in retention — the copies become garbage instead of being held for the life of the observation.

Validation (local)

• cd cmd/server && go test ./...: ok. cd cmd/ingestor && go test ./...: ok.
• go test -race on the _489, _411, _113, _215, chunked-load, eviction and ingest tests: ok, no races.
• sh test-all.sh: 263 passed, 0 failed (263 files). node test-frontend-helpers.js: 719 passed, 0 failed.
• Playwright against a local Go server on e2e-fixture.db prepared as in CI (freshen, the Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486/"Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791 seed, corescope-migrate, the four seed SQL files):
  • test-e2e-playwright.js: 154/157 passed, 3 skipped.
  • Nine more E2Es that read observer metadata or observation counts, all passing: test-issue-76-heard-by-e2e.js (8), test-issue-147-packets-url-obs-e2e.js, test-packet-detail-sender-hash-size-obs-e2e.js (3), test-issue-1562-observers-summary.js (13), test-issue-1639-observers-sort-e2e.js, test-issue-122-compare-reference-many-e2e.js (11), test-issue-497-rx-airtime-e2e.js, test-issue-199-inactive-observer-e2e.js (3), test-observer-iata-1188-e2e.js (12).
• scripts/check-xss-sinks.sh --diff origin/master: no public/**/*.{js,html} changes to scan.
• cmd/server stays read-only: no new writes, no new DB handle.
• map[string]interface{} in non-test Go code: unchanged (198 in store.go before and after; the new file has none).
• No frontend, config, schema, API or workflow change. Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.
• No staging or production access was used. Heap and RSS on the large instance are left to the coordinator; the peak-RSS question in particular is not answered here (see below).

What this does not claim

• Peak RSS is not improved by this change. The row scan still allocates every copy; the pool only stops them being retained. Peak RSS during a cold load is set by the load itself. The numbers above are live heap after load and retained string bytes.
• The 2.5M-observation figures come from the synthetic set, which matches the instance's observer count and observation distribution but not its exact names or path shapes. Steady-state ingest and reader contention on the real instance is the coordinator's measurement.

Follow-ups (not in this PR)

• path_json and timestamp repeat across observations too, but they are not what perf(store): evaluate bounded sharing of observer metadata strings #489 names and they are not a small closed set, so they want a measurement of their own before anyone shares them.
• The pool has no eviction. With 140 steady-state values and 58x headroom that is a non-issue for any realistic process lifetime; a long-lived instance with pathological rename churn would stop sharing (correctly, and with a bounded table) rather than grow. An LRU would be the follow-up if an instance ever reports full climbing.
• Exposing the pool's retention and full count on /api/perf?mem=1 would make the saving visible to operators, but that is a response-shape change and perf(store): evaluate bounded sharing of observer metadata strings #489 rules those out.

🤖 Generated with Claude Code

[adminopenclaw8-sketch]

Rapport — CS-Macmini PR#551 #489 — head 53682d4

Status: implemented as specified; 4 commits (red → green → sharding → test teeth), 18/18 mutants killed, all local suites green, CI all green on the first run (no re-run needed).

Requirements

#	Requirement (#489)	Implementation	Test (red before, green after)	Mutant
1	Bounded, concurrency-safe sharing pool for observer ID, name, IATA, direction	obs_strings_pool.go: obsStringPool owned by the PacketStore, 16 independently locked cache-line-padded shards, cap obsStringPoolMax = 8192 reserved in one shared counter before a slot is taken	[T] ObsStringPoolSharesOneCopy_489, ObsStringPoolConcurrentCallsShareOneCopy_489 (16 goroutines, -race), ObsStringsSharedOnEveryConstructionPath_489	[K] M1 reports shared without consulting the table; M3 the copy is not kept
2	Clone newly retained strings (no substring pinning / oversized backing buffer)	strings.Clone on every miss	[T] ObsStringPoolClonesNewValues_489: 2 bytes out of a 1 MiB buffer; asserts the canonical copy lies outside the buffer and survives dropping it	[K] M2 retain the value as it came
3	A full pool falls back to ordinary strings, never rejects	past the cap the value is returned exactly as it came in, shared=false	[T] ObsStringPoolSaturationFallsBackToPlainStrings_489, ObsStringPoolChurnStaysBounded_489 (2x the cap in distinct values), ObsStringsSaturatedPoolStillIngests_489 (a full pool before a cold load)	[K] M4 no cap; M5 a full pool drops the value
4	All constructors: cold load, chunked load, background fill, live ingest (x2), merge	all five &StoreObs{} sites intern; the #215 merge creates no strings and keeps the copies and charges	[T] ObsStringsSharedOnEveryConstructionPath_489 over all five fill paths (identical observations, totalObs, trackedBytes and pool retention); ObsStringsHashMergeKeepsSharingAndCharges_489	[K] M10–M14, one per constructor
5	No pool contention under PacketStore.mu (#495)	both live paths intern while scanning rows, before taking s.mu; the two cold paths hold s.mu for their whole scan anyway and sharding keeps them from waiting on the others	[T] ObsStringPoolNeverBlocksStoreWriters_489: every shard locked, the ingest asserted to be parked, then s.mu probed 20 times	[K] M15/M16 intern under s.mu (one per live path) — both fail on probe 0
6	A metadata change must not mutate older observations	nothing is ever replaced in place; a renamed observer is a different value with its own entry	[T] ObsStringsMetadataChangeLeavesOlderObservations_489: rename + live ingest; older observations keep their value and their copy, the new one gets its own, the pool grows by exactly 1	[K] M17 the table matches values by their first 4 bytes (the rename shares a prefix on purpose)
7	Separate logical charge from actual shared bytes in trackedBytes; account for the pool's own retention	StoreObs.sharedStrs uint8 in existing padding (struct stays 200 B); estimateStoreObsBytes charges only owned bytes; accountedBytes() = trackedBytes + pool.retainedBytes() feeds trackedMemoryMB and both eviction watermarks	[T] ObsStringsTrackedBytesHoldsSharedBytesOnce_489, ObsStringsEvictionCreditsExactly_489 (all five paths, trackedBytes back to exactly 0, pool retention untouched), ObsStringPoolRetainedBytes_489 (charge is an upper bound on the measured pool), StoreObsLayoutFitsSharedStrsInPadding_489	[K] M6 mask misses a field; M7 charge ignores the mask; M8 pool retention not accounted; M9 entry charge forgets the value bytes; M18 StoreObs grows to 208 B
8	GC stability	the canonical copies are strongly held for the life of the store	[T] ObsStringPoolSurvivesGC_489 (same data pointer after 3 GCs), ObsStringsSharingSurvivesGC_489 (an observation ingested after a GC shares the cold load's copies)	[K] M3

22 of the 26 _489 test and subtest results are red on the red commit (20aef204). The four that pass are the lock-discipline test plus its two subtests and the layout guard, both preservation guards — which is exactly why M15/M16 were needed, and why the first version of the lock test was replaced (commit 53682d4f) after those two mutants survived it.

Measurement first [A]

Same generator and data, intern() stubbed (red commit) vs. the real pool, one run each. "copies" = distinct string data pointers the observations hold for the four fields.

	e2e fixture (499 obs)	250K obs	1M obs	2.5M obs
metadata, logical	41,425 B	20.69 MiB	82.76 MiB	206.90 MiB
distinct values / bytes	48 / 1,770 B	140 / 5,381 B	140 / 5,381 B	140 / 5,381 B
duplicate share	95.7 %	99.98 %	99.99 %	100.00 %
copies, before → after	1,497 → 48	945,889 → 140	3,783,971 → 140	9,460,013 → 140
heap after load	11.78 → 11.74 MiB	166.1 → 141.9 MiB (−14.6 %)	642.2 → 545.3 MiB (−15.1 %)	1562.8 → 1321.4 MiB (−15.4 %, −241 MiB)
trackedBytes	1.54 → 1.50 MiB	158.2 → 137.6 MiB	633.2 → 550.4 MiB	1583.5 → 1376.6 MiB
Load wall time	—	1360 → 1390 ms	6802 → 6720 ms	23025 → 23062 ms

The synthetic set has the instance's shape: 65 observers, log-normal observations per transmission (mean 20.6, p50 15, p90 42, p99 96). trackedBytes/heap moved from 1.013 to 1.042 at 2.5M observations — still tracking, still over-charging slightly, so eviction triggers earlier rather than later.

Capacity policy from those numbers: 140 entries in the steady state, cap 8192 (58x headroom), pool retention bounded at 2.2 MB of charge for about 0.7 MB in fact.

Benchmarks [A]

BenchmarkLoad_Obs489 (250K observations, -benchtime 3x -count 6, medians, Go 1.27.0 darwin/arm64, 10 cores):

	before (20aef204)	after
sec/op	1.369 s	1.386 s (+1.2 %)
heap after load	166.1 MiB	141.9 MiB (−14.6 %)
trackedBytes	158.2 MiB	137.6 MiB (−13.0 %)
B/op	547.24 MB	547.27 MB (~)
allocs/op	13.056 M	13.057 M (~)

B/op does not move on purpose: the driver still allocates one string per row per column. The saving is in retention, not allocation.

Pool micro-benchmarks (140 held values, medians of 3): a hit is 11.4–11.9 ns and 0 allocs at -cpu 1/2/10; internObs (four fields) 39 ns and 0 allocs; filling an empty pool to the cap is 92 ns per miss. Sharding is justified by measurement: on a single RWMutex the same parallel hit cost 9 ns alone but 34 ns with two goroutines and 84 ns with ten, and a per-hit atomic counter was worse still. Sharded and counter-free it is flat to 4 cores and 22 ns at 10, for 2 ns more on a serial hit.

CI per job

Run 40084909581, head 53682d4f, conclusion success. Every job passed on the first attempt; the known-flaky #271 E2E did not fire, so nothing was re-run.

Job	Result
Go Build & Test	success
Go Server Tests (1/3)	success
Go Server Tests (2/3)	success
Go Server Tests (3/3)	success
Go Ingestor Tests	success
Go Libraries & CLI	success
JavaScript Unit Tests & Lint	success
Playwright E2E Tests (1/3)	success
Playwright E2E Tests (2/3)	success
Playwright E2E Tests (3/3)	success
Verify E2E Shards & Coverage	success
Verify Go Test Assignments	success
Script & Container Unit Checks	success
Validate Docker Image	success
Build & Publish Docker Image	success
Publish Badges & Summary / Release Artifacts / Deploy Staging	skipped (fork guards, as expected on a PR)

Local validation [T]

• cmd/server: go test ./... ok; -race on _489/_411/_113/_215/chunked-load/eviction/ingest/test(server): data race between startBackgroundIndexBuilds and PacketStore.Load under -race (TestHashMigrate_LogsMaxWriteLockHold_215) #301 ok, no races. cmd/ingestor: go test ./... ok.
• sh test-all.sh: 263 passed, 0 failed (263 files). node test-frontend-helpers.js: 719 passed, 0 failed.
• Playwright against a local Go server on e2e-fixture.db prepared as in CI: test-e2e-playwright.js 154/157 passed, 3 skipped; nine further observer/observation E2Es all pass (Heard By, packets ?obs=, sender hash size, observers summary and sort, observer comparison, RX airtime, inactive observer, observer IATA).
• scripts/check-xss-sinks.sh --diff origin/master: no frontend changes to scan.
• cmd/server stays read-only; map[string]interface{} in non-test Go code unchanged; no schema, API, config or workflow change; fork guards unchanged (9 in deploy.yml, 1 in release-fast-path.yml).
• No staging or production access was used.

Remainders [K]

1. Peak RSS is not addressed. The row scan still allocates every copy; only retention changes. Peak RSS during a cold load is set by the load itself. The issue asks for peak RSS on representative data — that measurement belongs to the coordinator on the instance.
2. Steady-state ingest and reader contention on real data is likewise not measured here; the local evidence is the parallel pool benchmark and the lock-discipline test.
3. The pool has no eviction. With 140 steady-state values and 58x headroom this is a non-issue for any realistic process lifetime; pathological rename churn would stop sharing (bounded, correct) rather than grow. An LRU is the follow-up if an instance ever reports the full counter climbing. Not filed.
4. The pool's retention and full counter are not exposed. TrackedMB reflects them, but a dedicated line on /api/perf?mem=1 would be a response-shape change, which the issue rules out. Not filed.
5. GetStoreMemoryBreakdown's ObsStringsMB is now a much looser upper bound (it counts the same ~140 values once per observation). Its doc comment says so; the field itself is unchanged.
6. path_json and timestamp repeat across observations too, but they are not what the issue names and are not a small closed set. Not filed.

[dborup-agent]

Review — CS-pve-agent3 PR#551 — head 53682d4

Dom: APPROVE with nits

Evidence labels: [T] I ran it myself · [A] my own analysis of the code or the numbers · [K] taken from the author's report or CI without reproducing it myself.

Trees tested: git merge-tree --write-tree origin/master 53682d4f…. The first run was against 9080b6aa (merged tree c3ea21ce). Master moved to 5af6129c during the review (#549, #550, #552; the cmd/server changes are in neighbors, paths LRU and routes). I re-merged onto it (merged tree 141f22ce, no conflicts) and re-ran the Go server suite, the JS suites and the core E2Es there. The head was 53682d4f per git ls-remote before, during and after the review.

Findings

#	Severity	Finding	Evidence
1	Low	The estimateStoreTxBytesTypical change has no test. Its per-observation cost drops by 60 B (the old 30 + 30 for observer ID and name), so under maxMemoryMB the bounded cold load (Load/LoadChunked maxPackets) now admits +9.9 % transmissions. At 512 MB that is 80,902 → 88,944 tx. The direction is justified: a shared observation really saves about 100 B of heap (measured: 243 MiB over 2.5M observations), so the estimate is still conservative, and post-load eviction still bounds the store on accountedBytes. But reverting the line (mutant R1) survives the full cmd/server suite. Suggest a test that pins the typical estimate to estimateStoreObsBytes of a shared observation, or that derives it from that function.	[T] mutant R1, full suite; [T] scratch test for the maxPackets numbers; [A]
2	Nit	The obsStringEntryBytes doc says the charge is an upper bound "at every fill". It is not true below about 100 entries. The 16 pre-sized shard tables cost about 21 KB even when they are empty and are never charged. Measured on linux/amd64 with Go 1.27.1, charged vs. real: 0 entries 0 B vs. 21.0 KB; 10 entries 2.5 KB vs. 21.6 KB; 48 entries (the e2e fixture) 12.1 KB vs. 23.2 KB; 140 entries 35.2 KB vs. 27.2 KB (1.30x); 500 to 8192 entries about 2.3x. The worst under-charge is about 21 KB, which does not matter at the 1 MB granularity of maxMemoryMB. Fix the comment, or charge a fixed per-pool base.	[T] scratch test against newObsStringPool()
3	Nit	The pool term is a floor that eviction cannot free. A saturated pool charges about 2 MB, so with maxMemoryMB ≤ 2 the low watermark sits below that floor and every memory pass would evict the whole store. This is not a realistic config, but one sentence next to accountedBytes would document it.	[A]
4	Nit	The comment on TestObsStringsConcurrentIngestAndReaders_489 says "live ingest, eviction and readers at once", but no eviction runs in that test.	[A]
5	Info	Peak RSS does go down in the harness, so "Peak RSS is not improved" in "What this does not claim" is too conservative. Whole-process max RSS for the measurement run (/usr/bin/time -v, one run each; it includes the data generator and the SQLite writes, default GOGC, no GOMEMLIMIT): 250K obs 308 → 254 MiB (−17.7 %), 1M obs 1093 → 897 MiB (−18.0 %), 2.5M obs 2533 → 2250 MiB (−11.2 %). Production peak RSS is still unmeasured.	[T]
6	Info	Pre-existing flakes I hit; all of them reproduce on master and none of them is caused by this PR. TestHandleNodePaths_SortByRecency_1145 and TestHandleNodePaths_PrefixCollision_1352_FallbackBranch return 503 "index loading" under CPU load (1/30 on master in isolation). TestRepeaterRelayInfoMap_ConcurrentIngestAndEviction fails 2/150 on master and 1/150 merged. Locally, "Version info lives on Perf dashboard" in test-e2e-playwright.js fails on both the master and the PR binary whenever the server cannot resolve a git commit from its cwd; it passes 154/0 when it can, as in CI. The observer-IATA mobile E2E fails on both binaries once the freshened fixture is about 30+ min old. Not filed (out of my mandate). These are worth an issue for the coordinator.	[T]

Answers to the review points

1. RAM change in the store. [T] I copied obs_strings_489_data_test.go onto plain origin/master (it compiles there unchanged) and ran the same generator against both binaries:

obs	heap after load, master → PR	trackedBytes, master → PR	copies held	author's heap
e2e fixture (499)	11.75 → 11.73 MiB	1.54 → 1.50 MiB	1,497 → 48	11.78 → 11.74
250,000	166.1 → 141.9 MiB (−14.6 %)	158.2 → 137.6 MiB	945,889 → 140	166.1 → 141.9
1,000,000	642.2 → 545.3 MiB (−15.1 %)	633.2 → 550.4 MiB	3,783,971 → 140	642.2 → 545.3
2,500,000	1563.7 → 1320.6 MiB (−15.5 %, −243 MiB)	1583.5 → 1376.6 MiB	9,460,013 → 140	1562.8 → 1321.4

Logical observer metadata at 2.5M is 206.90 MiB against 5,381 B for the 140 distinct values (65 IDs, 65 names, 8 IATA codes, 2 directions). The numbers match the author's within 1 MiB.

Startup time: BenchmarkLoad_Obs489 (-benchtime 3x, 2 × -count 3 interleaved, 4-core linux/amd64): median 3.86 s on master vs. 3.87 s on the PR. Run-to-run noise was ±15 %, so I see no measurable regression. B/op (547.1 MB) and allocs/op (13.06 M) are unchanged, as claimed. [T] The author's +1.2 % on darwin/arm64 and the parallel micro-benchmarks I did not re-run. [K]

2. Requirements from #489.

• Bounded pool with a capacity policy. The cap is obsStringPoolMax = 8192, reserved through an atomic count before the insert, so the cap is exact. The policy (140 steady-state values, 58x headroom) is documented. [A] Mutant R4 (the overflow does not release its reservation) is killed. [T]
• A full pool falls back to normal strings and never rejects. A full pool returns the input as is with shared=false and counts it in full. Mutant R6 (a full pool reports shared=true) is killed by SaturationFallsBackToPlainStrings and SaturatedPoolStillIngests. [T]
• Clone, no substring pinning. strings.Clone runs on every miss, and ClonesNewValues checks that the canonical copy lies outside a 1 MiB backing buffer. [A][T]
• All constructors covered. grep 'StoreObs{' finds exactly 5 non-test sites (Load, loadChunkWhere, scanAndMergeChunk, IngestNewFromDB, IngestNewObservations), and all 5 intern. The hash_migrate: server-side DB writes on the read-only handle; failures logged as collisions; ghost duplicates share a hash #215 merge only moves observations and keeps their copies and masks; HashMergeKeepsSharingAndCharges covers that. No production path writes ObserverID/Name/IATA/Direction on a StoreObs after construction; tx.ObserverID = best.ObserverID only re-points to a canonical copy. [A] Mutant R5 (the chunk merge interns but drops the mask) is killed by SharedOnEveryConstructionPath. [T]
• No pool contention under PacketStore.mu (perf(store): move runtime SQL outside packet-store locks #495). The two live paths intern during the row scan, before s.mu. Load and scanAndMergeChunk intern under s.mu, but the shard locks are leaf locks: no code path takes s.mu while it holds a shard lock, and a miss holds a shard for one clone of ≤ 64 B. The lock test parks the ingest first and then probes s.mu 20 times, which is a real test. [A] TestNoSQLUnderStoreLock and its detector pass on the merged tree. [T]
• Metadata changes do not mutate older observations. My mutant R7 (IngestNewObservations back-propagates the new name to the observer's older observations) is killed by MetadataChangeLeavesOlderObservations. [T]
• Concurrent calls. My mutant R3 (no re-check under the shard write lock, so two racers insert two copies) is killed 3/3 under -race by ConcurrentCallsShareOneCopy. [T]
• Red before, green after. On the red commit 20aef204, 22 of the 26 _489 results fail. The 4 that pass are the lock test with its 2 subtests and the 200-byte layout guard, which are preservation guards, exactly as stated. [T] The lock test's teeth come from the author's M15/M16, which I did not re-run. [K]

3. trackedBytes and eviction.

• The charge is still a pure function of the observation (the sharedStrs mask), so eviction credits exactly what ingest charged. acct113Check passes on every path, and EvictionCreditsExactly returns trackedBytes to 0 on all 5 paths. [T][A]
• Pool retention enters accountedBytes(), which both watermarks and trackedMemoryMB use. Mutant R2 (both watermarks use s.trackedBytes) is killed, by the existing TestObsDedup411_MemoryBudgetEvictsTheSameOnEveryPath. [T]
• Behaviour against master under the same budget [T], scratch test on 250K observations with the same data on both builds:

path, budget	master: kept obs / heap after eviction	PR: kept obs / heap after eviction
Load, 32 MB	43,832 / 43.2 MiB	49,461 / 43.6 MiB
Load, 64 MB	87,450 / 79.3 MiB	98,777 / 77.3 MiB
ingest, 32 MB	44,638 / 74.6 MiB	49,442 / 68.1 MiB
ingest, 64 MB	85,935 / 85.0 MiB	104,883 / 84.6 MiB

The same budget now holds 11–22 % more observations for equal or lower real heap. The budget is not loosened in real-memory terms, and nothing is evicted earlier or later than it should be. trackedBytes/heap moves from 0.986 to 1.009 at 1M (over-charge), consistent with the author's 1.013 → 1.042 at 2.5M. The only uncovered piece is the cold-load estimate (finding 1).

4. Conflict with #490 (pve-agent1). There is no #490 branch or PR on the remote yet (git ls-remote and the PR list), so this is [A] from the issue text and the current layout.

• Textual: perf(store): evaluate compact, concurrency-safe observation timestamp cache #490 replaces tsParseOnce, tsParsed and tsParsedOK. Those lines sit right next to the wireRoute comment ("fills padding after tsParsedOK") and the new sharedStrs block in StoreObs, so they will conflict.
• Guards: TestStoreObsLayoutFitsWireRouteInPadding_76 references o.tsParsedOK and pins Sizeof == 200, and the new TestStoreObsLayoutFitsSharedStrsInPadding_489 also pins 200. perf(store): evaluate compact, concurrency-safe observation timestamp cache #490 has to update both: the first will not compile and the second will fail.
• Accounting: if perf(store): evaluate compact, concurrency-safe observation timestamp cache #490 retunes storeObsBaseBytes or estimateStoreObsBytes, it overlaps the function bodies this PR edits.
• Semantic: perf(store): evaluate compact, concurrency-safe observation timestamp cache #490's heap savings must be measured on top of this PR, because the per-observation heap shrinks by about 100 B here.
• Recommendation: merge perf(store): bounded sharing of observer metadata strings #551 first (it is ready, the change is 1 byte in existing padding, and CI is green). Then perf(store): evaluate compact, concurrency-safe observation timestamp cache #490 rebases onto it, keeps wireRoute and sharedStrs together in the trailing padding, sets both size guards to the new actual size, and measures against the post-perf(store): bounded sharing of observer metadata strings #551 baseline. A cheap way to make that easier: relax the _489 guard to "sharedStrs does not grow StoreObs" (for example Offsetof(sharedStrs) == Offsetof(wireRoute)+1 plus a size check) instead of a hard 200.

Tests and mutants

• Go [T]:
  • cd cmd/server && go test ./...: ok on both merged trees (c3ea21ce, 141f22ce). One earlier run hit the 503 flake from finding 6, which also reproduces on master.
  • cd cmd/ingestor && go test ./...: ok on c3ea21ce; the new master does not touch the ingestor.
  • go test -race on _489|_411|_113|_215|TestNoSQLUnderStoreLock|Chunked|Evict|Ingest: ok, no races.
  • go vet: clean.
• JS [T]: sh test-all.sh gives 265/265 (c3ea21ce) and 266/266 (141f22ce); node test-frontend-helpers.js gives 719/0.
• E2E [T]: local Go server on e2e-fixture.db, prepared as in CI (freshen, the Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486/"Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791 seed, corescope-migrate, then seeds 2073, 199, 245 and 122). Server stopped by port.
  • On 141f22ce, all green: test-e2e-playwright.js 154/157 (3 skipped), heard-by Node "Heard By" card: decide whether to show direct-RF observers (defer) #76 8/0, observer IATA feat: show observer IATA on packets (table, observation rows, detail pane) + add to filter grammar Kpa-clawbot/CoreScope#1188 3/0, packets ?obs= fix(packets): updatePacketsUrl drops ?obs= and ?viewPath= on load and on filter changes #147 12/0, observers summary Observer Online/Stale/Offline aggregate counts derived from cached payload — wrong on CDN-fronted deployments Kpa-clawbot/CoreScope#1562 13/0, compare Feature request: Reference-to-many observer comparison with complete-window packet overlap #122 11/0, inactive observer Active observers vanish as nodes after retention.nodeDays without an advert; observer link dead-ends on 'Node not found' #199 3/0, observers sort bug: observers table is not sortable (regression-equivalent — same class as #679) Kpa-clawbot/CoreScope#1639 all, RX airtime feat(observers): show RX 1h airtime (channel load) next to TX 1h, and both on the observer detail page #497 all, sender hash size 3/0.
  • On c3ea21ce the same set passed, plus observers.js leaks delegated click/keydown handlers on the shared #app container #405 6/0 and channels path-hash-size 8/0.
• CI on head [T]: run 40084909581, attempt 1, every job green; publish/release/deploy skipped by the fork guards. Neither flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256 nor flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267 fired. CI ran against the older base, not the merged tree. [K]
• My mutants (each applied to a scratch copy, then restored and diff-checked) [T]:

#	Mutant	Result
R1	revert the estimateStoreTxBytesTypical obs cost (+30+30)	survives the full server suite (finding 1)
R2	both memory watermarks use s.trackedBytes, not accountedBytes()	killed (ObsDedup411_MemoryBudgetEvictsTheSameOnEveryPath)
R3	no re-check under the shard write lock	killed 3/3 under -race (ConcurrentCallsShareOneCopy)
R4	overflow keeps the reserved count	killed (ChurnStaysBounded, SaturationFallsBack…, SaturatedPoolStillIngests)
R5	scanAndMergeChunk interns but sets sharedStrs: 0	killed (SharedOnEveryConstructionPath)
R6	a full pool returns shared=true	killed (SaturationFallsBack…, SaturatedPoolStillIngests)
R7	live upsert rewrites older observations' ObserverName	killed (MetadataChangeLeavesOlderObservations)

• Policy [T]:
  • cmd/server stays read-only: the non-test diff adds no Exec, SQL writes or DB handles.
  • map[string]interface{} in non-test Go: 749 before and 749 after; none added.
  • No public/ change, so no colors are touched. scripts/check-xss-sinks.sh --diff origin/master reports "no public/**/*.{js,html} changes to scan".
  • Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml; workflows are unchanged.
  • No closing keywords in the commits or the body ("Relates to perf(store): evaluate bounded sharing of observer metadata strings #489").
  • All 4 commits are authored and committed by dborup <kontakt@meshview.dk>.
  • No behaviour change beyond the purpose: no response shape, schema or config change. The TrackedMB value now includes the pool's ≤ 2.2 MB, and the cold-load estimate moves as in finding 1, both within scope. [A]

What I did not verify

• Heap, peak RSS and steady-state ingest/reader contention on the real large instance. I had no staging or production access, by mandate. All numbers above come from the synthetic generator and the e2e fixture.
• darwin/arm64 numbers and the author's parallel micro-benchmarks (ObsStringPoolHitParallel, the sharding rationale).
• The author's mutants M1–M18, beyond the overlap with mine.
• A full-suite -race run (only the targeted subset above) and CI on the merged tree.
• perf(store): evaluate compact, concurrency-safe observation timestamp cache #490 code: no branch or PR exists yet, so the conflict analysis is from the issue text and the current layout only.