Run VACUUM after a large DeleteJob purge to fix B-tree fragmentation - #73
Open
srhoods wants to merge 1 commit into
Open
Run VACUUM after a large DeleteJob purge to fix B-tree fragmentation#73srhoods wants to merge 1 commit into
srhoods wants to merge 1 commit into
Conversation
Purging a large job (millions of shards/splits/etc. rows deleted in one transaction) left the shards B-tree fragmented badly enough to visibly slow every later query against it. The existing auto_vacuum=INCREMENTAL pump (500 pages/30s) is sized for the Shard Reaper's steady trickle and doesn't catch up before the next large purge lands. DeleteJob now triggers a full VACUUM whenever the purge removes at least vacuumShardThreshold (1M) shard rows — below that, incremental vacuuming already handles it. Runs on its own goroutine (checkpoint TRUNCATE, then VACUUM) so the purge API call isn't held open for it, and coalesces concurrent/rapid DeleteJob calls onto a single pending run via a CAS-guarded flag, since a bulk purge can call DeleteJob once per matched job and VACUUM compacts the whole file regardless of which job triggered it — running it once per job in that loop would be pure waste. This is expensive by design, not despite the cost: VACUUM needs an exclusive lock for as long as the full-file rewrite takes (the same class of cost WALCheckpoint's TRUNCATE mode already documents at up to 144s for a much cheaper operation) and — since db is the sole writer connection — has to run there, so it holds mu and blocks every other write (and every rdb reader once the rewrite starts) for its complete duration. Moving it off the request path doesn't remove that cost, only defers it to a moment the caller isn't blocked on it. Tests cover the threshold gate, that VACUUM actually reclaims space (freelist_count -> 0), and that concurrent large purges coalesce into exactly one run — each verified by deliberately breaking the logic first and confirming the test fails for the expected reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
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.
Summary
shardsB-tree fragmented enough to visibly slow later queries — the existingauto_vacuum=INCREMENTALpump (500 pages/30s, sized for the Shard Reaper's steady trickle) doesn't catch up before the next large purge lands.DeleteJobnow triggers a fullVACUUMwhen a purge removes at leastvacuumShardThreshold(1M) shard rows; smaller purges are left to the existing incremental pump.DeleteJobcalls onto a single pending run — a bulk purge (POST /api/v1/jobs/purge) can callDeleteJobonce per matched job, and since VACUUM compacts the whole file regardless of which job triggered it, running it once per job would be pure waste.docs/DESIGN-coordinator.md§3 documents the tradeoff, including that this genuinely does holdstore.mu(and therefore blocks the whole coordinator's writes, plus reads once the rewrite starts) for its full duration — moving it off the request path defers that cost, it doesn't remove it.Test plan
gofmt -l .cleango vet ./...cleango test -count=1 ./...— all greengo test -raceonstore/api(skipping one unrelated pre-existing slow scale test that also times out under-raceon unmodified master) — clean, no racesTestDeleteJobSkipsVacuumBelowThreshold,TestDeleteJobVacuumsAboveThreshold,TestDeleteJobCoalescesConcurrentVacuums) each verified via deliberate-breakage falsification: reverted the threshold gate / CAS coalescing, confirmed the corresponding test fails for the expected reason, restored🤖 Generated with Claude Code
https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm