perf(worker): keyset-paginate TTL expiry reaper scans - #82
Merged
Conversation
ExpireAnonymousWorker.Work and ExpireStacksWorker.Work each issued ONE unbounded batch SELECT per tick, materialising the entire expired set into a single result set/allocation before reaping. A backlog (e.g. a provisioner outage that stalled many teardowns, or a flood of expired anon stacks) pins a multi-MB result set in one shot. Both reapers now stream candidates in keyset-paginated batches, advancing the cursor by the last id::text and stopping on a short page — exactly mirroring orphan_sweep_reconciler's fetchLiveStackIDs. The reapers still process the WHOLE expired set every tick (the complete candidate list is assembled across pages, then every candidate is reaped); 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. Every side effect is preserved: expire.go keeps the per-row FOR UPDATE re-confirm + idempotent deprovision + mark-deleted (MR-P0-1a / MR-P1-5 race guards intact); expire_stacks.go keeps the in-cluster namespace teardown + not-in-cluster skip + DELETE ordering. Batch sizes are deliberately SMALL — expire.go=100, expire_stacks.go=50 — because each candidate triggers a real backend teardown (provisioner DeprovisionResource RPC / k8s namespace DELETE). Tight batches keep each tick's burst of teardown calls bounded so a large backlog cannot thundering-herd the provisioner / k8s API in one tick. Tests: expire_keyset_test.go adds multi-page-advance, second-page-error, and mid-stream rows.Err() coverage for both Work() scans, mirroring the orphan_sweep keyset tests. Existing query-regex expectations are unaffected (they match no args and return short pages). New limits exported via export_expire_test.go to avoid magic numbers in the external test package. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
enabled auto-merge (squash)
June 3, 2026 19:34
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
ExpireAnonymousWorker.Work(expire.go) andExpireStacksWorker.Work(
expire_stacks.go) each issued ONE unbounded batchSELECTper tick,materialising the entire expired set into a single result set/allocation
before reaping. This PR converts both to keyset pagination, mirroring the
already-merged
fetchLiveStackIDsloop inorphan_sweep_reconciler.go.Both reapers still process the WHOLE expired set every tick (the full
candidate list is assembled across pages, then every candidate is reaped) —
not a bare
LIMITthat drops rows.Coverage block
Notes
candidate triggers a real backend teardown (provisioner
DeprovisionResourceRPC / k8s namespace DELETE). Tight batches avoid a thundering herd against the
provisioner / k8s API on a large backlog.
re-confirm + idempotent deprovision + mark-deleted (MR-P0-1a / MR-P1-5 race
guards); expire_stacks.go keeps in-cluster teardown + not-in-cluster skip +
DELETE ordering.
make gategreen locally.🤖 Generated with Claude Code