Repository navigation
Retry a transient pod-hub delivery rejection instead of NACKing on the first attempt - #2314
Merged
Merged
Conversation
…e first attempt RoutingGrain.BuildGrainRoute (per-node hub, IMessageHubGrain) already retries a transient Orleans rejection via DeliverToGrainObservable, re-resolving the grain fresh on each attempt. BuildPodHubRoute (pod-process hub, IPodHubGrain) had no analogous retry: any rejection other than PodHubNotHereException — including the two Orleans-marked-transient shapes in the prod evidence (a ConnectionFailedException wrapped in OrleansMessageRejectionException, and a "DeactivateOnIdle was called...Rejecting now" forwarding rejection) — went straight to a terminal, sender-visible DeliveryFailure on the very first attempt (#2299). BuildPodHubRoute now drives its IPodHubGrain.Deliver call through the same DeliverToGrainObservable primitive BuildGrainRoute already uses. PodHubNotHereException stays un-retried at this layer (IsTransientFailure does not classify it), preserving its documented "definitive not-here, fall back to stream" contract. This does not fight the "no deactivating silo activates a grain" invariant (PR #2270): RoutingGrain's grain calls are the deliberately-ungated DRAIN, and it is Orleans' own placement (Catalog.GetOrCreateActivation / PlacementService.GetCompatibleSilos) that refuses to place a new activation on a non-Active silo, retry or not. Addresses the shared "stale route to a departed silo, not re-resolved" mechanism named by #2299 and #2307. #2307's specific claim about IMessageHubGrain.DeliverMessage retrying "without re-resolving placement" does not hold against the code — that path already retries with a fresh GetGrain call per attempt via DeliverToGrainWithRetry/DeliverToGrainObservable, bounded at 6 attempts with exponential backoff — but #2307 itself names this pod-hub gap and the incident control plane's own retries against it as a likely contributor to its volume. New test: RoutingGrainPodHubDeliveryRetryTest — deterministic, no cluster, pins (1) a transient pod-hub rejection is retried and recovers, (2) retries exhausted still propagates as a terminal answer, (3) PodHubNotHereException is never retried, (4) the IsTransientFailure classification boundary both rely on. Verified against two other open PRs in this area before writing this change (#2270 "no deactivating silo activates a grain", #2199 "Hub is shutting down is transient") — neither touches RoutingGrain.cs; this PR is disjoint from both. Closes #2299 Closes #2307 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Orleans routing reliability by adding bounded transient-rejection retries to the pod-hub delivery route (IPodHubGrain.Deliver), aligning it with the existing per-node hub (IMessageHubGrain) retry behavior so brief transport/activation blips don’t immediately surface as sender-visible DeliveryFailures.
Changes:
- Route pod-hub deliveries through the existing
DeliverToGrainObservableretry primitive (freshGetGrainper attempt;PodHubNotHereExceptionstill falls back immediately). - Add a deterministic, no-cluster test suite to pin retry behavior and classification boundaries.
- Add a What’s New entry documenting the user-visible behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/MeshWeaver.Hosting.Orleans/RoutingGrain.cs |
Reuses DeliverToGrainObservable for pod-hub delivery to retry transient OrleansMessageRejectionException failures while preserving the existing PodHubNotHereException fallback behavior. |
test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainPodHubDeliveryRetryTest.cs |
Adds deterministic coverage for retry success, retry exhaustion propagation, and the “don’t retry PodHubNotHere” contract. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-a-blip-to-a-hub-no-longer-drops-the-message.md |
Documents the fix as a user-facing reliability improvement. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Aug 26, 2026
This was referenced Sep 19, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mechanism, verified against the code
RoutingGrain.BuildGrainRoute(per-node hub,IMessageHubGrain) already retries a transientOrleans rejection via
DeliverToGrainObservable— re-resolving the grain (freshGetGraincall)on every attempt.
BuildPodHubRoute(pod-process hub,IPodHubGrain—cache/…,portal/…,client/…addresses) had no retry at all: any rejection other thanPodHubNotHereExceptionwent straight to a terminal, sender-visible
DeliveryFailureon the very first attempt.Issue #2299's own evidence is exactly the shape
BuildGrainRoutealready handles for the OTHERroute:
ConnectionFailedExceptionwrapped inOrleansMessageRejectionException("…will retry afterNms" — Orleans' own transport already considered it transient), and
Forwarding failed: … "DeactivateOnIdle was called." … Rejecting now— a grain mid-deactivation,the same shape
BuildGrainRoute'sDeliverToGrainWithRetrywas written for.Both are
OrleansMessageRejectionException, whichRoutingGrain.IsTransientFailurealreadyclassifies as retryable —
BuildPodHubRoutesimply never used that classification.The fix
BuildPodHubRoutenow drives itsIPodHubGrain.Delivercall through the sameDeliverToGrainObservableprimitiveBuildGrainRoutealready uses (it's grain-call-shapeagnostic —
Func<Task<IMessageDelivery>>— so this is reuse, not a new mechanism).PodHubNotHereExceptionstays un-retried at this layer (it isn't inIsTransientFailure'sclassification), so the existing "definitive not-here → fall back to the stream" contract is
unchanged — retrying it would fight
[PreferLocalPlacement]'s own documented bounce-and-loop risk.Does not fight "no deactivating silo activates a grain" (#2270)
I read PR #2270 (open, same author) before writing this. Its own doctrine is the reason retrying
here is safe:
RoutingGrain's grain calls are the deliberately-ungated DRAIN — a messagealready accepted for routing must still land, and it is Orleans' own placement
(
Catalog.GetOrCreateActivation,PlacementService.GetCompatibleSilos) that refuses to place aNEW activation on a silo that has left the ACTIVE set, retry or not. A retry here can only land a
fresh activation on a silo Orleans itself still considers healthy — never on one that is stopping.
#2270 does not touch
RoutingGrain.csat all (onlyOrleansRoutingService.cs+ two new testfiles), so this PR is disjoint from it.
I also checked #2199 ("Hub is shutting down is TRANSIENT") — it classifies a textual
Failedresult in
OrleansRoutingService, a different code path from the exception classificationthis PR touches in
RoutingGrain.cs. Also disjoint.git diff origin/main...HEAD --statshowsonly the three files below.
On #2307 — the claim doesn't fully hold, and I'm saying so
#2307 hypothesizes something calls
IMessageHubGrain.DeliverMessagein a "~1 Hz loop… withoutre-resolving placement." I verified:
RoutingGrain'sDeliverToGrainWithRetryis the onlycaller of
GetGrain<IMessageHubGrain>in the entire codebase (grep -rn "GetGrain<IMessageHubGrain>" src/ test/),and it already re-resolves (fresh
GetGrainper retry) and is bounded (6 attempts, exponentialbackoff capped ~3s, ~10s total) — not an unbounded loop. So the issue's literal claim about that
path does not hold against the code as written.
What #2307 does independently name as a likely contributor: "the incident control plane appears
to be one such caller, which would make part of this volume self-inflicted," and the pod-hub
DeliveryFailures are called out as the probable cause of the sibling incidents. That's exactlythe #2299 gap this PR closes — a pod-hub-addressed delivery (several of #2307's own destinations,
e.g.
cache/…) that used to fail terminally on one blip now gets several chances to land, whichdirectly reduces that cascade. Any residual portion of #2307 attributable to Orleans' own
cluster-membership death-detection latency (how long a genuinely-dead silo takes to leave the
ACTIVE set cluster-wide) is outside what
RoutingGrain.cscontrols, and I found no code-leveldefect there to fix — so I'm not inventing one; per AGENTS.md this PR does not widen any bound to
paper over that possibility.
Tests
New:
test/MeshWeaver.Hosting.Orleans.Test/RoutingGrainPodHubDeliveryRetryTest.cs— deterministic,no cluster (same style as the existing
RoutingGrainDeliveryRetryTest). Pins:OrleansMessageRejectionExceptiontype)is retried and recovers instead of surfacing on the first attempt,
PodHubNotHereExceptionis never retried at this layer (propagates on attempt 1),IsTransientFailureclassifies the former true and the latter false — the boundary the wholefix relies on.
Verification
dotnet build src/MeshWeaver.Hosting.Orleans -c Release -warnaserror— clean, 0 warnings.dotnet build test/MeshWeaver.Hosting.Orleans.Test -c Release -warnaserror— clean, 0 warnings.RoutingGrainDeliveryRetryTest(sibling node-hub retry suite): 7/7 pass, unaffected.PodHubTransportTest(real Orleans cluster, 4 tests covering claim/move/unclaimed/NACK): 4/4pass in isolation.
RoutingGrainTurnIsolationTest,RoutingGrainStreamPostBoundTest,MessageHubGrainKeepAliveTest,MessageHubGrainActivationSourceTest: all pass.MeshWeaver.Hosting.Orleans.Test: 197/198. The one failure,PodHubTransportTest.CrossSiloNack_ReachesASenderWhoseStreamSubscriptionIsGone, exercisesRoutingGrain.PostFailure's NACK leg — a method this PR does not touch (onlyBuildPodHubRoute'sforward-delivery call changed) — passed cleanly when run in isolation with its sibling tests, and
is the SAME test PR A deactivating silo asks Orleans for no new activation (#2252 class fix) #2270 (unrelated diff, same file area) already documented as flaking under
full-suite load ("Load-induced timeout on a machine under memory pressure… flagging it as a
possibly load-sensitive test rather than editing someone else's test"). Flake: OrleansGrainTeardownStragglerTest — the activation never leaves the silo catalog (4 hits on 2026-08-25, all unrelated diffs) #2301 independently
tracks this test project's broader pattern of load-sensitive Orleans-cluster flakes hitting
unrelated diffs. Not chased further as caused by this change; will watch this PR's CI run for it.
Closes #2299
Closes #2307
🤖 Generated with Claude Code