Repository navigation
fix(#6351): a recursive delete's commit fans out through one bounded lane; a leaf's refusal keeps its reason - #6357
Conversation
…lane The bottom-up walk posted one leaf delete per planned path at once. Every leaf's commit ended on the process-wide cap-1 pg write pool, and each leaf's own no-progress watchdog counted its wait behind its siblings' writes as a stall, so a wide delete (Marketing, 1,383 paths; SocialMedia, 1,060) failed on a leaf while the cascade was progressing. The commit's legs now share one admission lane for the whole tree (HierarchicalPathDeletion.DeleteSubtreeBounded, MeshOperationOptions.CascadeFanOutConcurrency, default 64 like the pre-flight). A failed leaf's own NodeDeletionRejectionReason now rides up with the cascade's wrapper, so an Unavailable leaf is no longer reported as ValidationFailed / "Unexpected error" and surfaced by MeshService.DeleteNode as an UnauthorizedAccessException. Refs #6351, Refs #1198 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The retype regression test does not deterministically exercise the failure path, and new mutable collections violate repository rules.
3 open findings
What changed in this PR
Bounds recursive-delete commit fan-out to prevent self-induced write-pool starvation while preserving leaf rejection reasons.
Changes:
- Adds a tree-wide bounded deletion lane and configurable concurrency.
- Preserves leaf failure classification through cascade wrappers.
- Adds regression tests and operational documentation.
| File | Description |
|---|---|
HierarchicalPathDeletionTests.cs |
Tests bounded traversal behavior. |
NodeTypePathOccupancyTest.cs |
Restores the retype-repair race test. |
DeleteDrainCompletionTest.cs |
Reproduces serialized write-pool pressure. |
MeshOperationOptions.cs |
Adds cascade concurrency configuration. |
MeshExtensions.cs |
Applies bounded deletion and preserves rejection reasons. |
HierarchicalPathDeletion.cs |
Implements the shared admission lane. |
RecursiveDeleteDrain.md |
Documents evidence, cause, and fix. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Straight after — no wait for the rebind recycle to finish, which is the whole point. | ||
| var repaired = await Upsert(Instance(id, "Markdown"), allowUnresolvable: false); |
There was a problem hiding this comment.
Agreed that this fact cannot prove the re-drive happened: whether the second write lands inside the teardown is timing, so it can stay green without the re-drive. It is the end-to-end repro that #6034 named as its close condition, and it is not meant as the pin. The re-drive is pinned deterministically in MeshWeaver.FaultInjection.Test (FleetWatchFreezeTests: mesh.Linger(…) holds the owner in its stop and the write must be re-driven past it; that is #5011's case, which failed with exactly Unknown … Rejecting now before the fix). Fixed in 7af86e7: the doc comment now says this and points at that pin, so nobody reads this fact as the guard.
| private static readonly string[] TwoWideBranches = Enumerable.Range(0, 10) | ||
| .SelectMany(i => new[] { $"root/a/{i}", $"root/b/{i}" }) | ||
| .Concat(new[] { "root/a", "root/b" }) | ||
| .ToArray(); | ||
|
|
||
| private static string[] GatedLeaves => TwoWideBranches.Where(p => p.Count(c => c == '/') == 2).ToArray(); |
There was a problem hiding this comment.
Fixed in 7af86e7: both lookups are now ImmutableArray<string>, and GatedLeaves is computed once instead of being re-materialised per access.
Test Results 19 files 19 suites 49m 25s ⏱️ Results for commit 7af86e7. ♻️ This comment has been updated with latest results. |
Refs #6351, Refs #1198, Refs #6034
What broke
On the control instance, two bulk recursive deletes failed today, 45 minutes apart:
Marketing, after removing 1,383 paths, andSocialMedia, after removing 1,060. Each failure was a single leaf whose own commit stage "made no progress for 25s — 0 of 1 planned". The write pool was saturated at the time:pg:Postgres(cap 1)had 219 and 764 waiting, and more than 1,000 admissions waited at least 1 s within the stage.The governed
LogsreadOps/Actions/logs-sevM-a4315-20261009-q1found 194 such leaf timeouts inside one second, all underSocialMedia/. The backlog was the delete's own leaf writes, not unrelated writes from other partitions.Root cause
HierarchicalPathDeletionfans every sibling set out with an unboundedMerge, and the levels nest. So the commit posted one leaf delete per planned path all at once. Each leaf re-entersHandleDeleteNodeRequestat its own hub and opens its own no-progress watchdog, which starts when that hub takes the request. Each leaf then ends in one write on the single process-wide cap-1 pool. A leaf deep in that self-made queue saw no progress for its whole budget, even though the cascade was removing rows steadily. Its timeout then failed the whole delete.The pre-flight has been capped at 64 for this reason all along (
PreValidateFanOutConcurrency). The commit was not.Fix
HierarchicalPathDeletion.DeleteSubtreeBounded, sized byMeshOperationOptions.CascadeFanOutConcurrency(default 64, the pre-flight's value). A per-level cap does not work because nested levels multiply.NodeDeletionRejectionReasonnow rides up with the cascade's wrapper. Before this, the root read the wrapper's type as the reason:ValidationFailedplus anUnexpected error:prefix.MeshService.DeleteNodeturned that into anUnauthorizedAccessException, which is what the delete-space script logged for an availability failure.Tests (negative controls run)
WideDeleteBoundedFanOutTest/WideDeleteUnboundedFanOutTest(Graph.Test): a 160-leaf root over a storage adapter that serves deletes one at a time at 50 ms (the cap-1 pool), against a 2.5 s leaf budget.made no progressline.got UnauthorizedAccessException, which is the production shape.HierarchicalPathDeletionTests(Hosting.Test):NodeTypePathOccupancyTest.Upsert_RepairIssuedStraightAfterAStrandingRetype_Landsre-adds the back-to-back retype that the filing removed. It is green in the full Graph.Test project run, which is the run that originally failed.Docs:
Doc/Architecture/RecursiveDeleteDraingains a section with the 2026-10-09 reading on the producing portal and the fix.Pairs-with: none — only public members are ADDED (
HierarchicalPathDeletion.DeleteSubtreeBounded,MeshOperationOptions.CascadeFanOutConcurrency); nothing is removed or renamed.Deploy: host code, so no recycle is needed. Verify on memex.systemorph.com once an image carrying this rolls: a bulk recursive delete of a 1,000-path space no longer produces
[DeleteNode:commit] … 0 of 1 plannedlines.🤖 Generated with Claude Code