Skip to content

fix(data): a sync stream that gives up on its owner ENDS; one teardown counts once - #6606

Merged
rbuergi merged 2 commits into
mainfrom
fix/sync-stream-giveup-surfaces
Oct 12, 2026
Merged

rbuergi merged 2 commits into
mainfrom
fix/sync-stream-giveup-surfaces

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

What

  1. A give-up is a terminal. When JsonSynchronizationStream's recycle re-arm gives up on an owner (giving up on owner … DISTINCT owner activations have refused this stream, or the same-activation ride-out variant), the stream now ENDS the way an owner verdict does. It calls OnError with a new typed SyncStreamOwnerRefusedException (it names the stream and the owner) and stops the keep-alive. Before this, it disarmed the re-ask and left the stream alive on its last value. Readers held a frozen snapshot that looked live, and no recovery ran.
  2. One teardown counts once. The activation budget is meant to count DISTINCT refusing activations. Two carriers inflated the count:
    • StreamEndedEvent carried no activation identity, so every announcement was charged as a new activation, and it broke the same-activation chain the NACKs build. It now carries Activation (ShutdownNack.ActivationToken(hub)), stamped by all three producers: Workspace recycle announce, the owner-side end of a served stream, and HandleHeartBeat's answer for an unserved stream.
    • MessageHubGrain.RefuseBecauseTheHostIsLeaving tagged its NACK with the grain, while the hub's NACKs tag the hub. One activation on a leaving silo therefore had two tags. It now tags _hub when there is one.
    • The budget now counts the set of activations seen since the last answered re-ask, not only "the previous one".
  3. Consumers recover. MeshNodeStreamCache.IsTransientOwnerFailure and AreaErrorClassifier.IsTransientHubFailure classify the new type as transient. The cache marks the entry faulted and evicts it, so the next read opens a fresh stream. A layout area re-subscribes, bounded by RetryAreaWithBackoff. The fleet watch's held read re-opens on the node's next write in Systemorph/MeshWeaver.Plugins#3503.

Why

#5011 (newest comment): on memex-cloud on 2026-10-12 the give-up lines hit Hosting/PlatformBuilds, Hosting/Modules, Ops/Status/memex and others, in bursts during rolls. The fleet watch held a stale Ops/Status/build while the store had fresh rows. Filed as #6602.

No timeout, budget, retry or pacing changed. The stream itself still stops asking. It now tells its readers that it stopped.

Tests

ASyncStreamThatGivesUpOnItsOwnerEndsTest (MeshWeaver.Data.Test) scripts the owner at its delivery pipeline:

  • RefusedByMoreActivationsThanTheBudget_TheReaderSeesTheStreamEnd: every re-ask is refused by a new activation, and the reader must see SyncStreamOwnerRefusedException. Negative control (run by hand): with the terminal removed, the stream never ends (red at 36 s).
  • OneActivationsNackAndAnnouncement_AreOneActivation_AndTheStreamRecovers: one activation refuses 4 re-asks and announces each end, and the stream must ride it out and be accepted. Negative control (run by hand): with the announcement's Activation ignored, it gives up after 4 DISTINCT owner activations.

Local results:

  • MeshWeaver.Data.Test: 571/571.
  • MeshWeaver.Layout.Test, the classifier/recycle/stream-end classes: 120/120.
  • MeshWeaver.Hosting.Test (ReadBudgetTest, SubscribeDuringRecycleTest, ChangeFeedResetReleasesUpstreamTest, MeshQueryTransientRetryTest): 10/10.
  • MeshWeaver.Graph.Test (RecyclingShapeDiagnosticTest, ExtractActivationTagTest, RecycledHubRefusalTest, OwnerAnswerRecognitionGuard, RouterTrafficOnNodeCreateFromTheRootHubTest): 27/27.
  • MeshWeaver.Messaging.Hub.Test: 576/576 (full project).
  • Every touched project builds clean with -c Release -warnaserror.

Doc: Doc/Architecture/RidingOutAShuttingDownAddress, new section "A give-up ends the stream (#6602)".

Public surface: additions only (SyncStreamOwnerRefusedException, StreamEndedEvent.Activation as an init property, ShutdownNack.ActivationToken). Nothing is removed and no interface member is added. An owner on an older image sends no Activation, and the subscriber then falls back to the previous accounting.

Recycle after deploy: none needed. This is compiled platform code, so the roll replaces it.

