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 @@ -39,8 +39,8 @@ thread**.
So on the `DataContext` seam the two live on different threads, and both of them drain the *same*
deferred backlog through `MessageService.DrainDeferredDeliveries`:

- the teardown's drain answers *"Hub X was disposed while <message> was still deferred — the
message was never processed"*;
- the teardown's drain answers *"Hub X is shutting down (…) — <message> was still deferred;
initialization gates closed at deferral: […] — the message was never processed"*;
- `FailGate` → `FailDeferredBacklog` answers with the transient cause.

`DrainDeferredDeliveries` claims each entry with `TryRemove`, so exactly one of them answers any
Expand Down Expand Up @@ -118,8 +118,17 @@ Hub host/1 is shutting down — its DataContext initialization met a transient i
reactivate (recycle / restart); retry to get the authoritative answer.
```

If instead you read *"…was disposed while X was still deferred … the message was never processed"*,
the retirement lost its own race: something failed the gate after the teardown had begun.
If instead you read *"…X was still deferred; initialization gates closed at deferral: […] — the
message was never processed"*, the retirement lost its own race: something failed the gate after the
teardown had begun.

🚨 **Tell them apart by the CLAUSE, never by the banner.** Since #4866 the teardown's drain composes
through `ShutdownNack.RetryForTheAuthoritativeAnswer` too — it was the last owner-side refusal that
did not, and the omission made it unrecognisable to every transient classifier, which is
[the defect that page records](/Doc/Architecture/RidingOutAShuttingDownAddress). So
`IsAnsweredByOwner` is now true of *both* answers, as it should be: both really are this owner
speaking. What separates them is what they say — a named fault, or a delivery that was still parked
behind its gates.

## Related

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,64 @@ answer that was already in the log. It is #1029's silence on the door #1029 did
intermittent only because the owner's own teardown notice usually reaches the reader before the
change-feed event latches the re-ask.

## 🚨 The promise lives in the TEXT, not in the envelope (#4866)

Everything above assumes the rider can *tell* that it was handed the promise. It cannot do that from
`ErrorType`. Every classifier that decides "re-probe the fresh activation" against "take this answer
as final" reads the **message text**:

| Classifier | Where | Reads |
|---|---|---|
| `MeshNodeStreamCache.IsTransientOwnerFailure` | `MeshWeaver.Hosting` | `"is shutting down"`, `"Rejecting now"`, `"invalid activation"`, … |
| `AreaErrorClassifier.IsTransientHubFailure` / `IsHubRecycling` | `MeshWeaver.Layout` | the same markers |
| `OrleansRoutingService.ClassifyRoutedFailure` | `MeshWeaver.Connection.Orleans` | the same markers |
| `ShutdownNack.IsAnsweredByOwner` | `MeshWeaver.Messaging.Contract` | `ShutdownNack.Banner` — *"Hub X is shutting down"* |

`ShutdownNack.IsAnsweredByOwner` is deliberately **not** an `ErrorType` test, and its own doc comment
says why: the routing layer mints that same classification off the very same text, so an
`ErrorType` test answers the question with the routing layer's echo of it. The banner is the only
evidence that names the speaker.

So an owner-side refusal that is *stamped* `ErrorType.ShuttingDown` but *worded* without the banner
is a promise nobody can collect. The envelope is transient, every reader is terminal, and there is
nothing to grep: no compiler error, no exception, no log line — just a consumer that quietly stops
asking.

**That is what the disposal drain did.** The seam that answers deliveries still parked behind an
initialization gate when the hub goes down hand-wrote its sentence — *"Hub X **was disposed** while
GetDataRequest (id=…) was still deferred …"* — instead of composing it through `ShutdownNack`. It
carried no banner and matched no marker, so `IsAnsweredByOwner` reported *"the routing layer refused
you"* and every rider took a recycling hub's answer as final. Measured in #4866: one pod, one
sub-second window, **105** accepted-and-deferred `GetDataRequest`/`SubscribeRequest` deliveries
discarded when every hub bound to a `Store/Plugin` overlay self-healed at once — each one answered
with a sentence that told the sender to *"retry to get the authoritative answer"* in wording the
sender could not recognise as retryable. The recycle itself was correct; the answer to the work it
displaced was not.

The fix is the seam, not a new drain: `ShutdownNack.RetryForTheAuthoritativeAnswer` is documented for
exactly this case — *"work this hub ACCEPTED and can no longer finish — a queued turn that came too
late, machinery that can no longer be created, **a gate that can never open**"* — and it carries the
banner and the activation tag by construction. Every fact the old sentence had (the gates recorded at
deferral, who asked for the teardown, the reason they stated) is preserved inside its `what` clause.

**The general rule: an owner-side refusal is COMPOSED by `ShutdownNack`, never written at the call
site.** A hand-written one looks right in review — it names the address, it explains itself, it even
ends with the retry sentence — and it is invisible to every reader that has to act on it.

**And the guard over the producers is an enumeration, which is the same trap one level up.**
`OwnerAnswerRecognitionGuard` calls each producer and runs the real predicate over what it says, but
the producer *list* is written by hand: #3017 was filed because a fifth terminal was missing from
such a list, a sixth was missing at the same time, and the class's own guard passed throughout. This
drain was the seventh, and it stayed unlisted through both. Adding a refusal seam means adding it
there — and moving the floor in `TheProducerSetCoversEverySeam`, so the shrink is a decision rather
than an accident.

Pinned live by `DeferredDeliveryNackedOnDisposeTest.ADiscardedDeferredDelivery_IsRecognisedAsTheOwnersAnswer`,
which discards a real deferred delivery and requires the refusal the sender actually receives to be
recognised as the owner's, by `Address` and by path, with an activation tag on it — with a different
address as the negative control, and the gate name and stated reason as positive controls so a
rewording cannot trade one fact for another.

Since #3498 the re-ask arm mirrors the initial one: `ShuttingDown` is still the promise it rides out;
any other classification faults the stream's subscribers with the classified error and disposes the
keep-alive. A reader of a deleted node learns "gone" milliseconds after the delete, and a stream never
Expand Down
30 changes: 25 additions & 5 deletions src/MeshWeaver.Messaging.Hub/MessageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3345,11 +3345,31 @@ public void Dispose()
+ "this hub went down with work still parked behind its gates.",
Address, delivery.Message.GetType().Name, delivery.Id, delivery.Sender,
gatesAtDeferral, hub.RunLevel, disposal);
NackThroughParent(delivery,
$"Hub {Address} was disposed while {delivery.Message.GetType().Name} "
+ $"(id={delivery.Id}) was still deferred; initialization gates closed at deferral: "
+ $"[{gatesAtDeferral}] — the message was never processed. The teardown was {disposal}. "
+ "The address may reactivate (recycle / restart); retry to get the authoritative answer.");
// 🚨 COMPOSED THROUGH ShutdownNack, never hand-written (#4866). This was the one
// ShuttingDown NACK this service minted without the seam, and the cost was invisible:
// the ENVELOPE said transient (NackThroughParent stamps ErrorType.ShuttingDown) while
// the TEXT said nothing of the kind, and the three classifiers that decide "re-probe
// the fresh activation" vs "take this answer as final" read the TEXT —
// MeshNodeStreamCache.IsTransientOwnerFailure, AreaErrorClassifier.IsTransientHubFailure
// /IsHubRecycling and OrleansRoutingService.ClassifyRoutedFailure. "Hub X was disposed
// while …" matches none of their markers and carries no ShutdownNack.Banner, so
// ShutdownNack.IsAnsweredByOwner answered "the routing layer refused you". Every
// consumer with recovery machinery therefore took a recycling hub's answer as terminal
// and did NOT ask again — so a delivery this hub ACCEPTED and threw away was genuinely
// lost, which is what makes the line above an Error rather than teardown noise. The
// sentence ends "retry to get the authoritative answer" and the readers it says that to
// could not tell it was retryable.
//
// 🚨 ActivationTag() rides here for the same reason it rides on the late-turn site: a
// consumer re-probing this address must tell ONE hub wedged in teardown from a recycle
// storm (#2025), and the per-DELIVERY id right after it varies on every retry against
// the SAME activation.
NackThroughParent(delivery, ShutdownNack.RetryForTheAuthoritativeAnswer(
Address,
$"RunLevel={hub.RunLevel}, {ActivationTag()}",
$"{delivery.Message.GetType().Name} (id={delivery.Id}) was still deferred; "
+ $"initialization gates closed at deferral: [{gatesAtDeferral}] — the message was "
+ $"never processed. The teardown was {disposal}"));
});

// No buffers to Complete — ScheduleNotify drops post-shutdown messages and the
Expand Down
17 changes: 16 additions & 1 deletion test/MeshWeaver.Hosting.Test/OwnerAnswerRecognitionGuard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ private static ObjectDisposedException DisposedScope() =>
"GetDataRequest (id=abc) was accepted before disposal began and its turn came "
+ "too late to process"));

// The SEVENTH terminal, and the one #4866 was filed on: the disposal drain, which answers
// every delivery still parked behind an initialization gate when the hub goes down. It
// hand-wrote its sentence ("Hub X was disposed while …") instead of composing it here, so
// it carried no banner and matched no transient marker — the envelope said ShuttingDown
// while the text read terminal, and the consumers decide on the text. Listed here so the
// shrink that hid a producer for six terminals cannot hide it for a seventh.
yield return ("MessageService disposal drain (ShutdownNack.RetryForTheAuthoritativeAnswer)",
ShutdownNack.RetryForTheAuthoritativeAnswer(
Owner,
$"RunLevel=ShutDown, {ShutdownNack.ActivationMarker}DEADBEEF",
"GetDataRequest (id=abc) was still deferred; initialization gates closed at "
+ "deferral: [DataContextInit,MeshNodeInit] — the message was never processed. "
+ "The teardown was requested by mesh/1 (routed DisposeRequest); why: "
+ "Overlay self-heal"));

// The typed refusal a handler throws, both constructors — they build the sentence
// independently of each other.
yield return ("HubDisposingException(address, what)",
Expand Down Expand Up @@ -173,7 +188,7 @@ public void ARoutingLayerFailure_IsNotTheOwnersAnswer(string message, string bec
[Fact]
public void TheProducerSetCoversEverySeam()
{
OwnerAnswers().Should().HaveCountGreaterThanOrEqualTo(7,
OwnerAnswers().Should().HaveCountGreaterThanOrEqualTo(8,
"one case per owner-side seam, plus the relayed shape — if a producer was genuinely "
+ "consolidated away, lower this number deliberately rather than letting the coverage "
+ "drop unnoticed");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,95 @@ public async Task ATeardownWhoseReasonIsBlank_ReportsItAsUNSTATED_NotAsNothing()
"normalising the reason must not cost the sender the attribution it travels with");
}

private static readonly Address RecognisedAddress = new("recognised", "1");

/// <summary>
/// 🚨 Systemorph/MeshWeaver#4866 — the discard NACK told the sender to <i>"retry to get the
/// authoritative answer"</i> in a sentence no reader could tell was retryable.
///
/// <para><b>The defect.</b> This site hand-wrote its refusal (<c>"Hub X was disposed
/// while …"</c>) instead of composing it through <see cref="ShutdownNack"/>, so it opened with
/// no <see cref="ShutdownNack.Banner"/>. Three independent classifiers decide retryability by
/// MESSAGE TEXT rather than by <see cref="ErrorType"/> —
/// <c>MeshNodeStreamCache.IsTransientOwnerFailure</c>,
/// <c>AreaErrorClassifier.IsTransientHubFailure</c>/<c>IsHubRecycling</c> and
/// <c>OrleansRoutingService.ClassifyRoutedFailure</c> — and none of their markers appears in
/// that sentence. So the transient <see cref="ErrorType.ShuttingDown"/> the test above pins was
/// carried by an envelope the consumers do not read, and every consumer with recovery
/// machinery took the corpse's answer as FINAL: the cache did not re-probe, the layout area did
/// not re-arm, and the accepted work was genuinely lost rather than re-asked. That is the
/// difference between the 105 discards of #4866 being retryable noise and being 105 requests
/// nobody ever answers.</para>
///
/// <para><b>Why this is not the ErrorType assertion again.</b>
/// <see cref="ShutdownNack.IsAnsweredByOwner"/> is deliberately NOT an ErrorType test — the
/// routing layer mints that same classification off the same text, so an ErrorType test answers
/// the question with the routing layer's echo of it. The banner is the only evidence that
/// identifies the speaker, and the drain was the one producer in this service not emitting it.
/// <c>OwnerAnswerRecognitionGuard</c> pins the same contract over every producer's sentence;
/// this test is the LIVE arm, over the delivery the production path actually discards.</para>
/// </summary>
[Fact]
public async Task ADiscardedDeferredDelivery_IsRecognisedAsTheOwnersAnswer()
{
var host = GetHost();

var gated = host.GetHostedHub(
RecognisedAddress,
c => c.WithTypes(typeof(GatedRequest), typeof(GatedResponse))
.WithInitializationGate("recognised-gate-never-opens", _ => false)
.WithHandler<GatedRequest>((h, d) =>
{
h.Post(new GatedResponse(), o => o.ResponseFor(d));
return d.Processed();
}));
gated.Should().NotBeNull();

var response = host
.Observe<GatedResponse>(new GatedRequest(), o => o.WithTarget(RecognisedAddress))
.FirstAsync()
.Await(TestContext.Current.CancellationToken);

await WaitForDeferredBacklog(host);

host.Post(new DisposeRequest { Reason = TheStatedReason },
o => o.WithTarget(RecognisedAddress));

var failure = await Assert.ThrowsAsync<DeliveryFailureException>(() => response);
failure.Failure.Should().NotBeNull();
Output.WriteLine(failure.Failure!.Message ?? "(no message)");

ShutdownNack.IsAnsweredByOwner(failure.Failure.Message, RecognisedAddress).Should().BeTrue(
"the hub that discarded the delivery IS the speaker, and every consumer that decides "
+ "'retry the fresh activation' vs 'take this as final' reads the banner to know it — "
+ "compose the refusal through ShutdownNack rather than widening the predicate");
ShutdownNack.IsAnsweredByOwner(failure.Failure.Message, RecognisedAddress.Path)
.Should().BeTrue(
"a reader that resolved the node by PATH holds nothing else, and the two halves "
+ "must answer identically or recognition is a coin toss on which one is at hand");

ShutdownNack.ExtractActivationTag(failure.Failure.Message).Should().NotBeNullOrEmpty(
"a consumer re-probing this address cannot otherwise tell ONE hub wedged in teardown "
+ "from a recycle storm — those have opposite fixes (#2025), and every ShuttingDown "
+ "NACK minted for a delivery the reader can re-probe carries the activation tag");

// NEGATIVE CONTROL — the predicate must still be able to say no, or the assertions above
// are satisfied by anything that mentions an address.
ShutdownNack.IsAnsweredByOwner(failure.Failure.Message, new Address("recognised", "2"))
.Should().BeFalse(
"a DIFFERENT address is not this owner answering; if this passes the refusal is "
+ "being recognised by something other than its own banner");

// POSITIVE CONTROLS — the facts the refusal already carried are ADDED to, never traded
// against, which is how a rewrite of this sentence reads as a fix while losing evidence.
failure.Failure.Message.Should().Contain("recognised-gate-never-opens",
"the gate that actually held the delivery (#3712/#3789) survives the rewording");
failure.Failure.Message.Should().Contain(TheStatedReason,
"and so does WHY the teardown happened, as its poster stated it");
failure.Failure.ErrorType.Should().Be(ErrorType.ShuttingDown,
"the envelope was never the broken half — it must stay transient too");
}

/// <summary>
/// Polls the public disposal diagnostics (which report <c>deferred=&lt;N&gt;</c> per hub,
/// walking hosted hubs) until something is parked. The gated hub is the only hub in this test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,13 @@ public async Task AfterTheTeardown_FailGateCanNoLongerNameTheCause()
failure.Failure!.Message.Should().NotContain("transient infrastructure fault",
"this is the loss #4261 names: the requester is told the generic disposal sentence "
+ "instead of the cause the retirement exists to carry");
failure.Failure.Message.Should().Contain("was disposed while",
failure.Failure.Message.Should().Contain("was still deferred",
"the generic sentence is what it does get — asserted positively so 'the cause is "
+ "missing' cannot be satisfied by the requester hearing nothing at all");
+ "missing' cannot be satisfied by the requester hearing nothing at all. The clause "
+ "is the drain's own ('… was still deferred; initialization gates closed at "
+ "deferral: …'), not its opening: since #4866 the drain composes through "
+ "ShutdownNack, so the sentence now OPENS with the owner banner every transient "
+ "classifier reads, and the old literal 'was disposed while' is gone");
}

/// <summary>
Expand Down
Loading