Repository navigation
Conversation
…bot#2101) `/api/nodes?region=X` filtered with an inline `public_key IN (SELECT DISTINCT from_pubkey FROM transmissions ⋈ observations ⋈ observers ...)` subquery. It was uncached and evaluated twice per request (COUNT(*) and the page), and fetchAllNodes() pages at 500, so each Nodes or Map view ran it several times. Measured on a 1.4 GB database (1.47M observations, 2 vCPU): ~10s per evaluation, 18-21s per region-filtered page on v3.12.0, and one open tab refreshing every minute kept the reader pool and both cores busy. Membership is cached per canonical region set (sorted, de-duplicated codes) with an observations.id watermark: - fresh for 30s; after that the cached set is served while one background refresh (singleflight, Kpa-clawbot#1910 pattern) scans only observations past the watermark, with the join order forced to drive from the rowid range; - every 30 minutes the refresh is a full rebuild instead, so retention pruning, observer IATA changes and late from_pubkey backfills are not held forever. Full rebuilds are serialised across region sets, since each holds a pooled connection for seconds; - only the first request for a region set waits on a full scan, and concurrent first requests share it; - a failed refresh is logged and leaves the previous entry serving; scans run under a context bounded by the rebuild interval; - entries are immutable once stored and the cache holds at most 32. GetNodes binds the set through `public_key IN (SELECT value FROM json_each(?))`, so COUNT and SELECT no longer re-run the join. Results match the previous subquery (the tests use it as the oracle, on v2 and v3 schemas); the only change is that a node newly heard in a region can take up to one refresh interval to appear. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A13xB0
marked this pull request as draft
October 2, 2026 20:42
Contributor
Author
|
moved back to drafts due to it all rebuilding at the same time causing waves at 200% CPU for 4 minutes at a tie every 30 minutes |
Contributor
Author
|
moving away from caching, leaving this here until I have a different solution |
|
I’ve been testing a smaller fix for this too. Mine also stops doing work when the client disconnects. It might be good to cover that in your changes too so that requests nobody is waiting for don’t keep running? A shared refresh could still continue if other requests need it. |
5 tasks
adminopenclaw8-sketch
pushed a commit
to dborup/CoreScope
that referenced
this pull request
Oct 7, 2026
Review follow-ups on the Kpa-clawbot#2102 port. 1. Keep #38's from_pubkey contract inside the cache. The port had reintroduced COALESCE(t.from_pubkey, JSON_EXTRACT(decoded_json, '$.pubKey')), which #38 measured and rejected: it rescues no rows, and one corrupt ADVERT fails the whole query. Behind a cache that is worse than before, because the failed refresh leaves the entry stale for every later request too, not just the one that triggered it. #38's comment block moves to the query it documents. 2. Bound the cache by evicting one entry, not by clearing the map. The old setNodeRegionEntry replaced the whole map when a 33rd region set arrived, so a client cycling through region sets — ?region= is a query parameter — emptied the cache on every request and every following request paid a full observation scan, serialised behind nodeRegionFullMu. Entries now evict least-recently-used, and a region set too long to key is stored under its SHA-256 digest (channelListMaxKeyBytes's rule). 3. Make the documented freshness the delivered freshness. An entry older than nodeRegionMaxStale is no longer served blind: the caller waits for its refresh, which at that age is a full rebuild. Without the wait a refresh that keeps failing serves membership of unbounded age with only a log line to show it. A failing refresh still falls back to the stale entry rather than 500-ing /api/nodes?region=. Tests: LRU eviction and the bound under 4x churn, the digest key, an addition visible within nodeRegionFreshTTL, removal by retention and by an observer IATA change gone within nodeRegionRebuildInterval and not by the delta scan, the over-stale wait and its fallback, and the cached result set against the uncached subquery for six region sets before and after new ADVERTs. The port's legacy-backfill test is replaced by one that locks #38's contract on the delta path. nodeRegionQueryHook now returns an error so a test can fail a scan. Perf (rule 0), nodes_region_cache_perf_176_test.go: 498MB / 1.5M observations / 749k transmissions, ANALYZE run; one Nodes page load with a region selected is 4 GetNodes pages of 500, 7 interleaved rounds of the same test file compiled against master and against this branch. Warm page load median 3729ms -> 31ms (121x; per GetNodes call 932ms -> 7.7ms, master's 932ms matching the 1-1.4s a region call costs on prod). Cold page load median 4292ms -> 705ms (6.1x): the one full membership scan replaces eight evaluations of the subquery.
dborup
added a commit
to dborup/CoreScope
that referenced
this pull request
Oct 7, 2026
…ache perf(nodes): port upstream region membership cache (Kpa-clawbot#2102)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2101.
/api/nodes?region=Xdecided membership with an inlinepublic_key IN (SELECT DISTINCT from_pubkey FROM transmissions ⋈ observations ⋈ observers …)subquery. It was uncached, unbounded in time, and evaluated twice per request (COUNT(*)and the page).fetchAllNodes()pages at 500, so a single Nodes view ran it several times, and one tab left open refreshing every minute was enough to occupy the reader pool.On the ScotMesh instance (1.4 GB database, 1.47M observations, 2 vCPU, v3.12.0) each evaluation took about 10 s, a region-filtered page took 18–21 s, and
corescope-serversat near 200% CPU. That matches what #2101 reports at larger scale.What changes
Region membership (the set of public keys heard in a region set) is cached per canonical region set, together with an
observations.idwatermark. The canonical key is the sorted, de-duplicated codes fromnormalizeRegionCodes, soEDI,GLA,gla, ediandEDI,EDI,GLAshare one entry. New file:cmd/server/nodes_region_cache.go.singleflight, the perf: /api/stats and /api/observers degrade to 10-17s under concurrent load (18k+ observers, ~4M observer_upserts/5min cycle) #1910 pattern) scans only observations past the watermark. That scan forces the join order withCROSS JOINso it drives from the rowid range. Left to the planner, thesqlite_stat1added in GetChannels/GetEncryptedChannels: cold query cost is driven by payload_type, not region (~10-13s solo) — EXPLAIN QUERY PLAN + numbers #2058 starts from the region's observers and walks their whole history.from_pubkeybackfills are not held forever. Full rebuilds are serialised across region sets, because each holds one of the four pooled connections for seconds and entries built together at startup fall due together.GetNodesbinds the set throughpublic_key IN (SELECT value FROM json_each(?)), soCOUNTandSELECTno longer re-run the join.Results are unchanged, except that a node newly heard in a region can take up to one refresh interval (about 30 s) to appear in that region's list. Region semantics are the same as before, so #1879 (attributing by home region) is neither addressed nor made harder: it would change what the cached set contains, not how it is cached.
Perf justification (rule 0).
json_eachover at most a few thousand keys.Measurements
The same database, queried directly:
EDIThe ScotMesh production instance ran this algorithm for 28 h, probed every 5 minutes. That build was an earlier revision of this branch on a v3.12.0 base; see Validation for what changed after it.
/api/nodes?limit=500®ion=EDI/api/nodes?limit=500/api/nodes, all traffic (/api/perf)corescope-serverCPU, averageMemory was flat (1.00–1.03 GiB, under
GOMEMLIMIT=1GiB).Worth a decision
nodeRegionRebuildIntervalcould reasonably be hours rather than 30 min, since pruned nodes lingering in a region's list matters little against multi-day retention. I left it at 30 min as the conservative choice and am happy to change it.EDI. Every later request is served from the cache.nodeRegionFreshTTL(30 s),nodeRegionRebuildInterval(30 min) andnodeRegionMaxEntries(32) are constants for now. The rebuild interval is the one worth exposing to operators later.GetNodes. bug: Regional /api/nodes queries exhaust the SQLite connection pool #2101 also notes the queries ignore client disconnects. Membership is now off the request path and the remainingGetNodesqueries are cheap, so I haven't threaded the request context through. The background scans have their own bounded context.Validation
cmd/server/nodes_region_cache_test.go, 10 tests:COUNT, pages and equivalent spellings;MAX(id)row: the connection test, with a clear message after 10 s;singleflight: 8 scans instead of 1.MAX(id)query and scanned only one, leaking a pooled connection per refresh, and it hung every DB-backed endpoint within minutes. It was rolled back, fixed, and the 28 h soak above ran on the fixed build.go test -race ./...passes. One unrelated test is flaky under CPU load:TestPollerBroadcastsNewDatafailed 7 of 40 runs on unchangedmasterand 8 of 40 on this branch.gofmtandgo vetare clean.staticcheck -checks all: nothing;golangci-lintwith a strict linter set: findings addressed except ones that conflict with this package's conventions (t.Parallel,varnamelen,noctxin tests, unchecked deferredClose) and a G201 false positive, since only?placeholders and fixed fragments are formatted into the SQL./api/nodes?region=results compared against the old subquery on the live database: identical forEDI(320),EDI,GLA(321) andINV(235);EDI: +1 nodes from obs 2424851..2425078 in 2ms).EDIand 321 forEDI,GLA, matching the API, with no console or page errors;EDI,GLAload;regionto/api/nodes, so it is unaffected.master(093e320c), one commit.🤖 Generated with Claude Code