Artery: honour the reconnect backoff, return the held control element on stage stop, publish the inbound context before bind, host the materializer under /system

[Aaronontheweb]

Stacked on #8570.

What changes

Three small defects in Artery's outbound path, each with a test that fails on the old code and passes on the new.

• The reconnect backoff is honoured. ScheduleOutboundRestart reset the stream's materialize gate before scheduling the backoff, so any control enqueue during the wait re-materialized the stream at once. The reset now happens inside the post-backoff callback, for the control, ordinary, and large streams. Measured on build 131174 before the fix: five control reconnects per second against a configured one, attempts 10 to 230 in 42.7 s.
• A held control element is returned on stage stop. OutboundHandshakeStage takes one element out of its channel while it gates on the handshake. When the stage stopped, that element was lost. It could be a HandshakeReq, an ArteryHeartbeat, or a remote-deploy message. PostStop now hands it back through a new internal ReturnUndelivered on the outbound context, which re-enqueues it into the right channel or publishes Dropped if the channel is full. The ordinary-lane path now gives each lane its own context so the element goes back to its own lane.
• The inbound context is published as soon as the bound port is known, before the default addresses, the control subscription, and the startup log line. A test-only hook on the transport setup reports whether the context was set at that point. The remaining window inside the TCP stage itself is documented in a comment.

No public API changes: the new members are on internal types, and the approval files are unchanged. These are bug fixes that restore intended behavior, so they are not recorded in the 1.6 breaking-changes ledger.

Why

How it was checked

Akka.Remote builds with warnings as errors. Akka.Remote.Tests, Artery filter: 278 passed. Akka.API.Tests: 18 passed, no approval diff. Each new test was run with its fix reverted by hand and failed, then with the fix restored and passed: ArteryOutboundRestartBackoffSpec (three of four facts fail without the fix), the new case in ArteryHandshakeSpec, and ArteryInboundContextPublishSpec.

Second commit: the materializer moves under /system, and LaneWriteBatchStage reports held bytes on stop

ArteryRemoting.Start materialized every stream on a materializer whose stream supervisor is a user-guardian actor. The system stops the user guardian before the remoting terminator runs, so on every graceful terminate every Artery stream aborted with AbruptStageTerminationException before the shutdown flush, and flush-wait-on-shutdown never applied. The supervisor is now a system actor, built from public Akka.Streams API. LaneWriteBatchStage reports the bytes of a retained batch it drops on stop through a callback that publishes Dropped. Unlike OutboundHandshakeStage it cannot hand the data back, since it sits after encode and the lane merge with no single envelope left, so it makes the loss visible rather than recoverable. That stage is only materialized at outbound-lanes > 1, so this has no effect at the shipping default of 1.

This is the product half of the RemoteNodeRestartDeathWatchSpec analysis; the test half is #8557. Pekko hosts the transport's materializer under its system materializer for the same reason, with two differences that remain: Pekko builds a separate control materializer, and it reads the materializer settings from pekko.remote.artery.advanced.materializer, a key Akka.NET does not have, so Artery's streams here are wired to the global akka.stream.materializer settings.

What a user sees on shutdown after this change: a graceful terminate with a peer that cannot be reached now waits up to flush-wait-on-shutdown, 2 s by default, where before the streams had already aborted and the wait cost nothing. Abort() pays that flush as well. Inbound streams also survive the user-guardian teardown, so a terminating node drains and closes its accepted connections and dead-letters late arrivals instead of resetting them. The unbind wait in Shutdown() is bounded by the TCP stage's own timer; #8570 removes the idle case, and a connection accepted but not yet materialized still holds it for up to subscription-timeout.

New spec ArteryGracefulTerminateFlushSpec: with the old placement, terminating a system with an open outbound stream logs the abrupt-termination warning; with the new one it logs none, because the shutdown's own graceful completion runs first. Verified failing with the materializer change reverted and passing with it restored. The LaneWriteBatchStage test was verified the same way. Artery filter: 280 passed. Akka.API.Tests: 18 passed, no approval diff; both changed classes are internal. Ledger: the existing fix/artery-shutdown-flush row already promised this flush and has not been true on dev until now; the fifth commit amends that row rather than adding one.

One pre-existing quirk noted, not changed: an inbound Artery connection that never decoded its stream-type preamble can leave a graph interpreter that only clears on the forced-shutdown timeout, independent of materializer placement.

Third commit: what the adversarial review found

• The first commit introduced a wedge. Once the gate reset moved into the post-backoff callbacks, the ordinary and large callbacks returned early whenever the association said not to restart. Quarantine is one such reason, and it is not permanent. The gate then stayed latched with no stream behind it and nothing left to reset it, so every later send to that peer queued into a channel nothing drained, including the selection messages that must cross a quarantine and the first sends to a new incarnation. The pre-schedule check already released the gate in that case; both callbacks now do the same. A new ArteryOutboundRestartQuarantineGateSpec quarantines the peer during the backoff window and asserts the gate reopens and a later send materializes, for the ordinary and large streams. Both facts fail on the second commit and pass now.
• The inbound-context test could not fail. The hook fired on the statement right after the assignment it checked. It now fires after the addresses are published, so it reports whether the context was set before them. Swapping the assignments by hand makes ArteryInboundContextPublishSpec fail; restoring them makes it pass.
• Sequenced system messages are no longer returned. A held element on the control stream can already be a SystemMessageEnvelope, which the delivery stage's resend buffer retransmits until acked. Returning it only handed a duplicate to the next materialization. The doc comment said as much and the code now agrees.
• Tail ordering is documented. The channel has no head insert, so a returned element lands behind everything enqueued while it was held. For an unwrapped DaemonMsgCreate that can reopen the create-before-watch ordering for that one element; the inbound side tolerates the race, and the comment on ReturnUndelivered says so.
• The fourth backoff fact now discriminates. It counted nothing before and passed on the old code too; it now counts materialize callbacks and its name says what it proves.
• The production return path is exercised. A new ArteryReturnUndeliveredOutboundElementSpec goes through ReturnUndeliveredOutboundElement itself, for lane routing and for the Dropped event when the channel is full, instead of a hand-wired delegate.

