Skip to content

MNTR: deterministic fixes for the Artery-lane spec failures - #8475

Merged
Aaronontheweb merged 9 commits into
devfrom
test/artery-lane-deflake-batch
Aug 26, 2026
Merged

MNTR: deterministic fixes for the Artery-lane spec failures#8475
Aaronontheweb merged 9 commits into
devfrom
test/artery-lane-deflake-batch

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Member

Batch of test-only deterministic fixes for the multi-node specs that fail on the non-blocking Artery lanes (#8373, build 130443). Five commits, three areas, no product code.

Gate specs pinned to classic (RemoteGatePiercingSpec, RemoteNodeRestartGateSpec): both trigger via ForceDisassociateExplicitly, which the artery transport does not support (returns false, unchecked), and assert on the "address is now gated" warning, which only classic's ReliableDeliverySupervisor emits. Artery has no gating concept, so under the artery lane these specs time out on a log line that cannot exist. akka.remote.artery.enabled = off in CommonConfig keeps them meaningful on every lane; CommonConfig outranks the env-injected artery tier (verified against the config fold in MultiNodeSpec).

StartNewSystemAsync made transport-aware: the shared restart helper emitted only dot-netty host/port, so under the artery lane a restarted node bound canonical.port = 0 and came back on a port nobody could reach. It now emits both transports' keys; each transport ignores the other's. RemoteNodeRestartDeathWatchSpec already carried this exact workaround inline, which corroborates the mechanism.

UnreachableNodeJoinsAgainSpec made deterministic (evidence: node logs from build 130443, where the master demonstrably sent EndAck and lost it to its own transport teardown - see #8474):

  • termination of the old system is now asserted before rebinding its port
  • the EndActor handshake resolves the master's endpoint first (ResolveOne proves and warms the reverse lane), with the ack bounded at 30s so the victim outlasts the master's 20s window
  • unreachable-member assertions snapshot once instead of re-reading live state across an await gap
  • the fresh-system convergence check runs on a probe bound to the live system instead of the disposed TestKit
  • the heartbeat-warmup Thread.Sleep is replaced with an assertion on FailureDetector.IsMonitoring
  • whole spec converted to async TestKit APIs (separate mechanical commit)

ClusterSingletonManagerLeaveSpec deadline inversion removed: the harness runs gossip and leader actions at 200ms, but the singleton under test ran at production defaults - a ~12s worst-case hand-over ladder against the spec's own 10s expects and coordinated shutdown's 10s cluster-exiting phase. The singleton now matches the harness tempo (200ms); no timeout was raised. The separately tracked gossip resurrection (#8467) also affects this spec and is deliberately not addressed here.

…ransport

Both specs trigger a gate with ForceDisassociateExplicitly and assert
on the classic "address is now gated" warning log. Artery ignores
that management command and has no gating concept, so these specs
only make sense under classic remoting.

Add akka.remote.artery.enabled = off to each spec's CommonConfig.
CommonConfig outranks the env-var-driven artery fallback tier in
MultiNodeConfig.Config, so this pins the specs to classic regardless
of AKKA_MNTR_TRANSPORT.
StartNewSystemAsync only emitted dot-netty tcp host/port when
rebuilding a node's ActorSystem. Under the Artery lane the inherited
config's akka.remote.artery.canonical.port = 0 fallback then won, so
the fresh system bound a random port instead of the address other
nodes expected.

Emit akka.remote.artery.canonical.hostname/port alongside the
dot-netty keys. Each transport ignores the other's keys, so emitting
both is safe.
The repo rule is to use the async TestKit methods everywhere. This spec still
used the blocking variants throughout.

Swap ExpectMsg/AwaitAssert/Within/EnterBarrier/RunOn and the cluster helpers for
their async twins, and turn the test body and its four phase methods into
async Task. TestConductor.Blackhole/PassThrough/Shutdown are now awaited instead
of blocked on with .Wait().

Behavior is unchanged. The Thread.Sleep and the discarded
Sys.WhenTerminated.Wait stay as-is here; a follow-up commit replaces them.
Node 4 [fourth] timed out after 15s waiting for EndActor.EndAck on the Linux
artery lane. The master's own log shows it received End, replied EndAck, passed
its assertion and then tore down its transport ~150ms later - the ack died in
the outbound stage. Five separate weak spots let that surface as a bare timeout.

Replace the 2s Thread.Sleep with an AwaitAssert on
Cluster.FailureDetector.IsMonitoring for every peer. The sleep stood in for
"the failure detector has seen a heartbeat from everyone"; now we wait for that
condition instead of guessing how long it takes.

Assert the result of Sys.WhenTerminated. It was discarded, and the spec then
rebound a fresh system to the same host:port. A failed termination now fails by
name instead of turning into a same-port bind race.

Snapshot the unreachable set once per assertion block. Both phases re-read the
live ClusterView several times, once across an AwaitSeenSameState gap, so the
assertions could describe different views of a moving cluster.

Drive the fresh-system membership check from a probe bound to that system. The
three chained AwaitAsserts ran against the spec's own TestKit, whose Sys had
just been terminated, so the await loop had no live scheduler. They now form one
snapshot assertion with an explicit bound matching AwaitMembersUp's own 25s.

Resolve the master's end actor before starting the handshake. The Identify round
trip confirms the association to the just-rebound address carries traffic in
both directions, which is exactly what the EndAck needs, and names the failure
when it does not. The ack wait gets an explicit 30s: the master waits 20s for
End, so the victim has to outlast it. The old code inherited the 15s
single-expect default - the smallest budget in the spec, guarding the step that
needs the most time.
The MNTR harness runs gossip and leader actions at 200ms, but the
singleton under test still used the 1s production default for
hand-over-retry-interval. That gave the hand-over ladder up to 12
retries, about 12s, to give up. This exceeded the spec's own 10s
expects and coordinated shutdown's 10s cluster-exiting phase,
turning the asserted stop-before-MemberRemoved order into a race
instead of a guarantee.

Set hand-over-retry-interval and singleton-identification-interval
to 200ms in this spec's CommonConfig to match the harness tempo.
No timeout was raised.
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 26, 2026 02:06
The hand-over retry count is derived from min-number-of-hand-over-retries,
so setting hand-over-retry-interval to 200ms also shrank the manager's
give-up patience from 12s to 2.4s. The artery lane caught the consequence
on the first combined run: the leaving manager threw
ClusterSingletonManagerIsStuckException before the new oldest took over,
died without sending its termination message, and the spec received
postStop where it expected stop.

Raise the retry count so give-up lands at 5s in 200ms ticks: above the
observed take-over latency on the lane, below the spec's own 10s expects.
@Aaronontheweb
Aaronontheweb merged commit e11bb6f into dev Aug 26, 2026
13 of 15 checks passed
@Aaronontheweb
Aaronontheweb deleted the test/artery-lane-deflake-batch branch August 26, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant