mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-23 03:21:17 +00:00
efitenandClaude Opus 5.5
d5e6d1b2a6
Fixes #2101. `/api/nodes?region=` filtered nodes with a subquery that joins every advert to all of its observations and observers, with no time bound (`cmd/server/db.go`, `GetNodes`). It ran twice per request, once for `COUNT(*)` and once for the page. A client paging through nodes repeats both per page. ## Reproduced On our staging database (10.3 GB, 16.5M observations, 219,750 adverts), read-only `sqlite3`, region `BRU`: | | real | user | sys | |---|---|---|---| | one regional `COUNT(*)`, as `GetNodes` builds it | **154.8 s** | 1.9 s | 5.8 s | Almost all of it is waiting on disk. The plan walks `idx_transmissions_payload_type` for every advert and then `idx_observations_dedup` for each one's observations. Two of those per request, a few requests at once, and the reader pool is gone, which matches the report of all four database workers sitting in `GetNodes`. ## Fix - **`PacketStore.RegionNodePubkeys(region)`** (new `cmd/server/region_nodes.go`) walks the store's in-memory adverts (`byPayloadType[ADVERT]`) once. It keeps the pubkey of every node with an advert heard by an observer in the region, using the `ObserverIATA` each observation already carries. It is cached for 30 s per region, and the cache is bounded to 64 entries because its keys come from the client's parameter. Lock discipline follows `resolveAreaNodes`: the cache mutex and `s.mu` are never held together, and the ordering note in `store.go` lists it. - **`GetNodes` takes a `NodeQuery` struct** with a `RegionPubkeys` field, passed as one `json_each` parameter: `public_key IN (SELECT value FROM json_each(?))`, a primary-key lookup. An empty set matches nothing. The SQL subquery stays for a server without a store (tests, tooling). - The advert-pubkey lookup that `trackAdvertPubkey`, `untrackAdvertPubkey` and `computeNodeHomeRegions` each copied is now one helper, `advertPubkey`. The struct instead of a second `GetNodes` variant keeps the `map[string]interface{}` count unchanged in `db.go` (75) and `routes.go` (59). ## Behaviour change The region filter now covers the adverts the store holds (`packetStore.retentionHours`), which is the window the rest of the UI shows, instead of all database history. A node heard in a region only before that window no longer matches the filter. While the store is still loading after a restart, the set grows as history loads. ## Performance | | before | after | |---|---|---| | region set, uncached | 154.8 s (SQL count, staging) | 37 ms (`BenchmarkRegionNodePubkeys`: 220k adverts × 8 observations, 4,000 nodes) | | region set, within 30 s | same again | cache hit | | node count over the set | (included above) | 2 ms on staging (1,200 keys) | | 500-row page over the set | same scan again | 3 ms on staging | The scan holds `s.mu` for reading for those 37 ms, at most once per region every 30 s. ## Tests - `TestRegionNodePubkeys`: the in-region advert, an advert heard in two regions, case and whitespace in codes, a comma list, an unknown region giving an empty set, a non-advert never counting, a blank region giving no filter. - `TestRegionNodePubkeysCached`, `TestRegionNodePubkeysCacheIsBounded` (1,000 distinct regions stay within 64 entries). - `TestGetNodesRegionPubkeys`: the set combines with the role filter and counts correctly, and an empty set returns nothing even with `Region` set. - `TestHandleNodesRegionUsesStore`: an advert that only the store knows about shows up through `/api/nodes?region=`, so the handler is proven not to ask SQL. - Existing region tests (`TestGetNodesRegionFilterV2` and the `db_test.go` region cases) pass unchanged through the SQL fallback. Full `cmd/server` suite passes locally; the new tests also pass with `-race`. ## Not done - No request context on these queries, also raised in the issue. With the scan gone they take milliseconds, so I left that out of this change. - The region semantics stay "heard by an observer in the region". #1879 argues for the node's home region instead; that is a separate decision. - No frontend change. I did not check this in a browser; the nodes page and map call the same endpoint with the same parameters. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>