Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ When a `MeshNode` references `nodeType = "X"` and no provider returns a node at
> NodeType 'X' is not registered (referenced by instance '<path>').
> Either register the type via `AddXxxType()` in your mesh builder, or fix the instance's `NodeType` field. Activation cannot proceed.

Before this probe existed, the slow path waited the full `SlowPathTimeout` (`NodeTypeEnrichmentHelpers` — **60 s** today) for a typeStream emission that would never come, and activation enriches the same node twice, so a second wait used to stack on top of the first. The stuck per-node hub then jammed the routing action block, cascading 10-second timeouts to every other activation posted through the same client. (The stacking is gone — a re-enrichment short-circuit returns once `HubConfiguration` is set; the regression guard is `NodeTypeEnrichmentDoubleCallTest`.)
Before this probe existed, the slow path waited the full `SlowPathTimeout` (`NodeTypeEnrichmentHelpers` — **30 s** today, deliberately inside the 60 s hub `RequestTimeout` so the overlay can actually be delivered) for a typeStream emission that would never come, and activation enriches the same node twice, so a second wait used to stack on top of the first. The stuck per-node hub then jammed the routing action block, cascading 10-second timeouts to every other activation posted through the same client. (The stacking is gone — a re-enrichment short-circuit returns once `HubConfiguration` is set; the regression guard is `NodeTypeEnrichmentDoubleCallTest`.)

See `test/MeshWeaver.Persistence.Test/UnregisteredNodeTypeTest.cs` for the regression guard.

