dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mesh): per-endpoint deadline + review fixes — failover survives a hanging central

Browse Source 
CRITICAL. The ack deadline was one absolute DateTime computed once for the whole
endpoint walk. Against the characteristic WAN failure — a central that HANGS
(SYN black-hole, frozen host) rather than refusing — endpoint 1 burned the whole
AckTimeoutSeconds and endpoint 2 was then dialled with a deadline already in the
past: instant DeadlineExceeded, no packet leaving the node, the surviving central
never really tried. Failover looked like it ran and did not. It also contradicted
MeshTransportOptions' own documented "applied per endpoint, not per ack". The
deadline is now computed fresh inside the loop; headers stay hoisted (read-only,
sequential walk).

Two test gaps that let that survive are closed: nothing pinned the ackRouter gate
(narrowing it back to ClusterClient-only compiled and passed every test while
dead-ending every Grpc ack to DPS), and the deadline fact's lower bound accepted
any positive timeout at all. Both new pins are mutation-verified.

Docs corrected where they overstated the case: duplicate acks CANNOT mis-seal a
deployment — ConfigPublishCoordinator.HandleAck keys _acks by NodeId behind an
expected-set guard, so a duplicate overwrites its own entry. The single-endpoint
rule is defence in depth and a bound on wasted work, not a correctness
dependency; and the walk is at-least-once on the deadline path, not at-most-once.
This commit is contained in:
Joseph Doherty
2026-08-06 06:15:23 -04:00
parent 308c6aaa0f
commit 667639e6ab
7 changed files with 267 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Communication/GrpcApplyAckSenderTests.cs
+73 -4
View File
@@ -115,8 +115,32 @@ public sealed class GrpcApplyAckSenderTests
options.Headers.GetValue("authorization").ShouldBe($"Bearer {Psk}");
options.Headers.GetValue("x-otopcua-cluster").ShouldBe("SITE-A");
options.Deadline.ShouldNotBeNull();
options.Deadline!.Value.ShouldBeGreaterThan(before);
options.Deadline!.Value.ShouldBeLessThanOrEqualTo(before.AddSeconds(7).AddSeconds(1));
// Bounded from BOTH sides against the configured 7s. A lower bound of `before` alone would
// accept any positive timeout at all — the value could be ignored entirely and the fact would
// still pass.
options.Deadline!.Value.ShouldBeGreaterThan(before.AddSeconds(6));
options.Deadline!.Value.ShouldBeLessThanOrEqualTo(before.AddSeconds(8));
}
[Fact]
public async Task Each_endpoint_gets_a_FRESH_deadline()
{
// The failure this pins is silent and is the characteristic WAN one: a central that HANGS
// rather than refusing. With one absolute deadline hoisted over the whole walk, endpoint 1
// burns all of AckTimeoutSeconds, and endpoint 2 is then dialled with a deadline already in
// the past — it fails instantly as DeadlineExceeded without a packet leaving the node, so the
// surviving central is never really tried and failover is defeated while still LOOKING like it
// ran. Both stamps come from real UtcNow, so a delay on the failing path plus a strict `>` is
// the whole test.
var calls = new List<(ApplyAckMessage Request, CallOptions Options)>();
var factory = (string endpoint) => endpoint.Contains("central-1", StringComparison.Ordinal)
? (CentralCommandService.CentralCommandServiceClient)new SlowThrowingClient(calls, TimeSpan.FromMilliseconds(50))
: new RecordingClient(calls);
await Sender(["http://central-1:4055", "http://central-2:4055"], factory).SendAsync(Ack());
calls.Count.ShouldBe(2);
calls[1].Options.Deadline!.Value.ShouldBeGreaterThan(calls[0].Options.Deadline!.Value);
}
[Fact]
@@ -268,9 +292,28 @@ public sealed class GrpcApplyAckSenderTests
}
[Fact]
public void Dispose_is_idempotent()
public async Task Dispose_releases_the_real_channel_cache_and_is_idempotent()
{
var sender = Sender(["http://central-1:4055"], _ => new RecordingClient([]));
// Built with NO client factory, so the production DefaultClientFactory runs and a real
// GrpcChannel lands in the cache — otherwise Dispose is walking an empty dictionary and this
// test proves nothing about the thing it names. The address is loopback port 1: nothing
// listens, the connection is refused immediately, and no traffic leaves the machine.
var log = new CapturingLogger<GrpcApplyAckSender>();
var sender = new GrpcApplyAckSender(
Options.Create(new MeshTransportOptions
{
Mode = MeshTransportOptions.ModeGrpc,
CentralGrpcEndpoints = ["http://127.0.0.1:1"],
GrpcPsk = Psk,
AckTimeoutSeconds = 5,
}),
Options.Create(new AkkaClusterOptions { Roles = ["driver", "cluster-SITE-A"] }),
log);
await sender.SendAsync(Ack());
// The post must have failed (nothing is listening) — which is what put a channel in the cache.
log.Entries.Any(e => e.Level == LogLevel.Warning).ShouldBeTrue();
sender.Dispose();
Should.NotThrow(sender.Dispose);
@@ -303,6 +346,32 @@ public sealed class GrpcApplyAckSenderTests
=> throw toThrow;
}
/// <summary>
/// A generated client double modelling a HUNG endpoint: it accepts the call, records it, then
/// takes real wall-clock time before failing — the shape a shared deadline cannot survive.
/// </summary>
private sealed class SlowThrowingClient(
List<(ApplyAckMessage Request, CallOptions Options)> calls, TimeSpan delay)
: CentralCommandService.CentralCommandServiceClient
{
public override AsyncUnaryCall<ReportAck> ReportApplyAsync(ApplyAckMessage request, CallOptions options)
{
calls.Add((request, options));
return new AsyncUnaryCall<ReportAck>(
HangThenFail(delay),
Task.FromResult(new Metadata()),
() => new Status(StatusCode.DeadlineExceeded, "hung"),
() => [],
() => { });
}
private static async Task<ReportAck> HangThenFail(TimeSpan delay)
{
await Task.Delay(delay);
throw new RpcException(new Status(StatusCode.DeadlineExceeded, "hung"));
}
}
/// <summary>A generated client double throwing something that is NOT an <see cref="RpcException"/>.</summary>
private sealed class FaultingClient(Exception toThrow) : CentralCommandService.CentralCommandServiceClient
{
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs
+89
View File
@@ -1,13 +1,17 @@
using Akka.Actor;
using Akka.Hosting;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Cluster;
using ZB.MOM.WW.OtOpcUa.Commons.Interfaces;
using ZB.MOM.WW.OtOpcUa.Commons.Messages.Deploy;
using ZB.MOM.WW.OtOpcUa.Commons.Types;
using ZB.MOM.WW.OtOpcUa.Configuration;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Runtime.Communication;
using ZB.MOM.WW.OtOpcUa.Runtime.Historian;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests;
@@ -121,6 +125,91 @@ public sealed class ServiceCollectionExtensionsTests
}
}
/// <summary>
/// Under <c>MeshTransport:Mode = Grpc</c> an ApplyAck produced by the driver host reaches the
/// registered <see cref="IApplyAckSender"/>, and no ClusterClient actor exists
/// (ClusterClient→gRPC migration Task 1B.2).
/// </summary>
/// <remarks>
/// This is the fact that pins the <c>ackRouter</c> gate in <c>WithOtOpcUaRuntimeActors</c>
/// (<c>useClusterClient || useGrpc ? nodeComm : null</c>). Narrowed back to ClusterClient-only
/// that gate still compiles and every unit test still passes, while every ack on a Grpc node
/// goes out on the <c>deployment-acks</c> DPS topic instead — where, in the split-mesh topology
/// Grpc exists to serve, central is not subscribed. The whole transport would be dead and
/// nothing below this level of the stack could tell.
///
/// The dispatch names a deployment that does not exist in the in-memory ConfigDb, so the apply
/// fails and the host emits a Failed ack — deterministic, synchronous, and it exercises the
/// real <c>SendAck</c> path rather than poking the router directly.
/// </remarks>
[Fact]
public async Task Under_Grpc_a_driver_host_ack_reaches_the_ack_sender_and_no_ClusterClient_exists()
{
var sender = new RecordingAckSender();
using var host = BuildRuntimeActorHost(services =>
{
services.AddSingleton<IApplyAckSender>(sender);
services.Configure<MeshTransportOptions>(o =>
{
o.Mode = MeshTransportOptions.ModeGrpc;
o.CentralGrpcEndpoints = ["http://central-1:4055"];
o.GrpcPsk = "rig-key";
});
});
await host.StartAsync();
try
{
var system = host.Services.GetRequiredService<ActorSystem>();
var driverHost = host.Services.GetRequiredService<IRequiredActor<DriverHostActorKey>>();
driverHost.ActorRef.Tell(new DispatchDeployment(
new DeploymentId(Guid.NewGuid()),
new RevisionHash("rev-that-does-not-exist"),
new CorrelationId(Guid.NewGuid())));
// The ack travelled DriverHostActor -> NodeCommunicationActor -> IApplyAckSender. Under a
// ClusterClient-only gate it would have gone to DPS and this never completes.
var deadline = DateTime.UtcNow.AddSeconds(15);
while (sender.Sent.Count == 0 && DateTime.UtcNow < deadline)
await Task.Delay(50);
sender.Sent.Count.ShouldBe(1);
// And the Akka cross-mesh association is genuinely gone, not merely unused.
await Should.ThrowAsync<ActorNotFoundException>(() => system
.ActorSelection($"/user/{ServiceCollectionExtensions.CentralClusterClientName}")
.ResolveOne(TimeSpan.FromMilliseconds(500)));
}
finally
{
await host.StopAsync();
}
}
/// <summary>An <see cref="IApplyAckSender"/> double recording what the node comm actor handed it.</summary>
private sealed class RecordingAckSender : IApplyAckSender
{
private readonly Lock _gate = new();
private readonly List<ApplyAck> _sent = [];
/// <summary>Gets a snapshot of the acks posted through this sender.</summary>
public IReadOnlyList<ApplyAck> Sent
{
get { lock (_gate) return [.. _sent]; }
}
/// <summary>Records <paramref name="ack"/>.</summary>
/// <param name="ack">The ack handed over.</param>
/// <param name="cancellationToken">Unused.</param>
/// <returns>A completed task.</returns>
public Task SendAsync(ApplyAck ack, CancellationToken cancellationToken = default)
{
lock (_gate) _sent.Add(ack);
return Task.CompletedTask;
}
}
/// <summary>Builds a driver-role host that runs <c>WithOtOpcUaRuntimeActors</c>, with optional
/// extra DI registrations applied before <c>AddAkka</c>.</summary>
private static IHost BuildRuntimeActorHost(Action<IServiceCollection>? extra)