Files
efitenandClaude Opus 5.5 4363403495 fix(nodes): match the region node filter on observer ID, not ingest-time IATA (#2115)
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>
2026-10-05 23:42:07 +02:00
..