mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-07 05:17:19 +00:00
Follow-up to #2114, from its review. `RegionNodePubkeys` compared the IATA copied onto each observation at ingest (`StoreObs.ObserverIATA`). When an operator changes an observer's code, those copies stay as they were until a restart or until the observations age out. `/api/nodes?region=` then kept matching the old region, while the other store region filters already used the new one. Those filters resolve observers by ID through `resolveRegionObservers`, for example the packets query and `computeNodeHomeRegions`. The SQL subquery that #2114 replaced also read `observers.iata` at query time, so this restores that behaviour. ## Also fixes a regression from #2114 Found in review: `loadChunk`, the background history loader (`cmd/server/store.go`, the chunk SELECT and the `StoreObs` it builds), sets `ObserverID` on each observation but never `ObserverIATA`. With #2114 matching on that IATA, **every node whose adverts came only from background-loaded history dropped out of `/api/nodes?region=`** on current master. Matching on observer ID fixes it. `TestRegionNodePubkeysMatchesChunkLoadedObservations` pins it (an observation with the observer ID and an empty IATA) and fails on master's `region_nodes.go`. ## Change - Resolve the region to observer IDs with `resolveRegionObservers` (own mutex, 30 s cache) and match observations by `ObserverID`. Lock order: `regionNodesMu` is released before it, and `s.mu` is taken after it; none of the three is held together. - Without a database there is nothing to resolve, so `RegionNodePubkeys` reports no set and the handler keeps the SQL path. ## Tests - The region tests now seed an observers table and leave each observation's IATA at a stale value, so they can only pass through the table. - New `TestRegionNodePubkeysFollowsObserverIATAChange`: an observer that moved from SJC to SFO matches SFO and not SJC. It fails on the previous code (`got [pk_moved]` for SJC). - `TestRegionNodePubkeysMatchesChunkLoadedObservations` (second commit), see above. - `setupTestDB` takes `testing.TB` so the benchmark can use it; every existing caller passes `*testing.T` unchanged. - Full `cmd/server` suite passes locally, the region tests also with `-race`. ## Performance `BenchmarkRegionNodePubkeys` (220k adverts × 8 observations): 37 ms to 30 ms per uncached scan, a map lookup per observation instead of a string normalisation. The observer lookup is one query on the small `observers` table, cached for 30 s. ## Not done - No singleflight on a cold cache, and the 64-entry cache still resets when full. Both were non-blocking in the #2114 review. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>