fix(routing): never re-send a delivery the callee is still holding (#1172)

src/MeshWeaver.Connection.Orleans/OrleansRoutingService.cs

@@ -612,7 +612,17 @@ private IObservable<IMessageDelivery> DispatchObservable(IMessageDelivery delive
                 .Select((ex, i) => (Exception: ex, Attempt: i))
                 .SelectMany(t =>
                 {
-                    if (t.Attempt >= 5 || !IsTransientFailure(t.Exception))
+                    // 🚨 IsResendableDeliveryFailure, NOT IsTransientFailure — issue #1172.
+                    // RouteMessage is not idempotent: it routes the delivery on. A response timeout
+                    // means the routing grain ACCEPTED this delivery and has not answered yet (its
+                    // turn is [StatelessWorker(1)] and non-reentrant, so a timeout here means that
+                    // one turn is busy), and re-sending queues a SECOND copy behind the first — on
+                    // the very queue whose depth was the 2026-08-07 541-deep NonReentrancyQueueSize
+                    // incident. The delay ladder below is sized for a rejection returned instantly;
+                    // a timed-out attempt costs the whole ResponseTimeout, so on the timeout class
+                    // the "bounded budget" that licensed retrying at all is minutes of held slots
+                    // plus up to six duplicate deliveries.
+                    if (t.Attempt >= 5 || !IsResendableDeliveryFailure(t.Exception))
                         return Observable.Throw<long>(t.Exception);
                     var delay = TimeSpan.FromMilliseconds(Math.Min(200 * Math.Pow(2, t.Attempt), 30_000));
                     logger.LogDebug(t.Exception, "Transient failure delivering to {Address}, attempt {Attempt}/5, retrying in {Delay}ms",
@@ -774,6 +784,13 @@ private bool SendDeliveryFailure(IMessageDelivery delivery, string message,
     /// A failure worth another attempt: a transport-level blip, an Orleans rejection, or the grain
     /// directory mid-handoff. Mirrors <c>RoutingGrain.IsTransientFailure</c>; <c>internal</c> so a
     /// test can pin the PREMISE of the attach retry (issue #2633) rather than only its effect.
+    ///
+    /// <para>🚨 <b>This answers "is another attempt worth making", NOT "may this request be SENT
+    /// AGAIN" — see <see cref="IsResendableDeliveryFailure"/>.</b> The two differ on exactly one
+    /// class, the response timeout, and a NON-idempotent call must gate on the narrower predicate.
+    /// This one stays as it is because its other caller — <see cref="AttachWithBoundedRetry{T}"/>'s
+    /// <c>IPodHubGrain.Attach</c> claim — IS idempotent (it sets flags and re-pins an activation),
+    /// so re-sending it after a timeout costs nothing and is how the claim converges.</para>
     /// </summary>
     /// <param name="ex">The exception a cluster call faulted with.</param>
     /// <returns><c>true</c> when re-attempting is worthwhile.</returns>
@@ -787,6 +804,62 @@ or TimeoutException
             || (ex.InnerException != null && IsTransientFailure(ex.InnerException));
     }
 
+    /// <summary>
+    /// 🚨 <b>The CALLER gave up waiting; the CALLEE may still hold the request — issue #1172.</b>
+    ///
+    /// <para>Orleans' <c>ResponseTimeout</c> is a caller-side give-up timer, never a cancellation.
+    /// When it fires, the request has already been handed to the target activation and sits in its
+    /// work queue until that activation gets to it; nothing recalls it. So a timeout says something
+    /// categorically different from a REJECTION: a rejection means the callee refused and holds
+    /// nothing (<c>"… to invalid activation. Rejecting now."</c> — a grain mid-<c>DeactivateOnIdle</c>,
+    /// which is the whole case the delivery retry was built for in #2314), while a timeout means the
+    /// callee accepted and has not answered yet.</para>
+    ///
+    /// <para>The walk is <see cref="ExceptionChain"/>'s — the exception GRAPH, not the
+    /// <c>InnerException</c> line — for the same reason <c>RoutingGrain.IsScopeTeardown</c> uses it:
+    /// these arrive through Rx <c>Catch</c> arms and two-transport <see cref="AggregateException"/>s
+    /// where which fault sits at index 0 is a race.</para>
+    /// </summary>
+    /// <param name="ex">The exception a cluster call faulted with.</param>
+    /// <returns><c>true</c> when the fault is — or carries — a transport response timeout.</returns>
+    internal static bool IsResponseTimeout(Exception ex) =>
+        ExceptionChain.Contains<TimeoutException>(ex);
+
+    /// <summary>
+    /// 🚨 <b>May this request be SENT AGAIN? Narrower than <see cref="IsTransientFailure"/> by
+    /// exactly the response-timeout class, and the difference is issue #1172.</b>
+    ///
+    /// <para>This is the predicate every retry over a NON-IDEMPOTENT call must use. Neither
+    /// <c>IMessageHubGrain.DeliverMessage</c> nor <c>IPodHubGrain.Deliver</c> nor
+    /// <c>IRoutingGrain.RouteMessage</c> carries an idempotency key — nothing on the receive path
+    /// reads <c>IMessageDelivery.Id</c> to recognise a repeat, and <c>DeliverMessage</c> ends in
+    /// <c>hub.DeliverMessage(delivery)</c>, an unconditional post onto the target hub's queue. So a
+    /// re-send after <see cref="IsResponseTimeout"/> does not retry the delivery, it DUPLICATES it:
+    /// the copy already queued at the callee still runs, and the handler runs once per attempt.</para>
+    ///
+    /// <para><b>Why that was the amplifier behind #1172.</b> The standing rationale for letting
+    /// <see cref="IsTransientFailure"/> be generous was that "it is bounded by a retry budget" — and
+    /// the budget's DELAYS (200 ms → capped) are sized for a rejection, which is returned instantly.
+    /// An attempt that ends in a timeout does not cost 200 ms, it costs the transport's whole
+    /// <c>ResponseTimeout</c>. So the same six-retry ladder means ~10 s for a rejection and MINUTES
+    /// for a timeout, during which the delivery holds a router dispatch slot and has deposited up to
+    /// seven copies of itself on a target that was already too slow to answer one. A response
+    /// timeout is most likely precisely when the silo is CPU/thread starved — so the retry multiplied
+    /// the starved silo's own work at the moment it had least capacity, which is a positive feedback
+    /// loop, not a recovery.</para>
+    ///
+    /// <para><b>Nothing is suppressed by declining to re-send.</b> The fault reaches the same arm it
+    /// reached after the retries were exhausted, and <c>RoutingGrain.ClassifyDeliveryException</c>
+    /// already answers a bare <see cref="TimeoutException"/> with the TERMINAL
+    /// <c>ErrorType.Failed</c> — deliberately, because "a target silent across the whole budget is
+    /// plausibly wedged". The sender therefore gets the identical verdict it always got, one
+    /// <c>ResponseTimeout</c> after the first attempt instead of seven of them later.</para>
+    /// </summary>
+    /// <param name="ex">The exception a delivery attempt faulted with.</param>
+    /// <returns><c>true</c> when the delivery may be sent again.</returns>
+    internal static bool IsResendableDeliveryFailure(Exception ex) =>
+        !IsResponseTimeout(ex) && IsTransientFailure(ex);
+
     /// <summary>
     /// Orleans could not ADDRESS the call because its own grain directory is mid-handoff — a silo is
     /// joining or leaving and the directory partition that owns the target's entry has not settled.

src/MeshWeaver.Documentation/Data/Architecture.md

@@ -362,6 +362,7 @@ Each theme starts with its introductory page, followed by related architecture t
 - [An Unreachable Store Is Not a Refusal](StoreUnreachableIsNotARefusal) — one classification, three consumers; reporting an availability failure as a verdict is how a retried create becomes a duplicate
 - [A Name That Does Not Resolve Is Not Transient](ANameThatDoesNotResolveIsNotTransient) — the default pipeline retried a hostname that does not exist three times and logged each attempt at Error, so a URL in somebody's data manufactured a platform incident; the one socket error that is permanent, why the breaker must not count it either, and the control that keeps a nameserver hiccup retryable
 - [A Departed Silo Is Not a Delivery Defect](ADepartedSiloIsNotADeliveryDefect) — two incidents (191 and 3,959 occurrences) read as two defects for four weeks and are ONE root; which predicate sees which of the four rejection shapes, why the cure is the classifier and never a retry, and why an incident fingerprinted on a dependency's logger counts attempts rather than verdicts
+- [A Timed-Out Delivery Is Still Held by the Callee](ATimedOutDeliveryIsStillHeldByTheCallee) — a response timeout is a caller-side give-up timer, so the six-retry ladder sized for an instant rejection re-sent seven copies of every slow delivery and held a dispatch slot for 3 m 40 s; the three-predicate ladder that separates "is this transient" from "may we send it again", and the one caller that keeps the wider answer because it is idempotent
 - [A Bulk Create Compensates Per Node](BulkCreateCompensation) — every row is durable before any post-creation handler runs, so one critical failure left the failed node AND every node after it, whose handlers never ran and which nothing can tell apart from a success; what the rollback removes, why it walks backwards, and the measured reason the stop is a fault and not a `Take(1)`
 - [Undetermined Is Not No](UndeterminedIsNotNo) — a read that did not answer is a THIRD state; the second door that shared the first door's failure domain, and the rule for what a gate does with "I could not determine"
 - [Reading a Silo Eviction](ReadingASiloEviction) — a heartbeat newer than the suspect votes is not proof the silo was healthy; the control arm that tells a correct eviction from a false positive

src/MeshWeaver.Documentation/Data/Architecture/ATimedOutDeliveryIsStillHeldByTheCallee.md

@@ -0,0 +1,130 @@
+---
+Name: A Timed-Out Delivery Is Still Held by the Callee
+Category: Architecture
+Description: Orleans' ResponseTimeout is a caller-side give-up timer, not a cancellation — so re-sending a timed-out delivery duplicates it and pays another whole timeout per attempt. The three-predicate ladder that separates "is this transient", "may we send it again" and "should the sender keep recovering", and why the routing back-pressure report at 64 in flight is the far end of that amplifier.
+Icon: <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><circle cx="12" cy="12" r="9"/><path d="M12 7v5l3 2"/><path d="M19 5 5 19"/></svg>
+---
+
+# A Timed-Out Delivery Is Still Held by the Callee
+
+A rejection and a timeout are both "transient". They are opposite facts about **who holds the
+request**, and a retry that cannot tell them apart turns a slow destination into a message storm.
+
+## The distinction
+
+| | What the callee did | What a re-send does |
+|---|---|---|
+| **Rejection** (`OrleansMessageRejectionException` — *"… to invalid activation. Rejecting now."*) | REFUSED it. Holds nothing. | Re-resolves placement, so the message lands on a freshly activated grain. This is the case the delivery retry exists for. |
+| **Response timeout** (`TimeoutException`) | ACCEPTED it and has not answered yet. The request sits in that activation's work queue. | DUPLICATES it. The queued copy still runs. |
+
+Orleans' `ResponseTimeout` is a **caller-side give-up timer, not a cancellation** — nothing recalls a
+message once it has been handed to the target activation. So a timeout says only that the caller
+stopped waiting.
+
+Nothing on the receive path can recognise a repeat. `IMessageHubGrain.DeliverMessage` ends in
+`hub.DeliverMessage(delivery)` — an unconditional post onto the target hub's queue — and no reader
+of `IMessageDelivery.Id` dedupes. **At-least-once delivery over a non-idempotent handler is
+at-least-once handler execution.**
+
+## Why it was an amplifier, not a nuisance
+
+The standing rationale for letting the transient predicate match `TimeoutException` was that *"it is
+bounded by a retry budget, so it can afford to be generous"*. The budget is bounded in **attempts**
+(6) and its delays are sized for the fault it was written for — a rejection, which Orleans returns
+instantly: 250 ms → 3 s, 9.75 s in total.
+
+A timed-out attempt does not cost 250 ms. It costs the transport's whole `ResponseTimeout`. The same
+ladder therefore means two very different things:
+
+| fault class | attempts | wall clock per delivery | copies queued at the callee |
+|---|---|---|---|
+| rejection | 7 | ~10 s | 1 (each previous one was refused) |
+| response timeout | 7 | **~3 m 40 s** | **7** |
+
+And the direction of the coupling is the problem. A response timeout on a delivery leg happens
+precisely when the destination is slow — a per-node hub whose `HubReady` has not emitted yet, which
+for an `_Activity/compile` address means an in-mesh NodeType compile — or when the silo is CPU/thread
+starved. In both cases the retry **multiplies the work of the thing that was already too slow**, at
+the moment it has least capacity, while each leg holds one `RoutingGrain` dispatch slot for the whole
+3 m 40 s. That is a positive feedback loop wearing a recovery's clothes.
+
+Issue #1172 is the far end of it: `[ROUTE] Routing back-pressure … 64 route dispatches in flight`,
+whose original evidence was dominated by `_Activity/compile` and `_Activity/import` targets — the
+addresses whose `HubReady` takes longest, i.e. exactly the ones the ladder re-sent to seven times.
+
+## The ladder
+
+Three predicates, three different questions, each strictly narrower than the last. Do not collapse
+them.
+
+```
+IsTransientFailure(ex)               "is another attempt CONCEIVABLE?"
+  ⊇ IsResendableDeliveryFailure(ex)  "may we SEND THIS REQUEST AGAIN?"
+      ⊇ ClassifyDeliveryException(ex) == ShuttingDown
+                                     "should the SENDER keep its unbounded recovery armed?"
+```
+
+- **`IsTransientFailure`** — unchanged, and its classification was never wrong; it was being asked
+  the wrong question. A timeout IS a transient fault.
+- **`IsResendableDeliveryFailure`** = `!IsResponseTimeout(ex) && IsTransientFailure(ex)`. The gate on
+  every retry over a **non-idempotent** call: `RoutingGrain.DeliverToGrainObservable` (which serves
+  both `IMessageHubGrain.DeliverMessage` and `IPodHubGrain.Deliver`) and
+  `OrleansRoutingService.DispatchObservable`'s `IRoutingGrain.RouteMessage`.
+- **`ClassifyDeliveryException`** — unchanged, and deliberately narrowest: a bare `TimeoutException`
+  stays the TERMINAL `ErrorType.Failed`, because telling a consumer "transient" arms an *unbounded*
+  resubscribe against a target that is plausibly wedged.
+
+`IsResponseTimeout` walks the exception **graph** (`ExceptionChain`), not the `InnerException` line.
+These faults arrive through Rx `Catch` arms and two-transport `AggregateException`s where which fault
+sits at index 0 is a race, so a walker that only followed `InnerException` would re-send or not
+depending on the ordering.
+
+### The one caller that keeps the wider predicate
+
+`OrleansRoutingService.AttachWithBoundedRetry`'s `IPodHubGrain.Attach` claim is **idempotent** — it
+sets flags and re-pins an activation — so re-sending it after a timeout costs nothing and is how the
+claim converges (#2633). That is why `IsTransientFailure` is left intact rather than narrowed in
+place: narrowing it would have silently disarmed the pod-hub claim retry.
+
+## Declining to re-send suppresses nothing
+
+The fault reaches the **same** arm it reached after the retries were exhausted, and
+`ClassifyDeliveryException` gives the sender the **same** verdict it always got. The only change is
+*when*: one `ResponseTimeout` after the first attempt instead of seven of them later, with one copy
+of the delivery at the callee instead of seven. The sender's own recovery — `SynchronizationStream`'s
+resubscribe latch, `MeshNodeStreamCache`'s transient-owner rule, an `Observe(...)` subject firing
+`OnError` — decides what happens next, as before.
+
+## What this does NOT explain
+
+The back-pressure report is a **gauge, not a bound** — nothing throttles, queues or refuses at 64;
+`RoutingGrain.ReportSaturation` carries the whole reasoning, including why the number it prints is
+always exactly the threshold. So this change makes the report rarer and each episode shorter; it does
+not make the counter mean something new. Two shapes remain at that log site and neither is closed by
+this:
+
+- **`deepest per-destination queue 0`** — pure breadth. Legs in flight, nothing waiting on anything.
+  Reduced by a factor of up to 7 on the timeout path, but still reachable under genuine load, and
+  still inflated by the unbounded wait for a ThreadPool thread that a slot is held across *before*
+  the leg's own timeouts start.
+- **`deepest per-destination queue 62` on one `cache/…` destination** — head-of-line blocking in
+  `OrderedRouteDispatcher`'s per-destination FIFO. The FIFO's key is the **destination address**
+  while the ordering invariant it protects (`SynchronizationStream`'s receive-side monotonicity
+  guard) is **per stream**, so all traffic to one process's cache hub is serialised whether or not
+  any ordering relationship exists between two frames. That over-broadness is what lets one slow
+  destination stack ~62 legs. Measured on memex-cloud 2026-09-19 in two independent episodes
+  (`fcf31578#14`, `67341d12#7`). Not addressed here.
+
+The earlier root already fixed on this path was O(node-size) JSON patch construction on hub action
+blocks (#1341), and the earlier slot LEAK was `IoPool.SubscribeThroughPool` terminating an observer
+in neither direction when the drain cancelled it (#1358). This is the third mechanism at the same
+log site, and the first one that was a *classification* error rather than a cost.
+
+## Where it lives
+
+| | |
+|---|---|
+| `OrleansRoutingService.IsResponseTimeout` | the one definition of "the callee may still hold it" |
+| `OrleansRoutingService.IsResendableDeliveryFailure` | the gate, client side (`RouteMessage`) |
+| `RoutingGrain.IsResendableDeliveryFailure` | the gate, router side (both forward delivery legs) |
+| `TimedOutDeliveryIsNotResentTest` | 6 facts: the delivery is sent exactly once, a rejection still spends its whole budget, both aggregate orderings agree, and all three rungs of the ladder |