Repository navigation
fix(graph): deleting a stranded partition's RECORD finishes its teardown (#5073, defect 2) - #5203
Conversation
…own (#5073, defect 2) A partition root deleted before the structural teardown existed (#3436) left its schema, every row in it — NodeType definitions included — and its Admin/Partition record behind. Nothing could drop it: the ordinary delete needs a root, and the first child create into the partition (the compile watcher cutting a Release for a surviving NodeType row is one) makes EnsurePartitionBootstrap heal a Space root under System with no grant, so the partition comes back as a shell nobody can see, own or delete. Every instrument then reports it live and the bake sweep compiles its NodeTypes on every roll — which is why #5163's store witness, right for a DROPPED schema, is inert on UWDeepfield (its record still exists, v4). The record is the one artefact that outlives the data by design and lives in Admin, so a platform admin reaches it with the ordinary delete. StrandedPartitionRecordTeardownHandler makes `delete @Admin/Partition/{p}` run the same provider drop and cache eviction the root delete runs, under the same tombstone (#3451) — ONLY for a stranded partition: no root at all, or the bootstrap's own ownerless Space shell (its exact fingerprint, no grant, no GitSync). A live, owned partition keeps its store, so a global admin stays a platform admin and not a data superuser. The drop is extracted from PartitionDropPostDeletionHandler so the two teardowns cannot drift, and the record handler stands down while the ordinary teardown is on record. Controls (StrandedPartitionRecordTeardownTest, monolith mesh): rootless drops, ownerless shell drops, live plugin-like partition keeps its store, shell with one grant keeps its store, ordinary teardown drops exactly once, shell fingerprint member by member — 6/6; with the registration reverted the two positive cases fail (2/6), the four controls hold. A root-row witness in the sweep was tried and retracted: it contradicts AMissingRootOverALivePartition_IsStillHealed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical concurrency and ownership-safety issues can cause destructive cleanup.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Adds record-based teardown for stranded partitions, sharing cleanup with normal root deletion and documenting recovery.
Changes:
- Detects stranded partitions and drops their stores.
- Registers shared teardown handling.
- Adds integration tests and architecture documentation.
| File | Summary |
|---|---|
test/MeshWeaver.Graph.Test/StrandedPartitionRecordTeardownTest.cs |
Adds coverage for stranded, live, and duplicate-drop scenarios. |
src/MeshWeaver.Graph/Configuration/StrandedPartitionRecordTeardownHandler.cs |
Implements record-triggered cleanup. Findings: Critical (3 votes): deletion classification and claim are not atomic. Critical (1 vote): rootless partitions skip ownership checks. Moderate (2 votes): failed drops can leave no repair handle. Nit (1 vote): missing tests for static grants, GitSync, and indeterminate probes. |
src/MeshWeaver.Graph/Configuration/PartitionDropPostDeletionHandler.cs |
Shares provider-drop and cache-eviction logic. |
src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs |
Registers the new handler. |
src/MeshWeaver.Documentation/Data/Architecture/PartitionTeardown.md |
Documents the orphan recovery procedure. |
src/MeshWeaver.Documentation/Data/Architecture/AStaleIndexRowConfirmsItself.md |
Documents the defect, recovery verb, and limitations. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return StrandedBecause(partition) | ||
| .SelectMany(reason => | ||
| { | ||
| if (reason is null) | ||
| { | ||
| logger?.LogWarning( | ||
| "[StrandedPartition] the record 'Admin/Partition/{Partition}' was deleted by " | ||
| + "{User} but the partition is LIVE — it has a root somebody owns, or is served " | ||
| + "by configuration — so its backing store is NOT dropped. The partition is now " | ||
| + "missing from partition listings and the routing prime; to tear it down, " | ||
| + "delete its root node, which runs the structural teardown.", | ||
| partition, deletedBy ?? "system"); | ||
| return Observable.Return(Unit.Default); | ||
| } | ||
|
|
||
| logger?.LogWarning( | ||
| "[StrandedPartition] finishing the teardown of '{Partition}' on the deletion of its " | ||
| + "record by {User}: {Reason}. No ordinary delete could reach it — dropping its " | ||
| + "backing store on every provider and evicting its cached queries (#5073).", | ||
| partition, deletedBy ?? "system", reason); | ||
|
|
||
| // The same tombstone the ordinary teardown holds across its drop (#3451): marked | ||
| // before the first DDL so a child write landing mid-drop is refused by the write | ||
| // guard instead of healing a root back over a schema that is being dropped. | ||
| return Observable.Using( | ||
| () => registry.BeginSubtreeDeletion(partition), | ||
| _ => | ||
| { | ||
| registry.MarkDeleted(partition); |
There was a problem hiding this comment.
Agreed — the verdict was a reading taken at one instant and acted on at another, the #3451 shape. Fixed in the next push: the deletion scope (registry.BeginSubtreeDeletion(partition)) is now acquired FIRST and StrandedBecause runs inside it, so SubtreeDeletionGuardStorageAdapter refuses any write at or under the partition root between the reading and the drop — a recreate (an installer provisioning the same name, a child write healing a root) cannot land and then be dropped on a stale verdict. The tombstone checks at the top still stand down while a partition's deletion is in flight or on record.
What the claim does NOT make exclusive, stated rather than implied: BeginSubtreeDeletion is ref-counted, not a lock, so a root delete that STARTS after the claim proceeds too. The residue is two idempotent drops of one schema, serialised by the provider's own advisory lock (DROP SCHEMA IF EXISTS), which cannot destroy anything — the recreate window was the non-idempotent half, and the claim closes it. Also folded into the same push: this is now a DELETE VALIDATOR rather than a post-deletion handler (see the third thread), so the whole classify-and-drop runs before the record goes, with the record still in place as the retry handle.
| if (r.Node is null) | ||
| return Observable.Return<string?>( | ||
| "its root node is gone and no static root serves it — the shape a partition-root " | ||
| + "delete left behind before the structural teardown existed"); |
There was a problem hiding this comment.
Correct, and it is exactly the mechanism that produced the orphan: the recursive delete enumerates mesh_nodes descendants only, and _Access rows live in the access satellite table the fan-out never visits, so a rootless partition can still carry every grant it ever had — and a grant holder can still reach its nodes through the synthesized placeholder root and re-roots it on their next write. Such a partition is somebody's. Fixed in the next push: ownership is asked for BOTH root shapes. StrandedBecause now folds the root into a shape ("absent" / "the heal's shell") and only then asks the grant listing (durable) + static grants + _GitSync; any positive or indeterminate probe reads as owned and the store is preserved. New control DeletingTheRecordOfARootlessPartitionSomebodyStillHoldsAGrantOn_LeavesItsStoreAlone (rootless + one surviving durable grant → store kept, no drop in the ledger). The class remark's decision table is rewritten as the conjunction. Consequence, named on the thread and in the doc: if UWDeepfield still carries its old _Access rows the record delete refuses and says so, and the clean-up is then the grant holder's (or the stale grants go first) — which is the right answer under the platform-admin-is-not-a-data-superuser rule.
| logger?.LogWarning( | ||
| "[StrandedPartition] finishing the teardown of '{Partition}' on the deletion of its " | ||
| + "record by {User}: {Reason}. No ordinary delete could reach it — dropping its " | ||
| + "backing store on every provider and evicting its cached queries (#5073).", | ||
| partition, deletedBy ?? "system", reason); |
There was a problem hiding this comment.
Right — and the first fix I tried (a post-deletion handler that writes the record back on a failed drop) was refuted by its own test: a post-deletion handler runs INSIDE the record's deletion scope, where SubtreeDeletionGuardStorageAdapter refuses the write-back (REFUSED write to Admin/Partition/{p}: the subtree … is being deleted). So the next push takes your first option: the teardown is now StrandedPartitionTeardownValidator, an INodeValidator on NodeOperation.Delete, registered as a scoped validator next to OwnsPartitionProvisioningValidator (the creation-side mirror, which provisions the schema from a validator for the same reason). Delete validation runs before the deletion scope opens (step 2 vs step 3 in HandleDeleteNodeRequest), so the drop runs first and a drop that faults returns Invalid(...) naming the exception — the delete is refused, the record stays as the retry handle, the partition tombstone is lifted. That is the ordinary teardown's own ordering (store drop → definition delete, "so the partition remains visible for a retry"). Only a DIRECT delete of the record counts (DeleteCascadeRootPath == Node.Path); a record removed as a leaf of a wider recursive delete is bookkeeping going, not a request to drop every stranded schema. Control AFailedDrop_RefusesTheDelete_SoTheRecordStaysAsTheRetryHandle: provider drop faults → Success == false, error names "could not be dropped", record readable, store still provisioned, IsRecentlyDeleted(partition) == false.
…rop first, refuse on failure, ownership for both shapes (review on #5203) Three review findings, each real: 1. The verdict was taken before the deletion claim (#3451's shape). The scope is now acquired first and StrandedBecause runs inside it, so a recreate cannot land between the reading and the drop; a root delete starting after the claim is two idempotent, provider-serialised drops — named, not hidden. 2. A rootless partition can still carry every _Access grant it ever had (the satellite table the recursive delete never visits), and its grant holders reach it through the placeholder root. Ownership is now asked for BOTH root shapes; any positive or indeterminate probe keeps the store. 3. A post-deletion handler runs inside the record's own deletion scope, where the write guard refuses to write the record back after a failed drop — its own test caught it. The teardown is now StrandedPartitionTeardownValidator (INodeValidator, Delete): the drop runs before the record goes, and a drop that faults refuses the delete so the record stays as the retry handle — the ordinary teardown's own ordering, and OwnsPartitionProvisioningValidator's shape on the creation side. Only a DIRECT record delete counts. Controls: 19/19 across the four teardown classes; with the validator unregistered 3/8 fail (exactly the three positive cases), 5 controls hold. New: rootless + surviving grant keeps its store; a faulting drop refuses the delete with the record intact and the tombstone lifted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Shard 3 red on
So this is a suite sitting at 98.5 % of its cap on a runner 2 % slower — a capacity defect, filed into triage ( |
#5073, defect 2 — "The enumeration snapshot must not outlive the partitions it names"
Issue #5073 stays OPEN after this merge —
sev:His verified in production after the roll, never at a merge. This PR is the second half; #5163 was the first. (Worded this way on purpose: GitHub parses a negated closing keyword as a closing keyword, which is how the sibling issue was auto-closed at its PR merge and had to be reopened.)What is actually wrong, measured on the control instance after #5163 was live
Image
c55301f3(carries #5163 and #5152), replica booted 2026-09-22 09:03Z:get @Admin/Partition/UWDeepfield→ the record still exists (version: 4, last modified 2026-07-09).PartitionDropPostDeletionHandlerdeletes that record as its second step, so the teardown never ran for this partition — nothing dropped its schema./health→bake-reportstill listsframeworkstale in … UWDeepfield/…among 230 enumerated types; the sweep completed, 0 timed out. The rows are in a store the sweep (as System) reads, and the store witness — truthfully — confirms nothing absent. fix(mesh,hosting): a recycle and a bake must ask a survivor, not the thing that is dying (#5099, #5073) #5163 is inert on the partition it was written for.Releasenode created underUWDeepfield/Section/Release/…— a child create into the orphan.EnsurePartitionBootstrapheals a missing root on any child create into a partition whose store exists, System-attributed included (pinned:AMissingRootOverALivePartition_IsStillHealed, Top-level Space create is not atomic — churn leaves content-less, grant-less ghost roots #638/memex: platform Agents catalog is GONE — 'Agent' is a ghost partition root (exists to create, 404s to read, no version history); preInstalled is dead code #902). So the platform re-roots the orphan as aSpaceunder System with no grant: a shell nobody can see, own or delete, that every instrument reports as live.So the rows outlive the ROOT, not the store, and no witness the sweep could ask can tell "hasn't existed in ages" from "exists to every instrument" — by the platform's own definition. What was missing was a verb.
The change
StrandedPartitionTeardownValidator(INodeValidatoronDelete, registered byAddGraphnext to the structural teardown): deleting the partition's RECORD —delete @Admin/Partition/{partition}, reachable by a platform admin through the ordinary delete — finishes the teardown with the same provider drop and cache eviction the root delete runs (PartitionDropPostDeletionHandler.DropStores, now shared so the two cannot drift), under the same tombstone (#3451). A validator, not a post-deletion handler (review): the drop runs BEFORE the record goes, and a drop that fails REFUSES the delete so the record stays as the retry handle — the ordinary teardown's own ordering, andOwnsPartitionProvisioningValidator's shape on the creation side. The claim (BeginSubtreeDeletion) is taken first and the verdict inside it. Only a DIRECT record delete counts.Only for a STRANDED partition — the property that keeps a global admin a platform admin rather than a data superuser. Stranded is a CONJUNCTION (review): a root nobody can reach the partition through AND nobody who owns it:
Space, named after the partition, no content, created by System{p}/_Access(durable or static), no GitSync config — asked for BOTH root shapesaccesssatellite, so a rootless partition can still carry its grants, and its grant holders reach it through the placeholder rootAnything else (a real root, a surviving grant, a synced or static partition) is somebody's: the store is untouched and the record delete says so at Warning. Every probe fails closed (unreadable ⇒ owned). The validator stands down while the partition's own deletion is in flight or on record, so the ordinary teardown — which deletes this same record as its step 2 — never drops twice.
Controls, both sides (
StrandedPartitionRecordTeardownTest, monolith mesh, store stand-in as inPartitionResurrectionTest)PartitionResurrectionTest,PartitionTeardownCoverageTest,PartitionTeardownDropsAnchoredQueriesTest+ this class — Total: 19, Failed: 0.DocumentationLinkIntegrityTest— Passed (trx).MeshWeaver.Graph,MeshWeaver.Graph.Test,MeshWeaver.Documentation.Test—-c Release -warnaserror,0 Error(s)each.Retracted on the way
A root-row witness in the bake sweep ("a listing that does not hold the root is a real negative") was implemented, tested, and retracted: it contradicts the pinned heal invariant above, and it would have been inert on a re-healed
UWDeepfieldanyway. The doc page says why.Docs
Doc/Architecture/AStaleIndexRowConfirmsItself— the second defect, the measured state, why no witness can see an orphan, the verb and its decision table, what is still not fixed (the running-replica synced-query residue is named as NOT measured).Doc/Architecture/PartitionTeardown→ Auditing a portal for orphans now ends in the verb.Post-roll verification (the read that closes defect 2 — on the thread)
On the control instance, after the roll carrying this:
delete @Admin/Partition/UWDeepfield(a platform admin) → the log carries[StrandedPartition] finishing the teardown of 'UWDeepfield' …andDropped partition 'UWDeepfield' across N provider(s) …;get @Admin/Partition/UWDeepfield→ Not found; the next replica's/healthbake-reportno longer namesUWDeepfield/…, and itstotaldrops by that partition's NodeType count. If the log instead says "NOT stranded", the partition still carries a grant or was re-rooted and granted — then the grant holder tears it down through its root, or the stale grants are removed first and the record deleted again. If the delete is REFUSED naming a drop failure, the record is still there and the drop can be retried.Pairs-with: none — additive only (one new public validator type;
PartitionDropPostDeletionHandler's public surface is unchanged, two private members becameinternal static).🤖 Generated with Claude Code