Files
wwtools/mbproxy/tests/Mbproxy.Tests/Proxy/Multiplexing/InFlightByKeyMapTests.cs
T
Joseph Doherty a2dba4bd07 mbproxy: add in-flight read coalescing (Phase 10)
When two or more upstream clients send the same FC03/FC04 read while a
matching request is already in flight on the same PLC's multiplexed
backend socket, attach the late arrivals to the existing InFlightRequest
.InterestedParties list instead of opening a second backend round-trip.
The single backend response fans out to every attached party with each
party's original MBAP TxId restored individually. Zero post-response
staleness — coalescing operates entirely within the in-flight window
(microseconds to ~10 ms typical); the proxy is NOT a cache layer.

Headline mechanism:

- New record struct CoalescingKey(UnitId, Fc, StartAddress, Qty) keys
  the per-PLC InFlightByKeyMap. FC03 and FC04 are separate Modbus
  tables and never share a key; different unit IDs never coalesce;
  writes (FC06/FC16) bypass the coalescing path entirely.
- InFlightByKeyMap uses a simple lock around a Dictionary; atomic
  TryAttachOrCreate either appends a new party to the in-flight
  request's mutable List<InterestedParty> or invokes a factory to
  build a fresh entry. Per-entry MaxParties cap (default 32) bounds
  fan-out cost; past the cap, the next arrival opens a new entry.
- PlcMultiplexer.OnUpstreamFrameAsync takes the coalescing path for
  FC03/FC04 when Mbproxy.Resilience.ReadCoalescing.Enabled. The
  factory closure does the Phase-9 work (allocate TxId, add to
  CorrelationMap); the channel send happens AFTER returning from
  TryAttachOrCreate so the map lock is not held across the async send.
- Response fan-out in RunBackendReaderAsync removes the entry from
  InFlightByKeyMap before iterating InterestedParties, ensuring no
  concurrent attach can mutate the list during iteration.
- Cascade + watchdog paths also drain the key map so a stale entry
  cannot outlive its backend round-trip.

Counter accounting balance (per snapshot): CoalescedHitCount +
CoalescedMissCount equals total FC03 + FC04 requests since startup.
Even with coalescing disabled, every read still bumps Miss so dashboard
math stays balanced.

New surface (additive only):
- src/Mbproxy/Proxy/Multiplexing/CoalescingKey.cs
- src/Mbproxy/Proxy/Multiplexing/InFlightByKeyMap.cs
- src/Mbproxy/Proxy/Multiplexing/CoalescingLogEvents.cs
- ReadCoalescingOptions on ResilienceOptions
- CoalescedHitCount / CoalescedMissCount /
  CoalescedResponseToDeadUpstream counters surfaced on /status.json
  per PLC and as a compact "Coal" cell on the HTML status page.

Phase 9 test patch: TwoUpstreams_ProxyTxIds_AreDistinct_OnTheWire
previously read the same register from both clients (which now
coalesces). Patched to read two different addresses so the test still
proves distinct backend TxIds without violating the coalescing
contract.