Artery filter: 284 passed, run twice, before and after the by-hand reversions. Akka.API.Tests: 18 passed, no approval diff. No ledger entry.

What this PR's own CI run showed, and the restack on #8570

Build 131333: the Windows unit-test job was cancelled at the 20-minute per-project cap inside Akka.Remote.Tests, and the Linux run of the same assembly took 19 min 46 s against 8 min on other PRs. Per-test timings showed the new tests adding about a minute and existing Artery tests slowing by 12 to 54 s each, all in the peer-restart and graceful-shutdown family. A bisect across the three commits put all of it on the second one, a flat 5 s per Artery ActorSystem termination.

The cause is not in this PR. Hosting the materializer under /system means Artery's listener now performs a real graceful unbind at shutdown instead of dying with /user first, and Akka.Streams' TCP unbind has a latent bug: its counter of connections awaiting initialization is seeded at -1, so the idle fast path never fires and every Unbind() waits the 5 s subscription timeout. That is fixed in #8570, one token plus a regression test, and this PR is now stacked on it. With the fix, the Artery test namespace runs in 2 min 10 s instead of 13 min 25 s, all 284 passing, and the three-cycle create-and-terminate test takes 395 ms instead of 15 s. The three commits here are unchanged in content, verified by patch-id after the rebase.

Fourth commit

The fourth backoff fact still passed on the buggy code: the materialize-once gate runs the winning caller's callback, and after the buggy synchronous reset the winner is the enqueue path's own lambda, so a callback counter cannot tell the two worlds apart. The fact now asserts on the association's restart bookkeeping, which latches the instant the gate is reset. Verified by moving the reset back in front of the backoff by hand: the fact fails, and passes again with the product code restored.

Fifth commit: what the review of the second commit found

The second commit had also widened the outbound materialize guards to refuse whenever the system was "terminating", and that predicate included the coordinated-shutdown reason, which is set before phase one runs. From the moment a graceful shutdown or a cluster leave began, an Artery node would not materialize a new outbound stream, the control stream and reconnects after backoff included, for the whole of the sharding hand-off, leave, and exiting phases, and the refused stream's gate stayed latched with no restart armed. Pekko checks only the transport's own shutdown flag.

• The guards now check the transport: its shutdown flag, the materializer, the system-hosted stream supervisor, an aborting system, or a completed termination. The coordinated-shutdown reason is out. A refused materialization resets the gate the way the existing failure paths do, so a later send can still materialize.
• A new ArteryOutboundMaterializeDuringCoordinatedShutdownSpec holds coordinated shutdown at its first phase and asserts a fresh outbound stream still materializes. It fails on the fourth commit and passes on this one.
• ArteryShutdownSystemMessageAckRaceSpec had become vacuous, because the broad guard returned before the race it exists to cover. It now stops the system-hosted supervisor directly and reaches the race again; with its guarding catch neutered by hand it fails five of five runs, restored it passes.
• Two comments still placed the stream supervisor under /user; corrected. The graceful-terminate spec's zero-count filter takes an explicit 3 s window. LaneWriteBatchStage's class doc says it is unreachable at the default lane count, and its append-failure path now folds into the single Dropped report on stop instead of discarding silently.
• The ledger row for the Artery shutdown flush is amended in place to say it takes effect with this PR and to name the two consequences it did not cover: no abrupt-termination warning on graceful terminate, and peers see a drained close instead of a reset.

Artery filter after this commit: 285 passed in 2 min 7 s. Akka.API.Tests: 18 passed, no approval diff.

Sixth commit: the maintainer's review nits

The stream supervisor is now a named top-level system actor, /system/artery-stream-supervisor, instead of a randomly named one. It stays a StreamSupervisor: the materializer attaches interpreter actors straight to its cell, but it still needs that actor's Materialize protocol for a supervisor that has not started yet, and its PostStop is what sets the flag behind IsShutdown. The explicit top-level config injection is gone, since ActorMaterializerSettings.Create already performs it. The three terminating predicates are plain methods with early returns; the null-materializer clause is gone, because the materializer is assigned as the first statement of Start() and nothing can materialize before that, so a null materializer means not started rather than terminating; and the two exception filters that spelled the clauses out inline call the predicate instead. The batch-stage comment says what the callback does in two sentences. Artery filter: 285 passed in 2 min 8 s. Approvals unchanged.

[Aaronontheweb]

Largely LGTM - with some nitpicks

[Aaronontheweb]

Technically not necessary.

[Aaronontheweb]

It's preferable that we have a static, named actor here serving as the materializer for all artery streams for the reasons elicited in this PR, but I wonder a wee bit about starting a StreamSupervisor directly and making that the materializer context, since I thought those actors were typically created downstream of the materialization root?

I suppose it doesn't necessarily matter, but I f find that a little strange. The idea in principle is still right though. We need an actor to act as the owner of all the Akka.Remote streams used for Artery and it has to be a top-level actor under the /system hierarchy so we don't accidentally terminate in-flight streams before the /user hierarchy has had a chance to complete terminating.

[Aaronontheweb]

LGTM

[Aaronontheweb]

LGTM

[Aaronontheweb]

Logs un-delivered outbound messages when the transport is terminated

[Aaronontheweb]

Can't the _materializer be null during startup too?

Overall I think this needs a bit of simplification - also, doesn't it overlap quite a bit with private bool IsActorSystemTerminating() ?