Repository navigation
fix(routing): back-pressure is Critical only for a leg past its own bounds — the oldest leg decides the level - #5657
Conversation
…ounds — the oldest leg's age decides the level The routing back-pressure crossing was levelled by queue depth (#5322): deepest >= 1 stayed Critical. Every Critical quoted in the tickets filed after that shipped (#5595, #5616-#5618, #5622, #5631, #5632, #5638) had a deep queue AND a young oldest leg (63 deep at 11 ms, 54 at 81 ms, 43 at 14.8 s) — a producer burst on one stream, sampled at the crossing, inside the leg's own bounds. Depth cannot tell that from a stuck stream; an age past the leg's own bounds can. Permanent level decision (AGENTS.md, log levels): Critical is what the ticketing path files, so it must mean act now. - Level = Critical iff the oldest in-flight leg is older than LegSelfBound, the sum of the leg's own timeouts (resolve + subscriber probe + post = 100 s): its timeouts cannot explain it, so it is a leaked slot or a starved silo. Everything younger is Warning, still logged in full. - A latched episode re-reads its oldest leg on dispatches it makes anyway (rate-limited to 10 s, no timer) and escalates ONCE with "has not drained" when a leg outlives its bounds under it — the one absence an event-driven report can state. - The crossing names the deepest channel (destination [stream]) so a recurring deep queue points at its producer. - Report logic moved out of RoutingGrain into RoutingSaturationReport (testable with a clock). Refs #5118 #5595 #5486 #5638 #5622 #5616 #5617 #5618 #5631 #5632 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical build-documentation and saturation-tracking issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
Updates routing back-pressure severity to use oldest-leg age, adds latched escalation, names the deepest channel, and documents the policy.
Changes:
- Moves reporting into
RoutingSaturationReport. - Updates routing integration and channel diagnostics.
- Adds tests and documentation for severity and escalation behavior.
| File | Summary |
|---|---|
test/MeshWeaver.Hosting.Orleans.Test/RoutingBackpressureShapeTest.cs |
Updates routing saturation snapshot tests. |
test/MeshWeaver.Hosting.Orleans.Test/BackPressureLevelByShapeTest.cs |
Tests severity and escalation behavior. |
src/MeshWeaver.Hosting.Orleans/RoutingSaturationReport.cs |
Implements saturation reporting and escalation logic. |
src/MeshWeaver.Hosting.Orleans/RoutingGrain.cs |
Integrates the saturation reporter. |
src/MeshWeaver.Hosting.Orleans/OrderedRouteDispatcher.cs |
Reports deepest channel identity. |
src/MeshWeaver.Documentation/Data/Architecture/ReadingARoutingSaturationReport.md |
Documents the revised alerting policy. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// routes terminate in milliseconds — and well below the 541-deep queue prod reached. | ||
| /// </summary> | ||
| internal const int SaturationThreshold = 64; | ||
| private RoutingSaturationReport SaturationReport |
There was a problem hiding this comment.
Fixed in 5421fc7: every remaining reference now names RoutingSaturationReport.SaturationThreshold (the three test mentions and the doc). They were <c> text, not <see cref>, so the Release -warnaserror build was already clean (0 Warning(s), 0 Error(s), and CI's Consolidate test results passed) — but the names were stale, so they are updated.
| if (inFlight > SaturationThreshold / 2) return; | ||
| if (Interlocked.Exchange(ref saturationReported, 0) == 0) return; |
There was a problem hiding this comment.
Considered and kept as designed. A single leaked leg after the episode clears is no longer back-pressure (the count is below half the threshold), so this report is not the instrument that should page for it — making it one would turn a saturation report into a general leak detector running outside any episode. It is still named, by two instruments: the NEXT crossing's oldest leg in flight reads that leg first (it is the oldest by construction) and, being past LegSelfBound, makes that crossing Critical; and the silo-stop residual (RoutingQuiescence, #2638/#2833) names any leg that never lands. What is lost is only the case of a leaked leg on a silo that never crosses again, which is by definition not under back-pressure.
| orderedDispatcher.QueueSnapshot, | ||
| quiescence is null ? null : quiescence.OldestInFlight, | ||
| () => (routingPool.CurrentInFlight, routingPool.CurrentlyWaiting)), null); |
There was a problem hiding this comment.
Considered and kept as designed. The population is deliberately RoutingQuiescence's: it is the routing work this SILO holds (forward legs and the NACKs they carry — both dispatched by RoutingGrain, both bounded by the same timeouts), and RoutingGrain is [StatelessWorker(1)], so "another routing activation" is a previous activation of the same per-silo grain whose legs are still this silo's routing work. A NACK or predecessor leg held past LegSelfBound is exactly the leaked-slot / starved-silo condition the Critical exists for, and it is not mislabelled: the printed label is the tracker's own (nack → …, dispatch → …, stream-routed → …), so the line says which kind of work is old.
| private void ReviewLatchedEpisode() | ||
| { | ||
| if (oldestLeg is null) return; | ||
| var now = time.GetUtcNow().UtcDateTime; | ||
| var due = Volatile.Read(ref nextReviewTicks); | ||
| if (now.Ticks < due) return; | ||
| if (Interlocked.CompareExchange(ref nextReviewTicks, (now + LatchedReviewInterval).Ticks, due) != due) return; | ||
|
|
||
| var episode = Volatile.Read(ref saturationEpisode); | ||
| if (Volatile.Read(ref escalatedEpisode) == episode) return; | ||
| var oldest = oldestLeg(); | ||
| if (SaturationLevel(oldest?.Age) != LogLevel.Critical) return; | ||
| if (Interlocked.Exchange(ref escalatedEpisode, episode) == episode) return; |
There was a problem hiding this comment.
Fixed in 5421fc7: after the scan and before escalating, the review revalidates that the latch is still held and the episode number is unchanged; a drained or superseded episode logs nothing. A residual window between that check and the log call remains (the two are not one atomic step, and serializing them would put a lock on the dispatch path); what can slip through it is a Critical about a leg that WAS past its own bounds when read — a true statement about a real leak, attributed to the episode it was read in.
…ating; references follow SaturationThreshold to RoutingSaturationReport Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Why tickets kept filing after #5322
#5322 levelled the
[ROUTE] Routing back-pressurecrossing by queue depth:deepest >= 1stayed Critical. Every Critical quoted in the tickets filed after it shipped had a deep queue and a young oldest leg:(extracted verbatim from #5595, #5616, #5617, #5618, #5622, #5631, #5632, #5638 bodies and recurrence comments). Depth is sampled at the crossing, where it is largely an artefact of the threshold: 63 frames of one stream enqueued within 11 ms is a producer burst, not a stuck stream. Every one of those legs was inside its own bounds.
The rule now (permanent level decision, AGENTS.md → log levels)
LegSelfBound=ResolveTimeout + SubscriberProbeTimeout + StreamPostTimeout(100 s, the longest leg composition). A leg past its own timeouts cannot be a slow destination — it is a leaked slot (the label names it) or a starved silo that never started its timeouts (Portal routing legs starved of ThreadPool threads during install burst (routing saturation gauge, not a routing defect) #5389). Both real defects, both still file.cleared after … msline; it no longer files a ticket for a burst the silo absorbed.Critical… has not drained after N ms: oldest leg …when a leg outlives its bounds under it. Before this, an episode that crossed on young legs and then leaked produced no second line at all.deepest per-channel queue 63 on cache/x [sync/…]), so a recurring deep queue points at the producer.Report logic moved from
RoutingGrainintoRoutingSaturationReport(clock-injectable) so the latch/escalation is testable without a cluster.Verification
MeshWeaver.Hosting.Orleans.TestRelease-warnaserror: 0 Warning(s), 0 Error(s).MeshWeaver.Documentationsame.BackPressureLevelByShapeTest16/16; with the saturation/dispatcher suites 28/28.ALatchedEpisodeWhoseLegOutlivesItsBounds_EscalatesExactlyOncefail (15/16) — the escalation is what the test measures.Doc:
Doc/Architecture/ReadingARoutingSaturationReport— alerting-policy row and "The level rule, and why depth could not carry it".Pairs-with: none — only
internalmembers changed (RoutingGrain.SaturationLevel,RoutingGrain.SaturationThresholdmoved toRoutingSaturationReport).Refs #5118 #5595 #5486 #5638 #5622 #5616 #5617 #5618 #5631 #5632
Production effect only after the portals roll onto an image carrying this commit; no recycle needed beyond the roll (RoutingGrain activations are per-silo and end with the pod).
🤖 Generated with Claude Code