Repository navigation
Conversation
…pa-clawbot#2101) `/api/nodes?region=X` decided membership with an inline `public_key IN (SELECT DISTINCT from_pubkey FROM transmissions ⋈ observations ⋈ observers ...)` subquery: uncached, unbounded in time, and evaluated twice per request (COUNT and page). fetchAllNodes() pages at 500, so each Nodes or Map view ran it several times. On a 1.4 GB database (1.47M observations, 2 vCPU) one evaluation took 3-20 s depending on page cache, and one open tab kept the reader pool and both cores busy. The answer is small (7,333 node/observer pairs on that database) and changes slowly, so it is now derived once and kept, the way neighbor_edges is (Kpa-clawbot#1287, Kpa-clawbot#1339): - dbschema creates node_advert_observers(public_key, observer_idx, last_seen, last_obs_id), one row per observer that has heard a node's ADVERTs, and AssertReady requires it. - The ingestor is its only writer. Observations are folded in by observations.id in bounded steps (50k ids, ~0.3 s each): the first full build runs as an async migration, then a 30 s ticker folds new rows. PruneOldPackets drops pairs at the same cutoff that removes their newest advert. NULL from_pubkey rows use the pubkey BackfillFromPubkey will write. - The server reads the table once that migration is `done` (re-checked at most every 30 s, then latched); until then, on an ingestor that predates it, or on a v2 schema it keeps the legacy query unchanged. The region tag is joined from observers at query time, as before. Results match the legacy query; tests use it as the oracle on both modules. Measured on a snapshot of that database: backfill 3.6 s, every region and the all-regions set identical to the legacy query before and after a 3-day prune, table query 0.3-3 ms against 38-677 ms for the legacy query on the same (warm, faster) machine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 each Nodes view ran it several times. On the ScotMesh instance (1.4 GB database, 2.48M observation ids, 2 vCPU) one evaluation took 3–20 s depending on the page cache, and one open tab kept the reader pool and both cores busy. #2101 reports the same at larger scale.The answer is small and changes slowly: 7,333 (node, observer) pairs on that database. This PR derives it once in the ingestor and keeps it, the way
neighbor_edgesis (#1287, #1339). The server then reads a table the size of nodes × observers instead of scanning the advert history.What changes
internal/dbschemanode_advert_observers(public_key, observer_idx, last_seen, last_obs_id): one row per observer that has heard at least one of a node's ADVERTs,WITHOUT ROWID, with anobserver_idxindex.AssertReadyrequires the table (bug(db): server caches "scope_name column not present" on startup race with ingestor migration — /api/scope-stats 500s until server restart #1321).NodeAdvertObserversBackfillnames the backfill's_async_migrationsrow, which both binaries read.cmd/ingestor/node_advert_observers.gois the table's only writer.observations.id, in bounded steps of 50k ids. Each step reads a rowid range, withCROSS JOINpinning the plan, then upserts in oneWriterTx.RunAsyncMigration, so boot is not blocked and ingest interleaves between steps. After it, a 30 s ticker folds whatever has arrived.MAX(last_obs_id)after a restart, so there is no state table: the table is the persistence, as forneighbor_edges.PruneOldPacketsdrops a pair at the samefirst_seencutoff that removes its newest advert. Steps and that prune share a mutex, so a step cannot write back a pair from transmissions a concurrent prune has just removed.from_pubkeyis still NULL (legacy rowsBackfillFromPubkeyhas not reached) are recorded under the pubkey that backfill will write,extractPubkeyFromAdvertJSON(decoded_json).cmd/serverGetNodesusespublic_key IN (SELECT nao.public_key FROM node_advert_observers nao JOIN observers obs ON obs.rowid = nao.observer_idx WHERE UPPER(TRIM(obs.iata)) IN (…))once the backfill row isdone.Semantics. A pair exists while at least one ADVERT with a non-empty
from_pubkey, still in the database, was observed by that observer. That is the legacy subquery's condition. The region tag is joined fromobserversat query time, as before, so an observer that changes region is followed immediately. The one change is freshness: a node newly heard in a region appears within one tick (30 s) instead of on the next request. #1879 (attributing by home region) is neither addressed nor made harder: it would change what the pairs table records.Perf (rule 0).
node_advert_observersjoined toobservers.TestRegionFilterPlanAvoidsHistoryasserts the plan never touchesobservationsortransmissions.PREFLIGHT-MIGRATION-SCALE: <30s N=2.48M observation ids — backfill 12.0 s on the production 2-vCPU instance, 3.6 s on a snapshot of it
Measurements
These are from a
VACUUM INTOsnapshot of the production database, run locally against the realOpenStore, the builder andPruneOldPackets:PruneOldPackets(3)(251,559 transmissions, 3,805 pairs)On the production instance, running this change (built from
342e7c79; the final commit differs only in moving the backfill's name intodbschemaand adding tests):done; the server switched 7 s later/api/nodes?limit=500®ion=…warmtick: 8 pair updates in 37ms; ingestor CPU 16 s over its first 11 minutesOperator notes
[node-advert-observers] backfill: … pair updates in …,"node_advert_observers_backfill_v1" done, then[nodes] region filter now reads node_advert_observers (#2101)from the server. Until then the region filter behaves exactly as before, slow included.AssertReady). In the single-container image supervisord restarts the server if it wins the race against the ingestor'sdbschema.Apply. Split deployments must upgrade the ingestor with or before the server, as for everydbschemaaddition.PruneOldPacketsclears pairs left by prunes the older version made.Worth a decision
NodeAdvertObserversInterval(30 s), the 50k step size and the 50 ms yield are constants. The interval is the only one an operator might want to tune.GetNodes(bug: Regional /api/nodes queries exhaust the SQLite connection pool #2101 also notes this). The region filter no longer scans history, so I have not threaded the request context through; the builder's steps take a context and stop on shutdown.Validation
cmd/ingestor, 14 tests:from_pubkey, observations without an observer, untagged observers);from_pubkeyrows matchingBackfillFromPubkey;last_seenkept across steps;cmd/server, 7 tests:_async_migrationstable;internal/dbschema, 1 test: the table and index are created byApplyand required byAssertReady.last_seen;from_pubkeyskipped;go test -race ./...passes incmd/server,cmd/ingestor,internal/dbschemaandcmd/migrate; the new ingestor tests also pass 20× under-race.gofmtandgo vetare clean.staticcheck -checks all: nothing;golangci-lint: findings addressed except ones that conflict with this repo's conventions (noctx, unchecked deferredClose,t.Parallel).EDIand 321 forEDI,GLA, matching the API and the legacy query, with no console or page errors;EDI,GLAload took 4.4 s for the whole page, against 19 s with perf(nodes): cache region membership for /api/nodes?region= (#2101) #2102;regionto/api/nodes, so it is unaffected.master(093e320c), one commit.🤖 Generated with Claude Code