fix(#2633): retry a transient stream-attach instead of latching a hub into "cross-process routing DISABLED" by rbuergi · Pull Request #2645 · Systemorph/MeshWeaver

rbuergi · 2026-08-29T06:35:51Z

Closes nothing by itself — linking, not closing: #2633.

The defect

OrleansRoutingService.SubscribeWhenStreamingReadyAsync attaches a hub's Orleans memory-stream subscription once the #1129 readiness gate opens. Everything past that gate is a cluster call: SubscribeAsync resolves the stream's PubSubRendezvousGrain through Orleans' grain directory, which is precisely the component that is unstable while cluster membership changes.

The attach was a single attempt inside a catch-all, so one such rejection latched the hub into

crit: Orleans 'Memory' stream subscription could not be attached for portal/… —
      cross-process routing for this hub is DISABLED (in-process routing remains active)

…for the rest of that hub's life. Nothing re-attempted; the loss persisted until the hub re-registered (a circuit reconnect, or a pod restart). Six per-user losses across three ReplicaSet generations on memex-cloud, every one of them an OrleansMessageRejectionException whose inner text is Orleans' own "Unable to connect to S… , will retry after 582.6889ms".

The same exception class is already classified transient and retried on the delivery leg (RoutingGrain.DeliverToGrainWithRetry → IsTransientFailure). The subscribe leg had no equivalent. That inconsistency is the defect.

The fix

The attach runs through a new AttachWithBoundedRetryAsync:

Bounded — 5 re-attempts, backoff 250 ms doubling to a 4 s cap (≈7.75 s total). Comfortably longer than the sub-second reconnect Orleans promises; far short of the 120 s readiness gate that precedes it and the 60 s request budgets above it.
Loud — every retry logs a Warning naming the attempt number and the wait. The evidence that a hub was momentarily unattached is not lost, it is just no longer a Critical for a condition that was over in half a second.
Still terminal — anything IsTransientFailure does not recognise gives up on the first attempt, and an exhausted budget still ends in the same LogCritical + null + latched disable. A permanent failure still fails.
Each retry re-invokes the attach from scratch (fresh GetStream, fresh SubscribeAsync), so Orleans re-resolves the rendezvous grain rather than re-using a reference minted mid-handoff.

No change to the readiness gate itself, to the outbound subscriptionReady gate, or to the delivery handlers.

Tests — `StreamAttachTransientRetryTest`

Deterministic, no cluster: reaching the stream provider is the observable, and the backoff is collapsed to zero through an instance seam (AttachBackoff, never static state).

test	asserts
`TransientAttachFailure_IsRetried_UpToTheBudget`	6 attempts (1 + `SubscribeAttachRetries`)
`NonTransientAttachFailure_StillGivesUpOnTheFirstAttempt`	exactly 1 attempt — a permanent failure must still fail
`TheFakeIsTheProductionException`	the driver is `OrleansMessageRejectionException` and is what `IsTransientFailure` matches — without this the retry tests could pass against nothing
`AttachBackoff_IsBoundedAndCapped`	the budget is small, growing, capped, and finishes well inside the enclosing budgets

Failing on unfixed code

Run with the retry budget forced to the pre-fix single attempt (maxRetries: 0):

[FAIL] StreamAttachTransientRetryTest.TransientAttachFailure_IsRetried_UpToTheBudget
  Expected value to be 6 because a grain-directory rejection is TRANSIENT … but found 1.
Failed!  - Failed: 1, Passed: 3, Total: 4

With the fix in place:

Passed!  - Failed: 0, Passed: 13, Total: 13

(13 = this file plus OrleansStreamingReadinessGateTest and OrleansDirectoryInstabilityClassificationTest, run together to confirm the #1129 gate's "attaches exactly once" assertion still holds — it does, because its probe throws a non-transient NotSupportedException.)

🤖 Generated with Claude Code

… into "routing DISABLED" `OrleansRoutingService.SubscribeWhenStreamingReadyAsync` attaches a hub's Orleans memory-stream subscription once the #1129 readiness gate opens. Everything past that gate is a CLUSTER call — `SubscribeAsync` resolves the stream's `PubSubRendezvousGrain` through Orleans' grain directory, which is exactly what is unstable while cluster membership changes. The attach was a SINGLE attempt inside a catch-all, so one such rejection latched the hub into "cross-process routing for this hub is DISABLED" for the rest of its life: nothing re-attempted, and the loss persisted until the hub re-registered (a circuit reconnect or a pod restart). Six per-user losses across three ReplicaSet generations on memex-cloud, every one of them an `OrleansMessageRejectionException` that the DELIVERY leg already classifies transient and retries (`RoutingGrain.DeliverToGrainWithRetry`). The inconsistency between the two legs was the whole defect. The attach now runs through `AttachWithBoundedRetryAsync`: bounded (5 re-attempts, 250 ms doubling to a 4 s cap — ≈7.75 s total, against the ~0.5–0.9 s reconnect Orleans' own ConnectionManager promises in the message it fails with), loud (a Warning naming the attempt on every retry), and still terminal — anything `IsTransientFailure` does not recognise gives up on the first attempt, and an exhausted budget still ends in the same LogCritical + null. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

Copilot

Pull request overview

This PR addresses issue #2633 by making the Orleans memory-stream subscription attach path resilient to transient grain-directory/connection instability during cluster membership changes, so a single transient rejection no longer permanently disables cross-process routing for a hub.

Changes:

Add a bounded, backoff-based retry around the stream subscription attach in OrleansRoutingService.SubscribeWhenStreamingReadyAsync.
Expose/extend transient-failure classification and add attach retry policy knobs (retry count + backoff + instance seam).
Add a deterministic test suite validating transient vs non-transient attach behavior and the bounded/capped backoff policy.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs	Adds bounded retry/backoff for stream subscription attach and makes transient failure classification testable.
test/MeshWeaver.Hosting.Orleans.Test/StreamAttachTransientRetryTest.cs	Adds unit tests to pin retry budget, non-transient behavior, production exception shape, and backoff bounds.

Suppressed comments (2)

src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs:1034

await Task.Delay(...) adds another async continuation in src/ and can run the retry loop on an arbitrary scheduler/thread, with the same deadlock/identity-loss risks noted elsewhere in this file. If a delay is needed, model it reactively (Observable.Timer) and keep the whole attach sequence inside an IObservable pipeline (or inside IIoPool if it must remain Task-based).

                wait = delay(attempt);
                onTransientRetry(ex, attempt + 1, wait);
            }
            await Task.Delay(wait, ct).ConfigureAwait(false);
        }

