Skip to content

Teardown lets accepted work finish and names what it had to stop — no forced teardown - #3291

Merged
meshweaver-cloud[bot] merged 5 commits into
mainfrom
fix/robust-disposal
Sep 4, 2026
Merged

meshweaver-cloud[bot] merged 5 commits into
mainfrom
fix/robust-disposal

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Why

The red runs in the last 130 (MeshWeaver Build and Test) split into two families. Five were the AGENTS.md shared rule blocks gate — a satellite added the rest-not-graphql block before core's manifest declared it — cured by #3287. The rest are teardown-shaped flakes: NackReachesTheWaiterDuringTeardownTest (run 33847949620, docs-only PR #3286), PodHubTransportTest.CrossSiloNack… (merge-queue run for the docs-only #3285), and their earlier sibling LateNackReenqueueTest.

The mechanism, from the passing trace beside the failing one: the owner's merge turn is parked, the sync hub cannot process its ShutdownRequest, and after 8 s the disposal watchdog force-tore the sync hub down from its timer thread; only then could the owner reach ShutDown and mint the verdict the waiter was armed for — 3 ms later locally, but the assertion window was 10 s, i.e. "the watchdog plus two seconds", and a loaded CI shard lost those two seconds.

Production shows the same instrument doing worse: Loki on memex-cloud (2026-08-29 → 09-03) has DISPOSAL DEADLOCK DETECTED: Hub sync/… RunLevel=ShutDown → [FORCE-TEARDOWN] … out-of-band teardown complete after 22706ms dozens of times per shutdown, on sync hubs and the node hubs above them, and once on a pod that was not shutting down. A hub that reports itself disposed while its work is still running is exactly "disposal seems to be keeping something".

Maintainer directives this implements (2026-09-04): let accepted work — activities, pooled I/O, handler turns — finish its job; never cancel on entry; no forced teardown, not in prod and not in tests; detect a wedge as no progress for a budget, cancel it once cooperatively, and log every such kill or discard at Error with enough detail to reproduce, so the red-log triage files it as an issue (prod only; tests just log).

What changes

Hub (MessageHub / MessageService)

  • Dispose() no longer calls CancelExecution(); the ShutdownRequest queues FIFO behind accepted work and the pump drains it.
  • The watchdog is now a stall detector (OnDisposalStall): re-armed on every subtree RunLevel transition (unchanged), fires every 8 s of no progress, and reaches one of five verdicts — busy (turns completing, Information), wedged turn (cancel once, Error 7311 with the turn, its age, queue depth and the recursive snapshot), ignores cancellation (Error 7312 every budget, disposal stays pending), ShutDown phase blocked (Error 7314 — a registrant blocking inside DisposeImpl; the exact shape Loki showed), stalled below (Error 7313). ForceTeardownAfterWatchdog is deleted.
  • Shutdown-time discards are Errors with event ids: a delivery still deferred behind an init gate (7301) and turns still queued when the pump stops (7302), with message type/id/sender/gates/run level.

I/O pools (IoPool / IoPoolRegistry / IoPoolOptions)

  • Drain() gives a grace before it cancels: waits on the admission count with DrainGrace (8 s) restarted on every completion, so leaves that finish are never cancelled and a burst drains in as many completions; only what outlives a whole grace is named (CancelledLeafSites) and cancelled, then joined under the drain budget as before. New DrainAll(out residual, out cancelledAfterGrace); the old overloads stay.

Activities (ActivityTracker / ActivityRunner)

  • TrackRun(label, cancel) + ActivityRunHandle.Progress() (called from every ctx.Log line). Quiesce(stallBudget) waits for progressing runs, cancels a run with no progress for one budget (Error 7321), abandons one that ignores it a budget later (Error 7322). Track() is unchanged.

