Skip to content

fix(hub): a bring-up discard reports at the level of who it strands; a sync sub-hub construction fault is not a shutdown - #5609

Merged
meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/teardown-discard-level-follows-the-protocol
Sep 23, 2026
Merged

meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/teardown-discard-level-follows-the-protocol

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Refs #4888, Refs #5424, Refs #5589, Refs #5592

Root cause

Two teardown classifications claimed facts that were false. Each false claim became an Error, and the red-log pipeline filed each Error as an issue.

1. [DISPOSE-DISCARD] (event 7301) called a re-asked subscribe "stranded work"

A delivery can be parked only behind an initialization gate. Those gates close for nothing but a bring-up. So every line at this site is a teardown that ended a bring-up, which is the TeardownLayers → Handler turns carve-out. The parked delivery is answered with a transient ShutdownNack.RetryForTheAuthoritativeAnswer (since #4866). For a SubscribeRequest every producer rides that answer out and asks the next activation:

Nobody is stranded. The line still said "Accepted work must be drained before a hub goes down" at Error. The three open incidents at this site (#4888, #5424, #5589) are all SubscribeRequests from cache/… senders, discarded when Orleans deactivated grains (DirectoryFailure) mid-bring-up.

Fix: a new [ReaskedOnShutdown] marker (MeshWeaver.Messaging.Contract), placed on SubscribeRequest. A marked request's discard reports at Debug with the same facts: gate, sender and teardown attribution. This follows the #4178 rule for self-addressed deliveries. Any unmarked request from another sender stays Error, because that waiter may take the NACK as final. The NACK itself is unchanged.

2. A sync sub-hub construction FAULT was reported as "shutting down" (#5592)

SynchronizationStream's constructor turned every null sub-hub into HubDisposingException("Hub X is shutting down … retry"). That included HostedHubOutcome.ConstructionFaulted on a live host: a configuration that threw, or an OOM under heap exhaustion. #5592's line comes from the arm of HandleInitialize that runs only when IsShuttingDown is false. That flag reads the same monotonic flags the creation freeze reads, so ConstructionFaulted is the only null that can produce it. The issue proposed absorbing the exception during init. That would have hidden a real fault behind a teardown label.

Fix: the constructor reads the outcome through TryGetHostedHub. On ConstructionFaulted it throws InvalidOperationException, which names the root fault and carries it as the inner exception. It is deliberately not an ObjectDisposedException, so no teardown classifier reads it as "retry". Every other null is still the transient HubDisposingException.

Evidence

  • DeferredDeliveryNackedOnDisposeTest.AReaskedRequestDiscardedByABringUpTeardown_IsTeardownNormal_NotAnError: the production initiator (NoteDirectDisposalBy("Orleans deactivating grain …") + Dispose()). It checks that the NACK is still transient and owner-recognised, that the Debug line names the gate and the attribution, and that no Error is logged. The existing DeferredRequest_IsNacked_… case (unmarked, still Error) is the control.
  • SyncHubConstructionFaultIsNotAShutdownTest: a real fault inside the sub-hub's construction (the host serves AccessService from a factory that throws while armed). The host is not shutting down, and the refusal is not an ObjectDisposedException and carries the injected fault. The second case is the control: the creation freeze still refuses with HubDisposingException.
  • Negative controls (each fix line disabled with false &&): both new tests fail. Case 1 fails because the Debug line is missing; case 2 because "Expected value to be false … but found True". With the fix restored, both pass.
  • Full suites: Messaging.Hub.Test 505/505, Data.Test 545/545.
  • dotnet build -c Release -warnaserror: 0 Warning(s) / 0 Error(s) for Messaging.Contract, Messaging.Hub, Data.Contract, Data, Documentation, Messaging.Hub.Test and Data.Test.

Doc: RetiringAnActivation → What is NOT a retirement now carries the level table and settles the "OPEN level" paragraph. TeardownLayers → Handler turns and HubDisposalModel (the sync-stream refusal) are updated to match.

Not established

Deploy / recycle

Framework code. It takes effect with the image roll, and no address needs a manual recycle.

Pairs-with: none — nothing public is removed (one public attribute type is added).

🤖 Generated with Claude Code

…a sync sub-hub construction fault is not a shutdown

Two classifications in the hub teardown path claimed facts that were false.

1. [DISPOSE-DISCARD] (event 7301) logged Error for every foreign sender's
   delivery parked behind the init gates of a hub torn down before bring-up
   finished. The sender is answered with a transient ShutdownNack, and for a
   SubscribeRequest every producer rides that answer out and re-asks the next
   activation. Nobody is stranded, yet each Orleans DirectoryFailure
   deactivation filed an issue (#4888, #5424, #5589). New
   [ReaskedOnShutdown] marker on SubscribeRequest; marked requests report at
   Debug with the same facts. Unmarked requests stay Error (the control).

2. SynchronizationStream refused with HubDisposingException ("is shutting
   down, retry") for EVERY null sub-hub, including HostedHubOutcome.
   ConstructionFaulted on a live host. That is how #5592 read "shutting down"
   on a hub whose IsShuttingDown was false. It now reads the outcome and
   throws an InvalidOperationException naming the root fault.

Tests: DeferredDeliveryNackedOnDisposeTest (new case),
SyncHubConstructionFaultIsNotAShutdownTest. Negative controls (fix lines
disabled) fail both; Messaging.Hub.Test 505/505, Data.Test 545/545.

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

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.

Copilot review overview

🔵 Needs a closer look

The marker does not cover the Python producer or cross-hub RawJson deliveries.

Review effort: Lite
Findings: None

What changed in this PR

Corrects teardown log severity for re-asked subscriptions and distinguishes sync sub-hub construction faults from shutdown races.

Changes:

  • Adds [ReaskedOnShutdown] handling for SubscribeRequest.
  • Reports marked deferred discards at Debug while preserving transient NACKs.
  • Reports construction faults as InvalidOperationException.
  • Adds regression tests and updates architecture documentation.
File Summary
test/​MeshWeaver.Messaging.Hub.Test/​DeferredDeliveryNackedOnDisposeTest.cs Tests marked and unmarked deferred-discard severity.
test/​MeshWeaver.Data.Test/​SyncHubConstructionFaultIsNotAShutdownTest.cs Tests construction-fault and shutdown behavior.
src/​MeshWeaver.Messaging.Hub/​MessageService.cs Applies deferred-discard classification.
src/​MeshWeaver.Messaging.Contract/​ReaskedOnShutdownAttribute.cs Defines the request marker.
src/​MeshWeaver.Documentation/​Data/​Architecture/​TeardownLayers.md Documents teardown discard levels.
src/​MeshWeaver.Documentation/​Data/​Architecture/​RetiringAnActivation.md Adds stranded-work classification guidance.
src/​MeshWeaver.Documentation/​Data/​Architecture/​HubDisposalModel.md Documents sync-stream refusal behavior.
src/​MeshWeaver.Data/​Serialization/​SynchronizationStream.cs Distinguishes construction faults from shutdown.
src/​MeshWeaver.Data.Contract/​Messages.cs Marks SubscribeRequest as re-asked.

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

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files    1 suites   2m 58s ⏱️
328 tests 328 ✅ 0 💤 0 ❌
332 runs  332 ✅ 0 💤 0 ❌

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

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

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

545 tests   545 ✅  1m 6s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    2 files      2 suites   6m 23s ⏱️
2 593 tests 2 593 ✅ 0 💤 0 ❌
2 594 runs  2 594 ✅ 0 💤 0 ❌

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 600 tests   1 600 ✅  8m 51s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    5 files      5 suites   14m 34s ⏱️
3 401 tests 3 401 ✅ 0 💤 0 ❌
3 405 runs  3 405 ✅ 0 💤 0 ❌

Results for commit 1fea86b.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   13 files     13 suites   34m 55s ⏱️
8 896 tests 8 896 ✅ 0 💤 0 ❌
8 905 runs  8 905 ✅ 0 💤 0 ❌

Results for commit 1fea86b.

@meshweaver-cloud
meshweaver-cloud Bot added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit fe33e13 Sep 23, 2026
40 checks passed
@rbuergi
rbuergi deleted the fix/teardown-discard-level-follows-the-protocol 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