Tests added: 24 new (19 unit + 5 E2E):
- CoalescingKeyTests (5)
- InFlightByKeyMapTests (6, includes concurrent stress)
- ReadCoalescingTests (8, stub-backend with deterministic delay)
- ReadCoalescingE2ETests (5, pymodbus simulator; coalescing-active
  during overlap is proven against the stub, not the sim, due to
  pymodbus 3.13's known concurrent-frame bug)

Total: 325 tests passing (282 unit + 43 E2E).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 02:26:06 -04:00

260 lines
10 KiB
C#
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
using System.Net;
using System.Net.Sockets;
using Mbproxy.Proxy.Multiplexing;
using Microsoft.Extensions.Logging.Abstractions;
using Shouldly;
using Xunit;
namespace Mbproxy.Tests.Proxy.Multiplexing;
/// <summary>
/// Unit tests for the Phase-10 <see cref="InFlightByKeyMap"/>. Covers the atomic
/// attach-or-create primitive (load-bearing concurrency invariant), the per-entry
/// max-parties cap (load-shedding safety valve), and concurrent attach correctness.
/// </summary>
[Trait("Category", "Unit")]
public sealed class InFlightByKeyMapTests
{
private static UpstreamPipe MakePipe()
{
// The map only retains references to InterestedParty; it never reads pipe state.
// A connected loopback socket satisfies the constructor contract.
var listener = new TcpListener(IPAddress.Loopback, 0);
listener.Start();
var c = new TcpClient();
c.Connect(IPAddress.Loopback, ((IPEndPoint)listener.LocalEndpoint).Port);
var s = listener.AcceptSocket();
listener.Stop();
return new UpstreamPipe(s, "PLC1", NullLogger.Instance);
}
private static InFlightRequest MakeRequest(InterestedParty party, byte fc = 0x03,
ushort start = 100, ushort qty = 1, byte unit = 1)
{
// The factory uses a mutable List<InterestedParty> so the map can append on attach.
var list = new List<InterestedParty>(capacity: 1) { party };
return new InFlightRequest(
UnitId: unit,
Fc: fc,
StartAddress: start,
Qty: qty,
InterestedParties: list,
SentAtUtc: DateTimeOffset.UtcNow);
}
[Fact]
public async Task TryAttachOrCreate_NewKey_CallsFactory_ReturnsTrue_WasNewTrue()
{
var pipe = MakePipe();
try
{
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
var party = new InterestedParty(pipe, OriginalTxId: 0x1234);
int factoryCalls = 0;
bool ok = map.TryAttachOrCreate(
key, party,
factory: () => { factoryCalls++; return MakeRequest(party); },
maxParties: 32,
out var req, out bool wasNew);
ok.ShouldBeTrue();
wasNew.ShouldBeTrue("a brand-new key must take the create branch");
factoryCalls.ShouldBe(1, "the factory must be called exactly once");
req.ShouldNotBeNull();
req.InterestedParties.Count.ShouldBe(1);
map.Count.ShouldBe(1);
}
finally
{
await pipe.DisposeAsync();
}
}
[Fact]
public async Task TryAttachOrCreate_ExistingKey_AppendsParty_ReturnsTrue_WasNewFalse()
{
var pipeA = MakePipe();
var pipeB = MakePipe();
try
{
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
var partyA = new InterestedParty(pipeA, OriginalTxId: 0x1111);
var partyB = new InterestedParty(pipeB, OriginalTxId: 0x2222);
int factoryCalls = 0;
map.TryAttachOrCreate(key, partyA,
factory: () => { factoryCalls++; return MakeRequest(partyA); },
maxParties: 32, out var first, out bool firstWasNew);
bool ok = map.TryAttachOrCreate(key, partyB,
factory: () => { factoryCalls++; return MakeRequest(partyB); },
maxParties: 32, out var second, out bool secondWasNew);
ok.ShouldBeTrue();
firstWasNew.ShouldBeTrue();
secondWasNew.ShouldBeFalse("the second attach must coalesce onto the first");
factoryCalls.ShouldBe(1, "the factory must only fire on the create branch");
second.ShouldBeSameAs(first, "both attaches must return the same InFlightRequest reference");
second.InterestedParties.Count.ShouldBe(2, "the second party must be appended in place");
second.InterestedParties[0].OriginalTxId.ShouldBe((ushort)0x1111);
second.InterestedParties[1].OriginalTxId.ShouldBe((ushort)0x2222);
}
finally
{
await pipeA.DisposeAsync();
await pipeB.DisposeAsync();
}
}
[Fact]
public async Task TryAttachOrCreate_ExistingKey_AtMaxParties_CreatesFreshEntry_NotAppend()
{
var pipeA = MakePipe();
var pipeB = MakePipe();
var pipeC = MakePipe();
try
{
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
var partyA = new InterestedParty(pipeA, OriginalTxId: 0xAAAA);
var partyB = new InterestedParty(pipeB, OriginalTxId: 0xBBBB);
var partyC = new InterestedParty(pipeC, OriginalTxId: 0xCCCC);
// MaxParties = 2 — first attach creates, second appends, third overflows.
map.TryAttachOrCreate(key, partyA,
factory: () => MakeRequest(partyA), maxParties: 2,
out var first, out _);
map.TryAttachOrCreate(key, partyB,
factory: () => MakeRequest(partyB), maxParties: 2,
out var second, out _);
int factoryCalls = 0;
bool ok = map.TryAttachOrCreate(key, partyC,
factory: () => { factoryCalls++; return MakeRequest(partyC); },
maxParties: 2,
out var third, out bool thirdWasNew);
ok.ShouldBeTrue();
thirdWasNew.ShouldBeTrue("the third attach must overflow into a fresh entry");
factoryCalls.ShouldBe(1, "the factory must fire to create the overflow entry");
third.ShouldNotBeSameAs(first, "the overflow must be a distinct InFlightRequest");
third.InterestedParties.Count.ShouldBe(1, "the overflow entry starts with only its triggering party");
first.InterestedParties.Count.ShouldBe(2, "the original entry stays capped at maxParties");
}
finally
{
await pipeA.DisposeAsync();
await pipeB.DisposeAsync();
await pipeC.DisposeAsync();
}
}
[Fact]
public async Task TryRemove_AfterAttach_AllPartiesPresent_InRetrievedEntry()
{
var pipeA = MakePipe();
var pipeB = MakePipe();
try
{
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
var partyA = new InterestedParty(pipeA, 1);
var partyB = new InterestedParty(pipeB, 2);
map.TryAttachOrCreate(key, partyA, () => MakeRequest(partyA), 32, out _, out _);
map.TryAttachOrCreate(key, partyB, () => MakeRequest(partyB), 32, out _, out _);
bool removed = map.TryRemove(key, out var req);
removed.ShouldBeTrue();
req.InterestedParties.Count.ShouldBe(2, "both attached parties must be present in the removed entry");
map.Count.ShouldBe(0);
}
finally
{
await pipeA.DisposeAsync();
await pipeB.DisposeAsync();
}
}
[Fact]
public void TryRemove_OfMissing_ReturnsFalse()
{
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
map.TryRemove(key, out _).ShouldBeFalse("removing a never-attached key must report false");
}
[Fact]
public async Task Concurrent_AttachOrCreate_From_Two_Threads_NoLostParties_AndNoDuplicateEntries()
{
// 16 tasks × 500 ops each, all racing on the same key. The map must keep exactly
// one entry per key (unlimited MaxParties → no overflow). Each successful attach
// must contribute exactly one party to whatever entry was created/joined.
//
// Each task reuses a single UpstreamPipe across its ops — the map only stores the
// InterestedParty reference; pipe state is irrelevant to the map's invariants.
// Spinning up 100 × 1000 = 100,000 loopback sockets exhausts the test machine's
// ephemeral port pool; we use one pipe per task instead.
const int Tasks = 16;
const int OpsPerTask = 500;
const int MaxParties = int.MaxValue;
var map = new InFlightByKeyMap();
var key = new CoalescingKey(1, 0x03, 100, 1);
var pipes = new List<UpstreamPipe>(Tasks);
for (int i = 0; i < Tasks; i++) pipes.Add(MakePipe());
long attaches = 0;
long creates = 0;
try
{
var work = new Task[Tasks];
var workCt = TestContext.Current.CancellationToken;
for (int t = 0; t < Tasks; t++)
{
var pipe = pipes[t];
work[t] = Task.Run(() =>
{
for (int i = 0; i < OpsPerTask; i++)
{
if (workCt.IsCancellationRequested) return;
var party = new InterestedParty(pipe, (ushort)i);
map.TryAttachOrCreate(
key, party,
factory: () => MakeRequest(party),
maxParties: MaxParties,
out _, out bool wasNew);
if (wasNew) Interlocked.Increment(ref creates);
else Interlocked.Increment(ref attaches);
}
}, workCt);
}
await Task.WhenAll(work);
(creates + attaches).ShouldBe((long)(Tasks * OpsPerTask), "every op must take exactly one branch");
creates.ShouldBe(1, "all ops share the same key with unlimited MaxParties — exactly one create");
// The retained entry must contain every attached party.
bool removed = map.TryRemove(key, out var entry);
removed.ShouldBeTrue();
entry.InterestedParties.Count.ShouldBe(Tasks * OpsPerTask,
"the entry's party list must hold every attached party — no lost parties under race");
map.Count.ShouldBe(0);
}
finally
{
foreach (var p in pipes)
try { await p.DisposeAsync(); } catch { }
}
}
}