perf(api): buffer usage-rollup observations in the isolate (#8823) - #8946
Conversation
Every /api/v1/* request drove one unauthenticated Postgres write on the self-hosted indexer box. recordUsageRollup called foldObservations with a single-element array and POSTed immediately, so N requests meant N DATA_API subrequests, N postgres() clients (withAccountsSql builds a fresh one per invocation), and N upserts. handleUsageRollupIncrement's own comment claimed the caller coalesced observations first; nothing did. Unmatched paths were the worst case: routeFamily collapses every non-matching path to one "unmatched" bucket, so a flood of /api/v1/<random> 404s serialised on a single row lock -- reached through the generic 404, which passes no rate limiter. Observations now accumulate in a module-scope buffer and flush on 64 observations or 10 seconds, whichever comes first, as one subrequest carrying one bucket per (day, route_family, cost_shape). The flush rides the triggering request's waitUntil, so it still adds no latency, and the buffer drains before the fetch so a concurrent request cannot re-send it. request_count/keyed_count keep their exact meaning -- batched, never sampled. The one new loss mode is isolate eviction with a partial buffer, bounded at 63 observations, which under-counts and never over-counts. ADR 0026 records why sampling and a log-derived rollup were rejected, why unmatched paths are still counted, and why withAccountsSql's per-invocation client is split out. Closes #8823
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
metagraphed-registry-sync-api | f6ccbe5 | Jul 31 2026, 03:48 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
metagraphed-data-api | f6ccbe5 | Jul 31 2026, 03:47 PM |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 15:58:03 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Summary
/api/v1/*request drove one unauthenticated Postgres write on the self-hosted indexer box. Observations are now buffered in the isolate and flushed in batches.Decision: Option A — isolate-scoped buffering, with a count-or-age flush
Written up as ADR 0026 (the change alters the accounting contract's loss modes, so it warranted one rather than just a PR paragraph). In brief:
waitUntilcan carry it.ctx.waitUntil, so it still adds no latency — same posture as the single-observation write.Why not B (sampling): it turns
request_countinto an estimate, and the table exists to answer ADR 0022's "does the free tier cost too much" — a figure someone multiplies by a rate. The scale factor would also have to be versioned into the rows to stay interpretable across a threshold change. Buffering gets the same write reduction without giving up exactness.Why not C (derive it off the request path): Logpush / Analytics Engine / PostHog all carry request volume, but none carries
route_family— that label comes fromrouteFamily()matching the Worker's own dispatch order, which is what makes the rollup attribute a request to the route that actually served it. Reconstructing it from a log stream means reimplementing that matcher in a second place and keeping the two in step forever, plus trading freshness for a scheduled job and adding a dependency to a table that has none. Still a reasonable future option; not worth its cost today.The other four things the issue required settled
(day, "unmatched", "edge")the worst case is now what makes it the cheapest: 64 distinct junk paths in one batch cost one bucket and one row-lock acquisition. Asserted in a test.withAccountsSqlbuilding a newpostgres()client per invocation is independent of which option was chosen and affects every accounts-tier route, including the awaitedhandleApiQuotaSpendthat fails open. Changing client lifetime inside a usage-rollup PR would silently alter routes this PR does not test. Buffering already removes 63 of every 64 client constructions on this path.workers/data-api.ts's "BATCHED on purpose. The caller coalesces a request's observations…" described nothing — the caller folded a one-element array. Rewritten to describe the buffer that now exists, including that every bucket in a batch is upserted inside onewithAccountsSqlcall, so one client serves the whole batch.request_count/keyed_countkeep their exact meaning — exact counts of observed requests, batched, never sampled or scaled. Nothing about existing rows changes. The only new behaviour is the eviction loss above, stated here and in the ADR.Tests
tests/usage-rollup.test.ts— a newrecordUsageRollup batches its writes (#8823)block; 6 cases fail against the unbuffered writer and pass with it:ceil(N / 64)writes, not N. 63 observations write nothing; the 64th trips one subrequest carrying one bucket withrequest_count: 64(not 64 buckets); 128 ⇒ exactly 2. Also asserts the buffer is drained, not copied./api/v1/<junk>paths ⇒ one subrequest, one bucket,family === UNMATCHED_FAMILY. This is the abuse case the issue is about.keyed_countexact), and distinct families become distinct buckets in one write.Date.nowstubbed.handleRequestwith a stubbedDATA_API, driving 64 real unmatched/api/v1/*requests — the case the issue asked for explicitly, since that path reaches the generic 404 without passing any rate limiter — plus a check that an OPTIONS preflight is still not counted.The existing
#8597tests now flush explicitly (they assert what is recorded, not when it is written);flushUsageRollupandusageRollupBufferSizeare exported for that. The buffer registers withsrc/module-state-registry.tslike every other isolate-scoped memo, so a leftover observation cannot shift the next test file's flush boundary.Registry Safety
Closes #8823) — required.bucketsarray.)Validation
npm run lint·npm run format:check·npm run typechecknpm run validate·npm run validate:docsnpx vitest run tests/usage-rollup.test.ts— 29 passednpx vitest run tests/api-coverage.test.ts tests/wallet-auth-keys-route.test.ts— 415 passed (the other suites that exercise this hook)git diff --check