perf(worker): keyset-paginate quota reconciler scans - #81
Merged
Conversation
The suspend / unsuspend / redis-eviction loops in EnforceStorageQuotaWorker each issued ONE unbounded `SELECT ... FROM resources` per tick, materialising the entire active/suspended (or active-redis) set into a single result set + allocation. On a table that grows to tens of thousands of rows that pins a multi-MB result set and a long-lived server cursor. Each loop now streams the eligible set in keyset-paginated batches of quotaScanBatchLimit (1000), advancing the cursor by the last id::text and stopping on a short page — exactly mirroring orphan_sweep_reconciler's fetchLiveStackIDs. The loops still process the WHOLE eligible set every tick (no silent row-dropping); only the per-fetch footprint is bounded. Keyset (id::text > $cursor ORDER BY id::text ASC) rides the PK, is restart-safe, and never re-scans or drifts under concurrent insert/delete the way OFFSET would. Every existing side effect (revoke/grant, suspend/unsuspend CAS, audit emit, hysteresis, intra-tick flap skip-set) is preserved. Batch size 1000: cheap small-projection scan; per-row work is gated independently so a 1000-row page drains a large table in a few round-trips. Tests: quota_keyset_test.go adds multi-page-advance, second-page-error, and mid-stream rows.Err() coverage for all three loops (mirrors the orphan_sweep keyset tests). Existing sqlmock expectations updated to the new WithArgs(status, cursor, limit) shape via the exported QuotaScanBatchLimit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
enabled auto-merge (squash)
June 3, 2026 19:28
mastermanas805
added a commit
that referenced
this pull request
Jun 4, 2026
…#3) The storage_bytes scanner queried only status='active', so a quota-suspended resource's usage was never re-measured — runUnsuspendLoop never saw it drop under cap and the resource stayed suspended forever, breaking the suspend email's promise that "access is restored automatically once usage drops". Fix: scan `status IN ('active', 'suspended')`. Minimal one-line change — the broader keyset-pagination rewrite was dropped from this PR (the other reconciler scanners already got keyset in #81/#82; this scanner's pagination is a separate concern and is left for its own PR to keep this fix small and fully covered). Test: TestUpdateStorageBytesWorker_RemeasuresSuspendedRow pins the WHERE clause to ('active','suspended') so dropping 'suspended' reds the build, and asserts a suspended row is re-measured + its storage_bytes updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 4, 2026
…#3) (#85) * fix(quota): re-measure suspended resources so they can auto-unsuspend Finding #3 (SWEEP-BACKLOG-2026-06-04, P1). UpdateStorageBytesWorker scanned only status='active' resources, so a quota-suspended resource's storage_bytes was frozen at the over-cap value forever. EnforceStorageQuotaWorker's runUnsuspendLoop reads that persisted column (readStorageBytes) to decide whether usage has dropped below the hysteresis threshold — with the value frozen it never could, so a suspended resource stayed suspended permanently. The suspend email promises "access restored automatically once usage drops"; that was a no-op. Fix: the scanner now selects status IN ('active','suspended') so suspended rows keep being measured. Suspend-trigger behaviour for active rows is unchanged — runSuspendLoop independently scans status='active'; this worker only writes the storage_bytes column both loops read. Since this touched a previously-unbounded scan (ORDER BY created_at, no LIMIT), it is now keyset-paginated (id::text > cursor ORDER BY id::text, batch 1000), mirroring the quota.go reconciler scans. Scan errors stay fail-open (CLAUDE.md #1): log + stop paginating this run (cursor can't advance without a valid id), re-run next tick. The api half (ElevateResourceTiersByTeam tier-upgrade rescue, finding #4) is fixed separately in the api repo. Tests: - TestUpdateStorageBytesWorker_RemeasuresSuspendedRow — asserts the scan status args are exactly ('active','suspended') and a suspended row is re-measured + its storage_bytes updated (measurement half). - TestEnforceStorageQuotaWorker_UnderQuota_UnsuspendsResource (existing) — once storage_bytes drops, the row is unsuspended (release half). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(quota): scan suspended resources so they can auto-unsuspend (sweep #3) The storage_bytes scanner queried only status='active', so a quota-suspended resource's usage was never re-measured — runUnsuspendLoop never saw it drop under cap and the resource stayed suspended forever, breaking the suspend email's promise that "access is restored automatically once usage drops". Fix: scan `status IN ('active', 'suspended')`. Minimal one-line change — the broader keyset-pagination rewrite was dropped from this PR (the other reconciler scanners already got keyset in #81/#82; this scanner's pagination is a separate concern and is left for its own PR to keep this fix small and fully covered). Test: TestUpdateStorageBytesWorker_RemeasuresSuspendedRow pins the WHERE clause to ('active','suspended') so dropping 'suspended' reds the build, and asserts a suspended row is re-measured + its storage_bytes updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <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
1 participant
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.
What
The three periodic scan loops in
EnforceStorageQuotaWorker(quota.go) eachissued ONE unbounded
SELECT ... FROM resourcesper tick:runSuspendLoop—status='active'postgres/redis/mongodbrunUnsuspendLoop—status='suspended'postgres/redis/mongodbrunRedisEvictionLoop—status='active'redisEach materialised the entire eligible set into one result set/allocation. This
PR converts all three to keyset pagination (batches of
quotaScanBatchLimit= 1000), exactly mirroring the already-merged
fetchLiveStackIDsloop inorphan_sweep_reconciler.go:WHERE id::text > $cursor ORDER BY id::text ASC LIMIT $n, advance cursor by last id, stop on a short page.The loops still process the WHOLE eligible set every tick — this is not a
bare
LIMITthat drops rows. Only the per-fetch footprint is bounded.Coverage block
Notes
re-check + optional revoke/grant) is gated independently.
concurrent insert/delete. A row inserted mid-sweep sorts either after the
cursor (seen) or before it (already seen) — never dropped.
hysteresis dead-band, intra-tick flap skip-set).
make gategreen locally.🤖 Generated with Claude Code