Orchestration

  • MeshTeardownExtensions.TeardownAsync uses Quiesce; TeardownReport gains CancelledActivities, AbandonedActivities, CancelledIoLeaves, CancelledIoByPool, WorkWasKilled (additive; Clean unchanged). The 30 s hang bound in TeardownAsync and MeshTeardownHostedService now carries the recursive disposal snapshot, at Error.
  • MonolithMeshTestBase runs the same phase 0, traces DISPOSE_WEDGED_WORK, and does not fail the class on killed work (maintainer: in tests, log).

Tests — the force-dependent tests are rewritten to the new contract: a cooperative wedged turn is cancelled at the stall budget and the phases finish; a non-cooperative one keeps disposal pending, is reported every budget, and completes when released with nothing forced; a backlog is busy, not wedged, and all 800 accepted turns run; the pool grace lets a finishing leaf finish and restarts per completion; the activity quiesce's four verdicts. NackReachesTheWaiterDuringTeardownTest and LateNackReenqueueTest park a turn that observes the owner's shutdown and assert the verdict inside 6 s with no stall verdict logged. DisposalRaceNackTest door one now asserts the accepted request is served before the hub goes down.

Docs — new Doc/Architecture/TeardownLayers (policy, verdict table, the Loki findings incl. the 28-minute preStop/live-circuit cause of the blocked rolls, which is a scheduling matter, not disposal); HubDisposalModel, MeshLifecycle, ControlledIoPooling, DebuggingDisposalAndLeaks updated; What's New entry (Fix).

Verification

Built every touched project with -c Release -warnaserror. Ran locally in Release: the teardown classes in MeshWeaver.Messaging.Hub.Test, MeshWeaver.Hosting.Test, MeshWeaver.Graph.Test; then the full projects Messaging.Hub.Test, Hosting.Test, Graph.Test, Data.Test, Layout.Test, Documentation.Test (link integrity) and Hosting.Orleans.Test under DOTNET_PROCESSOR_COUNT=4 — results in the PR conversation below. NackReachesTheWaiterDuringTeardownTest now completes in well under the 8 s stall budget and logs no stall verdict.

Pairs-with: none — no public type or member is removed; every surface change is additive. The two Plugins twins of the parked-turn tests (MeshWeaver.Hosting.Monolith.Test/{NackReachesTheWaiterDuringTeardownTest,LateNackReenqueueTest}.cs) are updated in a Plugins PR (behaviour change behind an unchanged signature, shape 7); the change there is backward compatible with the current pin. OwnerDisposalNackTest there needs no change — its NACK is minted when the owner's stream ends at the shutdown signal, never by the force.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HiucTsjSiMxEUxYM9ou688

… forced teardown