src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs:1077

The new bounded-retry attach is awaited directly here, increasing async/await usage in a hub-reachable service. To match the repository’s reactive conventions, consider implementing the attach-with-retry as an IObservable<StreamSubscriptionHandle<...>> (using the existing IIoPool for the Task-based Orleans call) and subscribing/bridging at the boundary, rather than adding another await in the subscription path.

            var handle = await AttachWithBoundedRetryAsync(
                AttachSubscriptionAsync,

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

rbuergi · 2026-08-29T06:53:27Z

+            try
+            {
+                return await attach().ConfigureAwait(false);
+            }


Adopted — d91493a. You are right about the idiom, and there was a strictly better version twenty lines below in this same file.

Two corrections to the reasoning, so the record is accurate about why:

The parked-scheduler / lost-AccessContext concern does not apply here. This attach runs detached — RegisterStream starts it and stores the Task; it is never on a hub action block or a grain scheduler (the method's own comment says so, and it is what SubscribeWithRetryAsync NREs because Memory stream provider never initializes #1129 restructured it to be). It also carries no AccessContext: it is subscription setup, not a delivery.

This change did not introduce async into src/. SubscribeWhenStreamingReadyAsync was already async Task<StreamSubscriptionHandle<…>?> on main — it awaits the readiness gate and awaits SubscribeAsync — and it has to stay Task-returning, because RegisterStream's teardown awaits that Task to unsubscribe whatever it produced. My edit added an async loop, not the async.

But the loop was still the wrong shape for this codebase, and the right one was already here: AttachPodHub, forty lines down, does Observable.Defer(...) + RetryWhen + Observable.Timer for the pod-hub claim, and RoutingGrain.DeliverToGrainObservable does the same for the delivery leg. The retry is now that shape:

Observable.Defer(() => attach().ToObservable()) .RetryWhen(errors => errors.Select((ex, i) => (ex, i)) .SelectMany(t => t.i >= maxRetries || !isTransient(t.ex) ? Observable.Throw<long>(t.ex) : Observable.Timer(delay(t.i), scheduler)));

That removes the async helper and the Task.Delay entirely, keeps the attach cold (so every re-subscribe re-resolves the grain — the point of the retry), and leaves exactly one bridge, .ToTask(ct), which is also where cancellation lands. IIoPool is not the boundary for this one: the leaf here is Orleans' own SubscribeAsync, invoked from an already-detached task, and routing this through a pool would add a hop without removing a Task.

Semantics are unchanged — same budget, same backoff, same Warning per retry, same terminal give-up — and I re-ran the mutation (maxRetries: 0) to confirm the test still fails with 1 attempt against an expected 6.

…b in the same file Review finding on #2645: the bounded retry was written as an async for-loop with Task.Delay, which adds async/await surface in src/ where this codebase's idiom is IObservable composition. The finding is right about idiom even though the parked-scheduler concern does not apply here (this attach runs detached, never on a hub action block or grain scheduler, and the enclosing SubscribeWhenStreamingReadyAsync was already async before this change — the Task is what RegisterStream's teardown awaits to unsubscribe the handle). There is a strictly better version and it is twenty lines above in the same file: AttachPodHub's Defer + RetryWhen + Observable.Timer, which is also RoutingGrain.DeliverToGrainObservable's shape. Adopting it removes the async helper and the Task.Delay loop, keeps the attach COLD so every re-subscribe re-resolves the grain, and leaves exactly one bridge — ToTask(ct) — where cancellation lands. Semantics are unchanged: same budget, same backoff, same loud Warning per retry, same terminal give-up. Re-verified by the same mutation (maxRetries: 0 → the transient test fails with 1 attempt against an expected 6). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

github-actions · 2026-08-29T08:09:44Z