Expand Down
56 changes: 33 additions & 23 deletions src/MeshWeaver.Graph/Configuration/NodeTypeEnrichmentHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,41 @@ namespace MeshWeaver.Graph.Configuration;
internal static class NodeTypeEnrichmentHelpers
{
/// <summary>
/// Slow-path budget. The slow path waits for the per-NodeType compile
/// stream to emit a settled state (Ok / Error / non-NodeTypeDefinition
/// content). On timeout we fall back to
/// <see cref="WithCompilationErrorOverlay"/> so the per-instance hub still
/// activates — with an error-overlay HubConfiguration so the operator
/// sees the diagnostic instead of a dead grain.
/// Slow-path budget — the wall clock that runs ONLY while the NodeType is making no
/// progress (see <see cref="WaitForCompileSettled"/>: the deadline is disarmed with
/// <c>Observable.Never</c> for as long as a compile is genuinely in flight). On expiry we
/// fall back to <see cref="WithCompilationErrorOverlay"/> so the per-instance hub still
/// activates — with an error-overlay HubConfiguration so the operator sees the diagnostic
/// instead of a dead grain.
///
/// <para>The slow path itself nests another remote-stream subscribe (the
/// per-NodeType hub's MeshNode stream behind <c>IMeshNodeStreamCache</c>),
/// which in turn drives that hub's own activation chain. 30 s leaves
/// budget for the chained SubscribeRequest to land its Initial frame
/// before the overlay fallback kicks in. The double-enrichment that
/// stacked TWO 30 s windows on the same activation is fixed at the
/// EnrichWithNodeType fast-path (re-entry on a node that already carries
/// HubConfiguration short-circuits to the cached result) — see
/// <c>NodeTypeEnrichmentDoubleCallTest</c>.</para>
/// <para>🚨 <b>THIS BOUND MUST FIRE STRICTLY BEFORE
/// <see cref="MessageHubConfiguration.DefaultRequestTimeout"/> (60 s), OR IT DELIVERS
/// NOTHING.</b> It is an INNER bound nested inside the caller's request ceiling, and the
/// rule <c>ReadBudget</c> states for every such pair applies here verbatim: <i>a bound
/// nested inside another bound must be able to fire FIRST, because it is the only one that
/// knows WHICH read starved.</i> This constant was 60 s — exactly equal to the ceiling — so
/// the overlay it exists to produce could never reach the caller inside the caller's own
/// window. Every such activation surfaced as the ceiling's generic <c>"No response received
/// in hub …"</c>, and the specific, correct message the overlay already carries —
/// <c>"NodeType 'X' is not registered (referenced by instance '…')"</c> — had never once
/// been seen in production. The same collision is recorded from the test side in
/// <c>OrleansDynamicCompilationTest</c>, where an equal Fact timeout preempted the
/// framework's graceful sink and left "five sessions theorising from that silence".</para>
///
/// <para><b>Why lowering it costs nothing.</b> The budget never bounded a legitimate
/// compile: it is disarmed while one is in flight, and in-flight VISIBILITY is
/// <see cref="InFlightOverlayGrace"/> (5 s to a live progress overlay that self-heals onto
/// the real hub via <see cref="WithOverlaySelfHeal"/>). So this number governs exactly one
/// case — nothing is compiling and the type stream still has not settled, i.e. a dangling
/// or unreachable NodeType — where the entire remaining wait is dead time. 30 s keeps the
/// margin the surrounding design has always argued for (enough for the chained
/// SubscribeRequest to land its Initial frame) while leaving a full 30 s for the overlay to
/// be built and delivered inside the caller's ceiling.</para>
///
/// <para>Internal rather than private so the ordering invariant can be pinned by a test
/// rather than restated in a comment — see <c>NodeTypeOverlayBudgetTest</c>.</para>
/// </summary>
// Reactive wait with a sane upper bound — NOT a "make the number bigger"
// fix. A wedged owning grain never responds no matter how long we wait, so
// the budget's only jobs are to (a) outlast a legitimate cold first-compile
// and (b) then surface the overlay instead of hanging activation forever.
// 60s is the agreed cap. Correctness comes from the activity FINISHING and
// DISPOSING (writing the terminal NodeType state), which consumers observe
// via this same stream.Where(settled) — not from a longer timeout.
private static readonly TimeSpan SlowPathTimeout = TimeSpan.FromSeconds(60);
internal static readonly TimeSpan SlowPathTimeout = TimeSpan.FromSeconds(30);

/// <summary>
/// How long an activation may sit behind an IN-FLIGHT compile before it stops waiting
Expand Down
30 changes: 21 additions & 9 deletions src/MeshWeaver.Hosting/NodeUpdatePipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,27 @@ public static IObservable<MeshNode> UpdateWithValidation(IMessageHub hub, MeshNo
.Take(1)
.Timeout(TimeSpan.FromSeconds(10))
.Catch<MeshNode, Exception>(ex =>
// A non-existent / unreachable node surfaces either as a fast
// DeliveryFailureException ("No node found at …") from the owner/routing,
// or — if the per-node hub never completes its handshake — as the 10s
// Timeout. Both map to the documented "Node not found"
// InvalidOperationException; any other read error propagates unchanged.
Observable.Throw<MeshNode>(
ex is DeliveryFailureException or TimeoutException
? new InvalidOperationException($"Node not found: {node.Path}")
: ex))
// 🚨 A VERDICT AND A NON-VERDICT ARE NOT THE SAME ANSWER, and this seam used to
// report them as one. A DeliveryFailureException from the owner/routing IS a
// verdict — the mesh looked and there is no node there. The 10s Timeout is NOT:
// it means the per-node hub never completed its handshake, which is exactly what
// a node whose NodeType no longer resolves does. Collapsing both into
// "Node not found" told a caller that an EXISTING node was absent, and the only
// repair that reads as sensible from there is delete-and-recreate — on live
// content. This is the same lie the READ path was fixed for (#974), which is why
// MeshOperations.UnavailableMessage spells out "this is NOT 'not found'".
Observable.Throw<MeshNode>(ex switch
{
DeliveryFailureException => new InvalidOperationException(
$"Node not found: {node.Path}"),
TimeoutException => new InvalidOperationException(
Comment on lines +49 to +53
$"Unavailable: {node.Path} — this update could not read the node's current "
+ "state, so it is UNKNOWN whether the node exists. This is NOT 'not "
+ "found': do not delete or recreate anything on the strength of it. A "
+ "node whose NodeType no longer resolves fails exactly this way. "
+ "Retry shortly.", ex),
_ => ex
}))
.SelectMany(existing => RunUpdateValidators(hub, node, existing, ctx)
.SelectMany(error => error is not null
? Observable.Throw<MeshNode>(error)
Expand Down
18 changes: 17 additions & 1 deletion src/MeshWeaver.Messaging.Hub/MessageHubConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,23 @@ private static bool IsAccessContextExempt(object? message)
// covered the test-side post but intermediate sync/mesh hubs still
// followed the 30s default. 60s as a framework default matches that
// ceiling; anything genuinely longer than that is a real bug.
internal TimeSpan RequestTimeout { get; init; } = new(0, 1, 0);
internal TimeSpan RequestTimeout { get; init; } = DefaultRequestTimeout;

/// <summary>
/// 🚨 THE FRAMEWORK'S LAST-RESORT REQUEST CEILING — <b>60 s</b>, and the OUTER bound every
/// nested budget must stay strictly inside.
///
/// <para><b>Why this is public.</b> It was previously a bare literal on
/// <see cref="RequestTimeout"/>, invisible outside this assembly, so every nested bound that
/// needed to stay under it hand-copied the number into a comment instead of referencing it.
/// That is exactly how a nested bound drifts up until it EQUALS this one — and an inner bound
/// that cannot fire first cannot deliver its diagnostic, so the caller gets this ceiling's
/// generic "No response received in hub …" instead of the specific reason. See
/// <c>ReadBudget</c>'s class remarks for the rule and its incident history, and
/// <c>NodeTypeEnrichmentHelpers.SlowPathTimeout</c> for the bound this exists to keep honest.
/// </para>
/// </summary>
public static readonly TimeSpan DefaultRequestTimeout = TimeSpan.FromSeconds(60);

/// <summary>
/// Quiescing-phase drain budget per hub. When <see cref="MessageHub.Dispose"/> fires,
Expand Down
66 changes: 66 additions & 0 deletions test/MeshWeaver.Graph.Test/NodeTypeOverlayBudgetTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using MeshWeaver.Graph.Configuration;
using MeshWeaver.Messaging;
using Xunit;

namespace MeshWeaver.Graph.Test;

/// <summary>
/// Pins the ORDERING INVARIANT between the NodeType slow-path budget and the hub request ceiling
/// it is nested inside.
///
/// <para><b>The bug this pins.</b> <c>NodeTypeEnrichmentHelpers.SlowPathTimeout</c> was 60 s and
/// <see cref="MessageHubConfiguration.DefaultRequestTimeout"/> is 60 s. Two equal bounds means the
/// inner one can never fire first — so the compilation-error overlay the slow path exists to
/// produce could never be delivered inside the caller's own window. The overlay's message is
/// correct and specific (<c>"NodeType 'X' is not registered (referenced by instance '…')"</c>) and
/// it had never been seen: every occurrence surfaced instead as the ceiling's generic
/// <c>"No response received in hub …"</c>, which names neither the culprit nor the remedy.</para>
///
/// <para><b>Why a test and not a comment.</b> The ceiling is in a different assembly and was, until
/// this change, invisible outside it — so the relationship existed only as a hand-copied number in
/// prose, which is precisely how it drifted into collision. A comment cannot fail. This can.</para>
///
/// <para>The same collision is recorded independently from the test side in
/// <c>OrleansDynamicCompilationTest</c>, where an equal Fact timeout preempted the framework's own
/// graceful sink and left, in its author's words, "five sessions theorised from that silence".</para>
/// </summary>
public class NodeTypeOverlayBudgetTest
{
/// <summary>
/// The inner bound must fire STRICTLY before the outer one. Equality is the defect.
/// </summary>
[Fact]
public void SlowPathBudget_FiresStrictlyInside_TheHubRequestCeiling()
=> NodeTypeEnrichmentHelpers.SlowPathTimeout
.Should().BeLessThan(MessageHubConfiguration.DefaultRequestTimeout,
"the slow-path overlay is the only party that knows WHICH NodeType starved, so it "
+ "must be able to fire before the hub ceiling reports its generic timeout");

/// <summary>
/// Strictly-less is necessary but not sufficient: the overlay still has to be BUILT and
/// DELIVERED after the budget expires. A one-second margin would satisfy "less than" and still
/// lose the diagnostic in transit, so the headroom is pinned too.
/// </summary>
[Fact]
public void SlowPathBudget_LeavesRealHeadroom_ForTheOverlayToBeDelivered()
{
var ceiling = MessageHubConfiguration.DefaultRequestTimeout;
var headroom = ceiling - NodeTypeEnrichmentHelpers.SlowPathTimeout;

headroom.Should().BeGreaterThanOrEqualTo(
TimeSpan.FromSeconds(ceiling.TotalSeconds / 4),
"expiring a hair under the ceiling still loses the overlay to the caller's timeout — "
+ "the margin has to cover building the overlay hub and delivering its first frame");
}

/// <summary>
/// The in-flight visibility grace must surface its live progress overlay well before the
/// no-progress budget gives up, or a slow-but-healthy compile would be reported as a failure.
/// </summary>
[Fact]
public void InFlightGrace_SurfacesProgress_BeforeTheNoProgressBudgetExpires()
=> NodeTypeEnrichmentHelpers.InFlightOverlayGrace
.Should().BeLessThan(NodeTypeEnrichmentHelpers.SlowPathTimeout,
"a compile that is genuinely running must show progress rather than fall out of "
+ "the no-progress budget as an error");
}
Loading