Refs #6602
Refs #5011

🤖 Generated with Claude Code

…n counts once

The recycle re-arm's give-up disarmed the re-ask and left the stream alive on
its last value, so every reader held a frozen snapshot that looked live, and no
recovery ran. On memex-cloud during rolls this hit Hosting/PlatformBuilds,
Hosting/Modules and Ops/Status/* (candidate cause of #5011's reader-side
shape). The give-up now ends the stream the way an owner verdict does: OnError
with the typed SyncStreamOwnerRefusedException, and the keep-alive stops. The
cache evicts the faulted entry; the cache and layout-area classifiers treat the
type as transient, so the next read and the area retry open a fresh stream.

Why rolls spent the budget: StreamEndedEvent carried no activation identity, so
every announcement was charged as a new activation and broke the NACK chain.
And MessageHubGrain tagged its leaving-host NACK with the grain while the hub's
NACKs tag the hub, so one activation had two tags. StreamEndedEvent now carries
Activation (ShutdownNack.ActivationToken), stamped by all three producers. The
grain tags its hub. The budget counts the SET of activations seen since the
last answer.

Pinned by ASyncStreamThatGivesUpOnItsOwnerEndsTest. Both negative controls
were run by hand.

Refs #6602, #5011

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.

🟡 Changes recommended

Interleaved activations are miscounted by a shared counter, and each give-up emits duplicate warnings.

1 open finding
What changed in this PR

Makes sync-stream owner give-up terminal and improves activation accounting during teardown.

Changes:

  • Adds typed terminal errors and consumer recovery classification.
  • Propagates owner activation tokens across stream-end events.
  • Adds regression tests and architecture documentation.
File Description
test/​MeshWeaver.Data.Test/​ASyncStreamThatGivesUpOnItsOwnerEndsTest.cs Tests terminal give-up and activation accounting.
src/​MeshWeaver.Messaging.Contract/​ShutdownNack.cs Exposes activation-token formatting.
src/​MeshWeaver.Mesh.Contract/​MeshExtensions.cs Tags heartbeat stream-end responses.
src/​MeshWeaver.Layout/​AreaErrorClassifier.cs Classifies owner refusal as transient.
src/​MeshWeaver.Hosting/​MeshNodeStreamCache.cs Classifies cache owner refusal as transient.
src/​MeshWeaver.Hosting.Orleans/​MessageHubGrain.cs Aligns grain and hub activation identities.
src/​MeshWeaver.Documentation/​Data/​Architecture/​RidingOutAShuttingDownAddress.md Documents terminal give-up behavior.
src/​MeshWeaver.Data/​Workspace.cs Tags recycle announcements.
src/​MeshWeaver.Data/​Serialization/​JsonSynchronizationStream.cs Implements terminal errors and activation tracking.
src/​MeshWeaver.Data.Contract/​Messages.cs Adds activation identity to stream-end events.

🧠 Review effort: Balanced


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

Comment thread src/MeshWeaver.Data/Serialization/JsonSynchronizationStream.cs Outdated
)

An A, B, A, B interleaving pooled both teardowns' re-asks onto one counter,
so the give-up named whichever activation refused last as having refused
re-asks it never saw. Each refusing activation now keeps its own count;
the regression test alternates two activations 10 times each.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 17:36
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 12, 2026 17:36

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

Activation accounting remains race-prone, and queued re-arms can continue after the stream terminates.

1 open finding
1 resolved since last review
Previously missed (4)

In code that hasn't changed since last review

Medium severity Avoid duplicate warnings for a single give-up

src/​MeshWeaver.Data/​Serialization/​JsonSynchronizationStream.cs:1086

This emits two warnings for one give-up: the explicit LogWarning here, then reduced.OnError enters SynchronizationStream.OnError's non-timeout branch and logs the same exception at Warning again. During the roll bursts this doubles the production log volume and contradicts the stated one-warning behavior; let the terminal's existing OnError logging be the single warning.

Medium severity Dispose recycle-rearm subscription when the stream terminates

src/​MeshWeaver.Data/​Serialization/​JsonSynchronizationStream.cs:1087

keepAlive.Dispose() does not dispose recycleReArm, which is registered separately on reduced. Because ChargeReArmBudget runs eagerly before Concat, earlier decisions may already be queued when a later rejection calls GiveUp; those queued legs can still call Resubscribe and post more SubscribeRequests after the stream has terminally faulted. Make the recycle-rearm subscription owned by keepAlive (or otherwise cancel it here) so “the stream stops asking” is actually enforced.

Low severity Add cache-level regression for refused stream recovery

src/​MeshWeaver.Hosting/​MeshNodeStreamCache.cs:2007

The new integration test subscribes with GetRemoteStream, so it never exercises the cache path whose recovery this branch is meant to enable. Add a cache-level regression that delivers SyncStreamOwnerRefusedException, verifies the faulted entry is not negative-cached, and proves the next read opens a fresh upstream rather than replaying the terminal.

Low severity Test layout retry and recovery for refused sync streams

src/​MeshWeaver.Layout/​AreaErrorClassifier.cs:41

No layout test covers this newly added exception type. Existing classifier/retry tests exercise each retry class, but the PR's end-to-end test bypasses the layout path; add a regression proving ShouldRetryArea accepts SyncStreamOwnerRefusedException and that a deferred area source re-subscribes and recovers.

🧠 Review effort: Balanced

Comment on lines 1029 to +1042
@@ -990,35 +1038,55 @@ void Resubscribe(string reason)
// observation — both, and labelled.
if (!sameActivation)
{
Interlocked.Exchange(ref sameActivationReAsks, 0);
if (activation is not null)
ImmutableInterlocked.TryAdd(ref refusingActivations, activation, 0);
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    20 files   -   4      20 suites   - 4   56m 2s ⏱️ +5s
11 893 tests  - 304  11 893 ✅  - 111  0 💤  - 193  0 ❌ ±0 
11 907 runs   - 304  11 907 ✅  - 111  0 💤  - 193  0 ❌ ±0 

Results for commit 7efb778. ± Comparison against base commit 514ee6f.

This pull request removes 347 and adds 19 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
 ---> (Inner Exception #1) System.Net.Sockets.SocketException (0xFFFDFFFF): Name or service not known<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "5dfc9fad-1e54-4fc8-9856-c781b85d5c97")
Memex.Portal.Shared.Test.SessionDenialIsAnAnswerTest ‑ OnlyAVerdictReadsAsADenial(shape: "the same verdict nested, as a late denial dispatch"···, failure: System.InvalidOperationException: write failed
 ---> System.UnauthorizedAccessException: Access denied
   --- End of inner exception stack trace ---, isDenial: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's FIRST member", fault: System.AggregateException: One or more errors occurred. (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.
   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall as an aggregate's SECOND member", fault: System.AggregateException: One or more errors occurred. (The operation has timed out.) (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> System.TimeoutException: The operation has timed out.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall nested in an aggregate inside a wrapper", fault: System.InvalidOperationException: outer
 ---> System.AggregateException: One or more errors occurred. (Value does not fall within the expected range.) (Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.)
 ---> System.ArgumentException: Value does not fall within the expected range.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---

   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "a stall wrapped as InnerException", fault: System.InvalidOperationException: outer
 ---> MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.
   --- End of inner exception stack trace ---, expected: True)
MeshWeaver.Compiler.Pipeline.Test.SourcesWatcherStallBackoffTest ‑ The_production_classifier_recognises_a_stall_through_every_wrapping(because: "an aggregate of unrelated faults", fault: System.AggregateException: One or more errors occurred. (The operation has timed out.) (Value does not fall within the expected range.)
 ---> System.TimeoutException: The operation has timed out.
   --- End of inner exception stack trace ---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
, expected: False)
MeshWeaver.Data.Test.ASyncStreamThatGivesUpOnItsOwnerEndsTest ‑ OneActivationsNackAndAnnouncement_AreOneActivation_AndTheStreamRecovers
MeshWeaver.Data.Test.ASyncStreamThatGivesUpOnItsOwnerEndsTest ‑ RefusedByMoreActivationsThanTheBudget_TheReaderSeesTheStreamEnd
MeshWeaver.Data.Test.ASyncStreamThatGivesUpOnItsOwnerEndsTest ‑ TwoInterleavedActivations_EachChargeTheirOwnRideOut_AndTheStreamRecovers
…

♻️ This comment has been updated with latest results.

@rbuergi
rbuergi merged commit 5d675f7 into main Oct 12, 2026
@rbuergi
rbuergi deleted the fix/sync-stream-giveup-surfaces branch October 24, 2026 14:14
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