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>