The 8 s disposal watchdog force-tore a hub down from its timer thread while
the wedged turn was still running and signalled Dead: a completion that was
not true. CI: NackReachesTheWaiterDuringTeardownTest's verdict could only be
minted AFTER the force, so its 10 s window was "watchdog + 2 s" and a loaded
shard lost it (run 33847949620). Prod (Loki, memex-cloud 08-29 → 09-03):
DISPOSAL DEADLOCK DETECTED … [FORCE-TEARDOWN] … 22706ms dozens of times per
shutdown, on sync/* hubs and their node hubs, once on a pod still serving.

Maintainer directive 2026-09-04: let activities, pooled I/O and handler turns
sit out their job; never cancel on entry; no forced teardown in prod or in
tests; detect a wedge as no progress for a budget, cancel once cooperatively,
and log every kill or discard at Error with what a reproduction needs so the
red-log triage files it (prod); in tests, log.

- MessageHub: Dispose() no longer cancels the in-flight turn (except a hub
  still Starting — its bring-up is not accepted work); the watchdog is a
  stall detector with five report-only verdicts (busy / wedged turn cancelled
  once / ignores cancellation / ShutDown phase blocked / stalled below), each
  Error with an EventId and the recursive snapshot; ForceTeardownAfterWatchdog
  deleted. HandleInitialize races the buildup against its token.
- MessageService: TurnsCompleted; shutdown-time discards logged at Error.
- IoPool.Drain: grace (admission-count join, restarted per completion) →
  name wedged leaves → cancel → join; DrainAll reports cancelled-after-grace.
- ActivityTracker: TrackRun + Progress + Quiesce(stall) (cancel, abandon);
  ActivityRunner reports progress per log line and logs a teardown kill.
- TeardownAsync/TeardownReport: CancelledActivities, AbandonedActivities,
  CancelledIoLeaves/ByPool, WorkWasKilled; hang bounds carry the snapshot.
- MonolithMeshTestBase: phase-0 quiesce; DISPOSE_WEDGED_WORK traced, not failed.
- Tests rewritten to the new contract; Doc/Architecture/TeardownLayers added;
  HubDisposalModel, MeshLifecycle, ControlledIoPooling, DebuggingDisposalAndLeaks
  updated; What's New (Fix).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HiucTsjSiMxEUxYM9ou688
Copilot AI lite review requested due to automatic review settings September 4, 2026 09:49
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge September 4, 2026 09:49
@rbuergi

rbuergi commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Local verification (Release, -warnaserror builds; full projects under DOTNET_PROCESSOR_COUNT=4)

project result
MeshWeaver.Messaging.Hub.Test (full) 367 passed, 0 failed, 2 m 7 s
MeshWeaver.Hosting.Test (full) 350 passed, 0 failed, 15 s
MeshWeaver.Graph.Test (full) 1324 passed, 0 failed, 1 m 27 s
MeshWeaver.Data.Test (full) 474 passed, 0 failed, 45 s
MeshWeaver.Layout.Test (full) 445 passed, 0 failed, 25 s
MeshWeaver.Documentation.Test (full, incl. link integrity and both ratchet guards) 257 passed, 0 failed, 11 s
MeshWeaver.Hosting.Orleans.Test (full, incl. PodHubTransportTest.CrossSiloNack…) 251 passed, 0 failed, 30 s
Plugins MeshWeaver.Hosting.Monolith.Test teardown twins, built against this branch 22 passed, 0 failed, 10 s

What the bulk runs found and what this branch fixed in response, in order:

  1. IoPoolTest grace test: the first-issued leaf is not the first admitted — the test now signals on whichever leaf runs first.
  2. DisposalStallWatchdogTest backlog case: the stall verdict had no baseline on its first look, so 512 completed turns read as a wedge — the detector now baselines the pump's turn count at Dispose() and treats a turn younger than the budget as movement.
  3. DisposalRaceNackTest door one: with the shutdown queued behind accepted work the request is served, not NACKed — the test asserts the new contract.
  4. DataContextInitTimeoutTest / DataContextShutdownGateAnswersDeferredTest: a sync hub whose InitializeHubRequest waited on a source that never emits held its owner for the whole 120 s startup bound — the bring-up now races its cancellation token (which Dispose() cancels for a hub that never reached Started; the one place that still cancels on entry, and it is not accepted work).

NackReachesTheWaiterDuringTeardownTest completes inside the 6 s bound with no stall verdict logged; PodHubTransportTest.CrossSiloNack… (the other flake in the census) passed in the full Orleans run.

Plugins half: Systemorph/MeshWeaver.Plugins — the two twin tests that parked a turn blind to the shutdown, updated in step (backward compatible with the current pin).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are concrete concurrency/teardown-correctness issues in the new ActivityTracker delta emission and IoPool drain terminality semantics that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR reworks Mesh teardown semantics to eliminate out-of-band “forced teardown,” letting already-accepted work drain while a stall detector reports (and cooperatively cancels) genuinely wedged turns, and extends the same “progress vs stall” idea to pooled I/O drain and activity teardown.

Changes:

  • Replaces hub disposal “force teardown after watchdog” with a stall detector (OnDisposalStall) that distinguishes busy vs wedged and logs event-id’d verdicts with diagnostics.
  • Updates IoPool.Drain() to add a completion-based grace period before cancellation, and enhances teardown reporting (per-pool residuals + cancellations after grace).
  • Adds activity quiescing with progress tracking/cancel/abandon verdicts, threads these outcomes into teardown reports, tests, and documentation.
File summaries
File Description
test/MeshWeaver.Messaging.Hub.Test/MessageHubTest.cs Updates disposal regression coverage to the new “drain backlog / cooperative cancel” contract.
test/MeshWeaver.Messaging.Hub.Test/HostedHubDisposalJoinTest.cs Aligns hosted-hub disposal join expectations with stall-based teardown (no force teardown).
test/MeshWeaver.Messaging.Hub.Test/DisposalStallWatchdogTest.cs Rewrites watchdog tests to validate the new stall-detector verdicts and behaviors.
test/MeshWeaver.Messaging.Hub.Test/DisposalRaceNackTest.cs Updates the race contract so accepted work is served (door one) and NACK behavior remains correct (door two).
test/MeshWeaver.Messaging.Hub.Test/DisposalDeadlockDiagnosticsTest.cs Pins the new diagnostics-rich stall verdict output.
test/MeshWeaver.Hosting.Test/IoPoolTest.cs Adds/updates tests for drain grace, restart-on-completion behavior, and cancellation reporting.
test/MeshWeaver.Hosting.Test/IoPoolResidualNamesItsPoolTest.cs Extends drain option coverage to include DrainGrace default/contract checks.
test/MeshWeaver.Hosting.Test/ActivityTrackerQuiesceTest.cs New tests validating activity quiesce verdicts (wait / cancel / abandon) and legacy surfaces.
test/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs Integrates activity quiesce + extended teardown reporting into the monolith test teardown trace/reporting.
test/MeshWeaver.Graph.Test/NackReachesTheWaiterDuringTeardownTest.cs Updates the parked-turn arrangement to observe shutdown (no reliance on forced teardown) and asserts no stall verdicts.
test/MeshWeaver.Graph.Test/LateNackReenqueueTest.cs Aligns late-verdict behavior with “accepted work drains” teardown and shutdown-observing parked turns.
src/MeshWeaver.Messaging.Hub/MessageService.cs Adds turns-completed counter and error-level discard logging with event ids for shutdown-time discards.
src/MeshWeaver.Messaging.Hub/MessageHub.cs Implements stall detector logic, removes forced teardown path, and adds event ids/verdict handling.
src/MeshWeaver.Messaging.Hub/HostedHubsCollection.cs Updates rationale/comments around joining children without out-running their terminal state (now stall-detected).
src/MeshWeaver.Mesh.Contract/Threading/MeshTeardownSignal.cs Extends TeardownReport with cancelled/abandoned activities and cancelled I/O leaves reporting.
src/MeshWeaver.Mesh.Contract/Threading/IoPoolRegistry.cs Passes DrainGrace into pools and extends DrainAll to return cancelled-after-grace details.
src/MeshWeaver.Mesh.Contract/Threading/IoPoolOptions.cs Adds configurable DrainGrace with production-default semantics.
src/MeshWeaver.Mesh.Contract/Threading/IoPool.cs Adds drain grace behavior and cancellation-after-grace reporting surfaces.
src/MeshWeaver.Mesh.Contract/MeshTeardownExtensions.cs Switches phase-0 from “wait for idle” to progress-based activity quiesce and logs event-id’d findings.
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs Updates commentary around recycle/shutdown probing to match the new stall-detector behavior.
src/MeshWeaver.Mesh.Contract/ActivityTracker.cs Adds run tracking with progress/cancel/abandon quiesce and new public handle/report types.
src/MeshWeaver.Hosting/MeshTeardownHostedService.cs Upgrades teardown-failure logging to Error and attaches disposal diagnostics.
src/MeshWeaver.GitSync/ActivityRunner.cs Registers activity runs with label/cancel and reports progress on each appended log line; distinguishes “cancelled by teardown.”
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-04-shutdown-lets-work-finish-and-names-what-it-had-to-stop.md What’s New entry describing the new shutdown/teardown contract.
src/MeshWeaver.Documentation/Data/Architecture/TeardownLayers.md New architecture doc formalizing “application drains first, infra after; no forced teardown,” with verdict table.
src/MeshWeaver.Documentation/Data/Architecture/MeshLifecycle.md Updates lifecycle teardown description to incorporate progress-based activity quiesce and I/O drain grace.
src/MeshWeaver.Documentation/Data/Architecture/HubDisposalModel.md Updates hub disposal model to describe the stall detector and non-forcing behavior.
src/MeshWeaver.Documentation/Data/Architecture/DebuggingDisposalAndLeaks.md Updates debugging guidance to the new “stall is reported, not forced” model.
src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md Updates I/O drain documentation to “grace then cancel/join” and how cancellations are reported.
src/MeshWeaver.Data/Workspace.cs Adjusts disposal/join commentary to match “accepted work drains; stall is reported” model.
src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs Updates disposal/join commentary to match the new teardown semantics.
src/MeshWeaver.Data/DataExtensions.cs Updates rationale around DI resolution during shutdown after removal of out-of-band force teardown.
Review details

Suppressed comments (2)

src/MeshWeaver.Mesh.Contract/ActivityTracker.cs:130

  • Complete calls deltas.OnNext(-1) directly from the caller thread, but Subject<T> is not thread-safe under concurrent OnNext calls. This can race with TrackRun and corrupt or fault the count stream. Emit the delta via the EventLoopScheduler (or otherwise serialize emissions) to keep counts consistent.
    internal void Complete(ActivityRunHandle handle)
    {
        if (runs.TryRemove(handle.Ticket, out _))
            deltas.OnNext(-1);
    }

src/MeshWeaver.Messaging.Hub/MessageHub.cs:1958

  • The inline comment says "three verdicts" but OnDisposalStall now produces five distinct outcomes; the comment should match the current contract to avoid misleading future debugging/triage.
        // Pending: the outer bounds — the test base's dispose deadline, the host's teardown
        // budget — report a hang with these diagnostics attached instead of a lie about
        // completion. See OnDisposalStall for the three verdicts.
  • Files reviewed: 32/32 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 739 to +742
if (!TryEnterGateRegion()) return 0;
try
{
// 🚨 GRACE FIRST — let the work finish its job. Nothing is cancelled until the pool has

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed and fixed: Drain() now sets _draining on its first line, and every entry point refuses a leaf issued after that (Cancelled<T>(), the same answer as after disposal). Already-admitted work keeps its grace; a producer issuing one leaf per completion can no longer keep the admission count moving. IoPoolTest (grace tests included) and the full MeshWeaver.Hosting.Test pass.

Comment on lines 118 to 122
public ActivityRunHandle TrackRun(string label, Action? cancel)
{
var handle = new ActivityRunHandle(this, Interlocked.Increment(ref ticketSeq), label, cancel);
runs[handle.Ticket] = handle;
deltas.OnNext(1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: TrackRun/Complete now go through Emit(delta), which schedules the OnNext onto the tracker's own EventLoopScheduler — the subject is only ever called from that one thread, as the surrounding comment claims. (Track() had the same shape before this PR; it is covered by the same change.) ActivityTrackerQuiesceTest passes.

Comment on lines +98 to 99
quiesce = await activities.Quiesce(ActivityStallBudget).ObserveCompletion(
ex => logger?.LogError(ex,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberate and pre-existing, not introduced here: TeardownAsync is the ONE sanctioned async edge — the mesh-teardown boundary — per Doc/Architecture/MeshLifecycle ("the only sanctioned await is that single three-phase drain at the boundary") and HubDisposalModel. It runs on the host's shutdown thread / xUnit's teardown thread, never on a hub turn, and every wait inside it is a subscription bridged through ObserveCompletion (RunContinuationsAsynchronously), which is what keeps hub threads from carrying it. The hub-side machinery this PR changes stays purely reactive.

Comment on lines +1789 to +1793
/// <summary>
/// The STALL budget of the disposal watchdog: how long the subtree may make no
/// <see cref="RunLevel"/> progress before the watchdog looks at what is holding the
/// teardown. It is not a duration cap and it forces nothing — see
/// <see cref="OnDisposalStall"/> for the three verdicts it can reach.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: both comments now say five verdicts, matching OnDisposalStall and Doc/Architecture/TeardownLayers.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

251 tests   251 ✅  2m 23s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

475 tests   475 ✅  44s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

368 tests   366 ✅  47s ⏱️
  3 suites    2 💤
  3 files      0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

2 275 tests   2 275 ✅  2m 36s ⏱️
    3 suites      0 💤
    3 files        0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

588 tests   396 ✅  2m 56s ⏱️
  3 suites  192 💤
  3 files      0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    4 files      4 suites   1m 33s ⏱️
1 478 tests 1 478 ✅ 0 💤 0 ❌
1 479 runs  1 479 ✅ 0 💤 0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   15 files     15 suites   11m 3s ⏱️
5 435 tests 5 241 ✅ 194 💤 0 ❌
5 436 runs  5 242 ✅ 194 💤 0 ❌

Results for commit 4628df9.

♻️ This comment has been updated with latest results.

rbuergi and others added 3 commits September 4, 2026 11:59
…erialised, the verdict count is five

- IoPool.Drain marks the pool draining before the grace, so a leaf issued once
  the drain began is refused (Cancelled) and a producer issuing one leaf per
  completion cannot keep the grace open; already-admitted work keeps its grace.
- ActivityTracker queues every delta onto its own event loop before the
  subject sees it, so OnNext is single-threaded as the comment claims.
- MessageHub doc comments say five verdicts, matching OnDisposalStall.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HiucTsjSiMxEUxYM9ou688
…med late watch is served through the sink first; quiesce idle is decided on the run registry

- MessageHub.Quiescing: when the budget expires with pending callbacks whose targets resolve
  (at the mesh root, the way HierarchicalRouting does) to a local hub that is shutting down but
  has not signalled disposal, re-arm the wait instead of advancing — the sibling answers
  (NACK ShuttingDown / DISPOSE-DISCARD) when its own pump stops, phases later. Bounded by
  MaxQuiesceRearms (20); the cut-off is an Error (7315 DisposalQuiesceWaitCutOff) with the
  owed replies named. The stall detector reads Quiescing-with-owed-replies as busy.
- DataExtensions.RoutePatchVerdict: when a late watch is armed for the request id, dispatch
  through the sink FIRST — the merge ack the owner accepted was otherwise dropped by the
  parent's routing at DisposeHostedHubs (CI shard 4 on 46e6f79, NackReachesTheWaiter…).
- ActivityTracker.Quiesce: idle is decided on the run registry being empty, not on a replayed
  count that a scheduled Emit can leave stale for one turn.
- Docs: TeardownLayers (Quiescing owed-reply section), HubDisposalModel.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HiucTsjSiMxEUxYM9ou688
… — not on every patch

The previous commit dispatched an armed late watch FIRST, unconditionally. A late watch is
armed for EVERY patch at post time, not just an expired one, so that moved every ordinary
ack off its designed transport and forward of the state it acknowledges (the sink dispatches
synchronously on the owner's turn; the posted response arrives after the change). Measured:
ComboGateRollTest went red under load on run 33863349195 — the write completed before what it
wrote was readable — while it passes 7/7 and its project 727/727 locally.

Now: post first, as before. Only when the owner is shutting down is the armed sink served as
well, because there the accepted post is not proof of delivery — the parent's routing drops it
a turn later, where this seam cannot see it. The registry removes the entry before calling
back and a surviving posted response reaches the same registry through the cache hub, so the
caller is answered exactly once whichever arrives first.

PatchVerdictRoutingTest gains both arms: shutting-down also serves the sink, live leaves it
alone.

Verified in Release: PatchVerdictRoutingTest 7/7, the two teardown Nack tests 2/2,
Memex.Portal.Shared.Test 727/727, MeshWeaver.Data.Test 476/476, MeshWeaver.Graph.Test 1324/1324.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Verification of the last push (62be916): the previous commit's late-verdict change was too broad and it reddened shard 5 here — ComboGateRollTest.ANotVerifiableVerdict… on run 33863349195. Root cause, measured: a late watch is armed for EVERY patch at post time, not only an expired one, so dispatching the armed sink FIRST moved every ordinary ack off its designed transport — and forward of the state it acknowledges, because the sink dispatches synchronously on the owner's turn while the posted response arrives after the change notification. The caller's write completed before what it wrote was readable.

Now it posts first, exactly as before, and serves the armed sink as well ONLY while the owner is shutting down — where the accepted post is not proof of delivery, because the parent's routing drops it a turn later where this seam cannot see it. The registry removes the entry before it calls back and a surviving posted response reaches the same registry through the cache hub, so the caller is answered exactly once whichever arrives first. PatchVerdictRoutingTest pins both arms.

Release runs on this head: PatchVerdictRoutingTest 7/7 · NackReachesTheWaiterDuringTeardownTest + LateNackReenqueueTest 2/2 · Memex.Portal.Shared.Test 727/727 · MeshWeaver.Data.Test 476/476 · MeshWeaver.Graph.Test 1324/1324 (all -c Release, DOTNET_PROCESSOR_COUNT=4).

… the ack ahead of the state

Twice now, closing the teardown hole at this seam has reddened a LIVE test, and the mechanism is
the same both times: a late watch is armed for EVERY patch at post time, not just an expired one,
so serving it dispatches the caller's ack synchronously on the OWNER's turn — ahead of the state
change the ack is about. The caller's write then completes before what it wrote is readable.

- dispatch-first (46e6f79 → ef42bed): ComboGateRollTest, run 33863349195.
- gated on hub.IsShuttingDown (62be916): ImportTypeBeforeInstanceTest, run 33865385033. That
  test recycles node hubs throughout its import, so a recycling owner meets the gate on the LIVE
  path — the gate did not mean "the mesh is going down" at all.

So RoutePatchVerdict goes back to post-first, sink only on refusal, and PatchVerdictRoutingTest
keeps a guard that an accepted post never also serves an armed sink, with both measurements named
so this is not attempted a third time.

The teardown hole is real and belongs one layer down: a correlated reply HierarchicalRouting
cannot forward is dropped with nobody told, while the process still holds the registry that could
take it. That is a separate change set; the owner's ShutDown-phase NACK
(RegisterOwnerDisposingNack) is the designed answer meanwhile.

Verified in Release at DOTNET_PROCESSOR_COUNT=2 (CI's shape): PatchVerdictRoutingTest 6/6,
MeshWeaver.Graph.Test 1324/1324 — including ImportTypeBeforeInstanceTest and the two teardown
Nack tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Second red on this branch, and it was mine again — same mechanism, so I have taken the change out rather than gate it a third way.

ImportTypeBeforeInstanceTest.InstanceEnumeratedBeforeItsType_StillLandsInOnePass timed out on shard 4 (run 33865385033): the import itself succeeded (outcome=Imported count=14 failed=0) and then GetMeshNodeStream(path) never emitted for 60 s. That is the ComboGateRollTest shape again — a write acked before what it wrote is readable.

Root cause of both: a late watch is armed for EVERY patch at post time, not only an expired one. Serving it alongside the post dispatches the ack synchronously on the OWNER's turn, ahead of the state change the ack is about. My IsShuttingDown gate did not mean "the mesh is going down": an import recycles node hubs continuously, and a recycling owner satisfies it on the live path.

RoutePatchVerdict is back to post-first, sink only on refusal — its behaviour before this branch. PatchVerdictRoutingTest keeps a guard that an accepted post never also serves an armed sink, with both run ids named in the test so this is not tried a third time.

The teardown hole that prompted it is real and stays open here deliberately: a correlated reply that HierarchicalRouting cannot forward is dropped with nobody told, while the process still holds the registry that could take it. That is a change set one layer down (a sink for undeliverable replies in the routing layer), not something this seam can answer — it runs before routing has. Meanwhile the owner's ShutDown-phase NACK is the designed answer, which is what NackReachesTheWaiterDuringTeardownTest rides.

Verified at DOTNET_PROCESSOR_COUNT=2, CI's shape: PatchVerdictRoutingTest 6/6 · MeshWeaver.Graph.Test 1324/1324 in one process (ImportTypeBeforeInstanceTest and both teardown Nack tests included).

rbuergi added a commit that referenced this pull request Sep 11, 2026
…is branch

Evidence: scaffold test/MeshWeaver.Graph.Test/_TempDiagTest.cs + DIAG-4023 fate stages on this branch.
Runs (Release, DOTNET_PROCESSOR_COUNT=2): run1 200/200 pass (no stress); run2 1/400 FAIL (18-proc CPU
stress); run4 (v2 journey) 1/400 FAIL; run5 (v3) 0/400.

1. #2778's fix (#2868, direct ILatePatchVerdictSink.Dispatch in RegisterOwnerDisposingNack) NEVER failed:
   every iteration where the ShutDown registrant claimed the ack gate shows dispatched=True (41/200, 42/400).
2. The TEST moved off that path. Core #3291 (2026-09-04, no forced teardown) made the parked merge turn
   release on owner.IsShuttingDown, which flips at the FIRST instant of Mesh.Dispose() (CloseCreation
   cascade) while the owner is still Started. The merge then COMMITS and the live PATCH_ACK ok claims the
   once-only gate; the registrant stands down (claimed=False). Routes that consumed the watch (run2):
   347 live ack via 97a0284 route-up sink; 42 registrant dispatch; 10 live ack via #3196 refused-post
   fallthrough; 1 FAIL = live ack on the ORDINARY route (owner->mesh->cache queues, parent still
   < DisposeHostedHubs).
3. a9249a4 (2026-09-08) asserts at owner RunLevel==Dead on the premise "DisposeImpl runs the registrant
   before Dead". True for the registrant and for the sink (synchronous on the owner's turns); FALSE for
   the ordinary route: the ack has left the owner and sits in mesh/cache queues. "Owner Dead" no longer
   implies "answer delivered".
4. Two failure shapes (MonolithMeshTestBase DisposeAsync 'Mesh.Disposal completed in' = time REMAINING
   after the assertion): B run4 i=220 = 12ms (reply queued at cache depth 3, delivered right after the
   assertion instant: test-premise race). A run2 i=21 = 1849ms, CI 34513634943 = 1949ms,
   CI 34581527040 = 1948ms (passing max 69-107ms): answer held for the cache hub's whole 2s
   QuiesceTimeout. Shape A mechanism NOT YET NAMED; v3 snapshots the hub tree at Dead to name it.
5. Falsified: cache queue depth (i=399 passed at depth 3, mesh Started, handled +38ms); DataChangedEvent
   push holding the cache pump (handled on the sync sub-hub, synchronous); thread-pool starvation
   (all monolith hubs use TaskScheduler.Default, owner went Started->Dead in ~80ms same iteration);
   ShuttingDown subscribers posting to cache (only ActivityControlPlane, which just Stop()s).
6. Registrant-ran elimination HOLDS (DIAG_NACK_REGISTRANT runLevel=ShutDown in every failing trail) but
   proves nothing about a NACK: the gate was claimed by a SUCCESS ack; no OwnerDisposing NACK is minted.
7. The assertion's "burns the whole 31s" is wrong for this scenario: the cache's quiesce force-cancels
   at ~2s (CancelCallbacks -> caller onError -> lateRegistry.Complete) — a committed write reported as
   HubDisposedBeforeResponse.
@rbuergi
rbuergi deleted the fix/robust-disposal branch October 10, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants