Addresses #1910. The Observers page hangs on "Loading..." for 10-20s;
the reporter measured `/stats` at 10-17s under the mixed load that page
produces, while the same endpoint stays under 70ms at 8x concurrency
when it is the only one being hit.
## Cause
Two cache layers guard the expensive work and **neither has
single-flight**:
| | | |
|---|---|---|
| `handleStats` | 10s cache | releases `statsMu` before rebuilding
(`routes.go:774`) |
| `GetStoreStats` | 30s cache | releases `statsCacheMu` before scanning
(`store.go:2048`) |
Both do check, release, then work. The moment either window expires,
**every in-flight request does the whole thing itself**.
The expensive part is a range scan over 24h of `observations` with two
`SUM(CASE...)` over it. The column is indexed
(`idx_observations_timestamp`), but the scan still visits every row in
the window, and at 18k observers that is millions. The pool is
`SetMaxOpenConns(4)` (`db.go:111`), and the page fires stats, observers,
nodes, channels and clock-skew at once, so one cache miss turns a single
scan into a queue of them.
That is exactly the reported profile: fast alone, slow only when mixed.
## Changes
1. **Single-flight both layers.** Concurrent callers that miss the cache
wait for the first one's result instead of each running the same
queries.
2. **Serve the observation counts stale while refreshing in the
background.** An expired cache answers from the previous value and kicks
off one refresh, so a miss is never a wait.
Single-flight alone would not have fixed the hang: the first caller
still waits for the full scan. The second change is what removes it.
## Contract change, stated plainly
`TestGetStoreStats_CacheExpiry` asserted that an expired cache returns
**fresh DB values on the same call**. It no longer does.
For `packetsLastHour` / `packetsLast24h` on a dashboard, answering with
a value up to ~30s older instead of blocking for seconds looks like the
right trade to me. But that is a judgement, not a bug fix, and **a
reviewer should be able to reject it**. I did not quietly delete the
test: it now asserts what still has to hold, that the refresh happens,
and the new behaviour is pinned separately by
`TestGetStoreStats_StaleCacheServedWithoutBlocking`.
If you would rather keep the old contract, drop change 2 and keep change
1; the diff separates cleanly.
## Verification
The new test **fails without the change**:
```
stale cache not served: got (0, 2), want (424242, 434343). An expired cache must
answer from the previous value and refresh in the background, not block the
request on the observations scan
```
`gofmt` clean, `go vet` clean, `cmd/server` suite ok in 267s.
## Two things I did not verify
**The race detector.** It needs cgo and there is no gcc on this machine,
so `go test -race` cannot run here. This change adds a background
goroutine writing the cache under `statsCacheMu`, so that check matters.
CI runs `go test -timeout 20m -race` for `cmd/server`
(`deploy.yml:134`), which covers it before merge.
**The 10-17s itself.** I have no database with 18k observers. The
mechanism above explains the reported profile, including why the
endpoint is fast in isolation, but I did not measure the figure.
@dborup, if you can run a build from this branch, the number to watch is
`/stats` under the same mixed-load command from your issue.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
