Skip to content

fix(routing): a grain that deactivated is a lifecycle transition, not a delivery failure (#2299, classification half) - #4914

Merged
meshweaver-cloud[bot] merged 2 commits into
mainfrom
fix/2299-deactivated-activation
Sep 19, 2026
Merged

meshweaver-cloud[bot] merged 2 commits into
mainfrom
fix/2299-deactivated-activation

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Addresses the classification half of #2299, which that issue's 2026-09-18 triage named as "what still stands":

ClassifyDeliveryException answers ShuttingDown only for IsDirectoryUnstable || IsShutdownShaped || IsScopeTeardown, and IsShutdownShaped matches only SiloUnavailableException / OrleansLifecycleCanceledException — neither of this issue's two shapes. So after the six retries the sender still receives a terminal Failed.

The rule, and why this qualifies under it

The classifier's own rule is that only conditions that are a lifecycle transition by construction are transient, and "anything this does not recognise stays terminal, so a genuine defect is still reported as one". Two qualified: the grain directory mid-handoff, and the host going away.

Orleans deactivating an idle grain is a third by the same argument — the grain-level analogue of the host going away, and the target re-activates on the next call. Reported as terminal it tore down every consumer carrying its own recovery machinery, which is the damage #2346/#2357 removed for the silo-level shapes and left standing for this one.

The production shape, read from Admin/_LogIncident/e849e4a7795e0c92 (memex, 2026-09-17, 191 occurrences against a single address, senders Hosting/InstanceAction / Hosting/Deployment / Hosting/DeploymentStatus):

Orleans.Runtime.OrleansMessageRejectionException: Forwarding failed: tried to forward message
Request […] for 2 times after "DeactivateOnIdle was called." to invalid activation. Rejecting now.

🚨 The signal is Orleans' message text, and I want that flagged rather than buried

There is no typed rejection reason to read. OrleansMessageRejectionException carries every rejection kind, so accepting the type alone would classify genuine refusals as transient — the one direction this file says must not happen. The test is therefore the narrowest phrase that means the activation is gone, matched on the OrleansException base it derives from (Orleans keeps the derived type's constructors internal, which is also why the test constructs the base).

This is the same shape IsDirectoryUnstable already uses beside it, so it is not a new kind of fragility — but it is fragility. If Orleans re-words that message this quietly returns to the pre-fix answer rather than misclassifying anything, which is the safe direction to fail in, and the test pins the production string verbatim rather than a paraphrase so a reader can see exactly what is being matched.

Verification

Release with -warnaserror: 0 Warning(s), 0 Error(s). Classification suites 34/34 — the pre-existing ones included, so nothing else moved.

Mutation-measured in both directions, because over-accepting here masks defects:

mutation red
predicate not wired into the classifier (the pre-fix answer) TheProductionRejection_IsClassifiedAsShuttingDown, ItIsFoundThroughTheExceptionGraph_NotJustAtTheTop
accept the exception type alone, drop the phrase AnOrdinaryRejection_StaysTerminal

The second is the one that matters: it shows the phrase is load-bearing rather than decoration.

The refusing direction is pinned deliberately too — an unrelated fault and a TimeoutException both stay terminal. The timeout especially: this file argues a target that did not answer across the whole budget is plausibly wedged rather than restarting, and demoting it would produce a resubscribe storm against a hub that never comes back (the 2026-06-08 shape).

What this does NOT do

#2299 stays open. Its second half is untouched: the self-sustaining re-activation cycle against portal/reads-8Q6M4hfklkiWR_j2go6T-w — the same address deactivating and being re-activated from two pods and three senders over 13:06→14:15Z. This changes what the sender is told; it does not stop the loop. I have not attempted that half, which needs the deactivation/re-activation interplay traced rather than a classifier edit.

I also only cover the shape I have ground truth for. The triage mentions "two shapes"; the incident's newest samples are all this one.

Mirror-sync: none — no i18n catalog key is added or re-worded.

🤖 Generated with Claude Code

… a failure

ClassifyDeliveryException answers ShuttingDown for the directory
mid-handoff, the host going away and the container being disposed --
"only the two conditions that ARE a lifecycle transition by
construction". Orleans deactivating an IDLE GRAIN is a third, and was
not recognised, so after the retries the sender received a terminal
Failed and every consumer with its own recovery machinery tore it down
for a target that re-activates on the next call.

The production shape, from Admin/_LogIncident/e849e4a7795e0c92 (memex,
2026-09-17, 191 occurrences against one address):

  Forwarding failed: tried to forward message ... for 2 times after
  "DeactivateOnIdle was called." to invalid activation. Rejecting now.

🚨 The signal is Orleans' own MESSAGE TEXT, which is weaker than a type
and known to be -- the same shape IsDirectoryUnstable already uses. There
is no typed rejection reason: the rejection type carries every kind of
refusal, so accepting it alone would classify genuine refusals as
transient, the one direction this file says must not happen. If Orleans
re-words it this returns to the pre-fix answer rather than
misclassifying, which is the safe direction to fail in.

Tested against the VERBATIM production string, through the exception
graph (it arrives via Rx Catch arms and a two-transport
AggregateException), and in the refusing direction: an ordinary
rejection, an unrelated fault and a timeout all stay terminal.

Controls: not wired in -> the two positives red; type alone without the
phrase -> the ordinary-rejection test reds. Classification suites 34/34.

This is the classification half of #2299 only. The re-activation loop
underneath -- the same address deactivating and being re-activated in a
self-sustaining cycle -- is untouched and that issue stays open for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 19, 2026 18:52

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

🟡 Changes recommended

The shared classifier also marks persistent activation-fault rejections as transient, potentially masking real failures and causing resubscribe storms.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Extends routing failure classification so deactivated Orleans grains are reported as transient lifecycle events, with regression tests and a What's New entry.

Changes:

  • Adds IsDeactivatedActivation classification based on Orleans rejection text.
  • Adds positive, nested, and terminal classification tests.
  • Documents the user-visible routing fix.
File Description
src/​MeshWeaver.Hosting.Orleans/​RoutingGrain.cs Adds deactivated-activation classification.
test/​MeshWeaver.Hosting.Orleans.Test/​DeactivatedActivationClassificationTest.cs Covers classification behavior.
src/​MeshWeaver.Documentation/​Data/​WhatsNew/​2026-09-19-a-grain-that-deactivated-is-not-a-delivery-failure.md Adds release documentation.

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

Comment on lines +1875 to +1878
internal static bool IsDeactivatedActivation(Exception ex) =>
ExceptionChain.Contains(ex, e =>
e is global::Orleans.Runtime.OrleansException
&& e.Message.Contains("to invalid activation", StringComparison.Ordinal));

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.

You are right, and this is the direction the file says must never happen. I am reworking the PR rather than defending it.

GrainActivationFailureRegistry's own summary is the proof: a per-node hub in a persistent activation-fault loop — a broken NodeType compile that cannot materialise a hub configuration — has an alive window of about zero, so "every delivery ... lands in a deactivation window. Orleans then answers the raw OrleansMessageRejectionException" with that same DeactivateOnIdle / Rejecting now text. Byte-identical text, opposite meaning: one recovers on the next call, the other never will.

My predicate matches the text alone, so it would report that broken NodeType to the sender as ShuttingDown — a genuine, permanent defect classified as a lifecycle transition. It also fights the registry, which exists so RoutingGrain can serve the REAL activation error on the exhausted-retry path instead of Orleans internals.

The fix is to make the registry the discriminator, the same shape IsScopeTeardown already uses for its container probe: the text becomes a necessary condition, and a probe finding NO stored activation failure for that grain key is what turns it into a positive statement about an idle deactivation. Absent a probe it must answer false, so the default stays terminal — the safe direction.

I will push that with a test for the persistent-loop case specifically, and will not merge this as it stands.

Comment on lines +1857 to +1859
/// <para>🚨 <b>This matches Orleans' own message text, which is a weaker signal than a type and
/// is known to be</b> — the same shape <see cref="OrleansRoutingService.IsDirectoryUnstable"/>
/// already uses beside it. There is no typed rejection reason to read:

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.

Correct — "is known to be" has no complement. It was meant to read "is known to be fragile". Fixing it in the same push as the substantive rework above.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

601 tests   601 ✅  27s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

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

Results for commit f738004.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

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

Results for commit f738004.

♻️ This comment has been updated with latest results.

Copilot is right and the PR was unsafe as it stood.
GrainActivationFailureRegistry documents the collision: a per-node hub
in a PERSISTENT activation-fault loop (a broken NodeType compile) has an
alive window of about zero, so every delivery lands in a deactivation
window and Orleans emits the SAME 'DeactivateOnIdle ... to invalid
activation. Rejecting now.' text as an ordinary idle deactivation. One
recovers on the next call; the other never does. Matching the text alone
classified that permanent defect as transient -- the one direction this
file says must not happen.

The text is now a NECESSARY condition and the registry is the
discriminator. ClassifyDeliveryException takes activationErrorRecorded,
defaulting to TRUE (assume the worse case), so any caller that cannot
consult the registry keeps the pre-#4914 terminal verdict.

The one call site that CAN answer already computes it: the grain-call
fault arm resolves the stored activation error for #464's Defect 3 and
passes !string.IsNullOrEmpty(activationError). The pod-hub arm has no
grain activation and passes nothing.

Also fixes the incomplete doc sentence the review flagged.

Tests: the persistent-loop case and the no-answer case both pin
terminal. Control -- drop the registry check -- reds exactly those two.
Classification suites 36/36, Documentation 601/601.

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