Fan out orphan-directory deletion across the fleet - #67
Merged
Conversation
Today, a single large orphan directory is removed depth-first by whichever one agent's shard named it — the delete-pass analogue of the entry-list problem (a directory too big for one shard), but with no equivalent fix. On a tree where the orphan set is dominated by a handful of large stale subtrees, this makes the delete pass take as long as the rest of the job combined, with the whole fleet idle except the one agent working through it. Mirrors split_entrylist_stream (the entry-list fan-out used during SCANNING) more closely than plain directory-split, since delete has nothing to diff against a destination — the whole subtree is already condemned (D5) — so a batch is just names to remove: - Wire: new ShardSplit.DeleteRemainder (field 7, additive) carries a dir_rel + batch of names, same shape as NewEntryList. total_children is set only on the last batch streamed for a directory — the agent learns the true count only once readdir hits EOF, unlike a chunk group's upfront byte-size-derived n_chunks. - Agent: remove_orphan (agent/src/delete.c) probe-reads a top-level orphan directory against tuning.delete_split_threshold; over that, stream_delete_split streams it out in tuning.delete_split_batch batches instead of unlinking inline. ship_split/drain_splits (the ack/backpressure machinery every split kind already shared) moved out of walker.c into a new split.c so delete.c can use them too. - Coordinator: a new delete_groups table (the delete-pass analogue of chunk_groups) tracks n_total/n_done/closed per directory. A split-produced child's own completion is an independent frame from the streaming parent's batches, so a child can complete before the coordinator has even recorded the group's true total — CompleteDeleteRemainder and RecordSplit each independently check "is this group now closeable," whichever lands last seeds the one-entry cleanup shard that removes the now-empty directory, and closed stops the other from seeding a second one. - New tuning.delete_split_threshold (200_000) / delete_split_batch (20_000), independently tunable from the entry-list knobs since delete work per name is a plain unlink, not a full copy pipeline. New coordinator-side tests cover the ordinary case, the child-vs-final- batch race, and double-seed prevention — each verified to fail against a deliberately broken version before asserting the fix. New test/delete_fanout_e2e.sh drives the real agent+coordinator end to end; not runnable on this dev host (a pre-existing live drsyncd deployment's /etc/drsync config makes every e2e script here fail identically, confirmed against an unmodified scale_e2e.sh), so CI is the first real run of it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
Local e2e verification (delete_fanout_e2e.sh) surfaced two bugs the unit tests never touched: 1. delete_groups.n_total was wired to total_children (the directory's entry count) while n_done counts completed split-produced shards (batches) — comparable only by coincidence, so the group never closed and the DELETE phase silently "completed" without seeding the cleanup shard. n_total now counts shards, one bump per DeleteRemainder batch, with streaming-finished tracked separately in a new done_streaming column. 2. onShardSplit passed DeleteRemainder's bare readdir basenames straight into DeleteBatch.RelPaths instead of joining them under dir_rel, so a split-produced delete shard tried to unlink e.g. "f1.txt" at the destination root (a silent ENOENT, not an error) and never touched the actual file under the orphan directory. Added TestDeleteRemainderPathsJoinDirRel to catch #2 at the onShardSplit level, since the existing delete_groups tests only drive store.RecordSplit/CompleteDeleteRemainder directly with pre-built shards. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
srhoods
added a commit
that referenced
this pull request
Aug 11, 2026
…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
srhoods
added a commit
that referenced
this pull request
Aug 12, 2026
…rectory (#69) * Add work-budget-based DELETE fan-out for trees with no single wide directory 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 * Add missing schema migrations for delete_groups.done_streaming/pending_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 --------- Co-authored-by: Claude Sonnet 5 <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.
Summary
Today, one large orphan directory is removed depth-first by whichever single agent's shard named it — no fan-out equivalent to the entry-list mechanism scanning already has. On a tree where the orphan set is dominated by a handful of large stale subtrees (common right after a reorg), this makes the delete pass take as long as the rest of the job combined, with the whole fleet idle except one agent.
ShardSplit.DeleteRemainder(field 7, additive) —dir_rel+ batch of names, mirroringNewEntryList.total_childrenset only on the last batch for a directory (the agent only knows the true count oncereaddirhits EOF).remove_orphanprobe-reads a top-level orphan directory againsttuning.delete_split_threshold; over that,stream_delete_splitstreams it out intuning.delete_split_batchbatches instead of unlinking inline. Extracted the shared split ack/backpressure machinery (ship_split/drain_splits) out ofwalker.cinto a newsplit.csodelete.ccan use it too.delete_groupstable (the delete-pass analogue ofchunk_groups) tracks completion per directory and seeds a cleanup shard once every split-produced child is done — handles the real race where a child can complete before the streaming parent's final "total count" batch even arrives.tuning.delete_split_threshold(200,000) /delete_split_batch(20,000), independently tunable from the entry-list knobs.Test plan
go build ./...,go vet ./...,gofmt -l .cleango test ./...full coordinator suite green;-race -shortcleanmake -C agent test(all existing unit suites) greenTestDeleteGroupSeedsCleanupOnceAllChildrenDone,TestDeleteGroupHandlesChildCompletionRacingFinalBatch,TestDeleteGroupNeverSeedsCleanupTwice) — each verified to fail against a deliberately broken version before asserting the fixtest/delete_fanout_e2e.sh— not runnable on this dev host (a pre-existing live drsyncd deployment's/etc/drsyncconfig makes every e2e script here fail identically, confirmed against an unmodifiedscale_e2e.sh), so CI is the first real run — please watch this leg specifically🤖 Generated with Claude Code
https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm