Add work-budget-based DELETE fan-out for trees with no single wide directory - #69
Merged
Conversation
…rectory The prior fix (#68) caught a directory that is itself WIDE at any depth, but a real production tree hit a shape that mechanism structurally cannot see: 77 top-level branches, several levels deep, every individual directory comfortably under any threshold. No directory anywhere in the tree was ever wide enough to trip delete_split_threshold, so the whole multi-million-object tree ran serially inside 2 shards. Adds tuning.delete_shard_budget (default 250000, objects removed), mirroring the scan walker's shard_budget/queue_split: agent/src/ delete.c decrements a per-shard budget on every object removed, threaded through the recursive descent. Once exhausted, every not-yet-opened subdirectory is handed off as its own new top-level DELETE shard (ShardSplit.delete_subdirs) instead of being recursed into. This introduced two more completion-ordering bugs, both caught locally by the new delete_fanout_budget_e2e.sh before reaching CI: 1. A handed-off shard's own single-shard "group" was seeding a redundant cleanup rmdir on top of its own ordinary removal — fixed with a NoSelfCleanup flag on DeleteGroupTotal. 2. A directory that was never itself handed off, but merely an ancestor of one deep in an otherwise-inline rm_tree recursion, had no way to know a descendant was still mid-removal elsewhere, and rmdir'd itself prematurely. Fixed by having the agent track this locally (no coordinator round-trip): a handoff anywhere inside an in-progress rm_tree call propagates back up the C call stack, and every ancestor that sees a deferred descendant skips its own rmdir too, reporting the deferred paths once in its own ShardResult (deferred_rmdirs, new proto field). Coordinator-side, registerPendingChildTx now walks every ancestor up to the top-level orphan path (not just the direct parent) the first time anything beneath it is handed off, stopping the climb the moment it reaches an already-tracked level — an actual over-counting bug (pending_ children reaching 69 instead of 10) was caught and fixed during this same investigation before that stop condition was added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
…g_children delete_groups' CREATE TABLE IF NOT EXISTS predates both columns (done_streaming from #67, pending_children from #68/this PR) and is a no-op against an already-existing table, so a coordinator whose data-dir was created by an older binary hit "SQL logic error: no such column: pending_children" on its very first delete pass after upgrading — reported live against exactly that scenario. Neither column had a corresponding ALTER TABLE entry in the migrations slice. Added both, plus a regression test that builds a pre-migration delete_groups table by hand and confirms Open (the real migration path) adds both columns and the table is fully usable afterward. Verified end-to-end against a fresh coordinator combining both fan-out mechanisms (a wide directory nested inside a 77-way branching tree, matching the reported production shape) with the exact tuning values reported: delete_split_threshold=1000, delete_split_batch=2500, delete_shard_budget=1000 — fan-out fires, the tree is fully removed, and content stays intact. 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
delete_split_threshold.tuning.delete_shard_budget(default 250000, objects removed), mirroring the scan walker'sshard_budget/queue_split: once a shard's budget runs out mid-descent, every not-yet-opened subdirectory is handed off as its own new top-level DELETE shard instead of being recursed into.rmdiron top of its own ordinary removal (NoSelfCleanupflag).rm_tree) had no way to know a descendant was still mid-removal elsewhere andrmdir'd itself prematurely — fixed with agent-local tracking (ShardResult.deferred_rmdirs, no coordinator round-trip) plus a coordinator-side ancestor-registration walk inregisterPendingChildTx(which itself had an over-counting bug, caught and fixed in the same investigation).delete_fanout_budget_e2e.sh: a 10×10 branching tree, 5 files per leaf, every directory capped at 10 entries — the only way to exercise the budget mechanism specifically, since nothing in this tree is ever wide enough to trip mechanism 1.Test plan
go test -count=1 ./...— all greenmake -C agent test— all greenNoSelfCleanupand the ancestor-registration chain via the existingdeletefanout_test.gosuite (all still pass)delete_fanout_e2e.sh(flat WIDE case) — still passesdelete_fanout_nested_e2e.sh(nested WIDE case) — still passesdelete_fanout_budget_e2e.sh(new: branching, never-wide case matching the production shape) — passes, all 10 b-dirs / 100 c-dirs / 500 files fully removed🤖 Generated with Claude Code
https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm