perf(mqtt): publish a write batch concurrently, serialised per topic; writes feed health (R06:M-8)

WriteAsync published one awaited QoS 1 round-trip per request, so 50 writes to a
stalled broker cost 50 publish deadlines. Different topics now publish concurrently
(at most 8 in flight); same-topic writes stay in request order. A caller cancel still
fills every slot. A successful publish advances LastSuccessfulRead. MiniBroker now
frames packets properly and records/PUBACKs PUBLISHes.

Refs #744
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:42:50 -04:00
parent 6a51025012
commit c529948745
4 changed files with 272 additions and 29 deletions
@@ -5,7 +5,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests;
/// <summary>
/// The smallest broker that can complete an MQTT 3.1.1 handshake: accept TCP, answer CONNECT with
/// a CONNACK, answer PINGREQ with PINGRESP, and nothing else. That is enough for MQTTnet to report
/// a CONNACK, answer PINGREQ with PINGRESP, SUBSCRIBE with a SUBACK (unless told not to), and record
/// PUBLISHes (PUBACKing QoS 1 only when told to). It never forwards a PUBLISH to a subscriber. That is enough for MQTTnet to report
/// a real connection — which is what makes a real drop (<see cref="DropAll"/>) drive a real
/// <c>DisconnectedAsync</c> and a real reconnect, rather than a test seam standing in for one.
/// </summary>
@@ -107,6 +108,96 @@ internal sealed class MiniBroker : IDisposable
return [.. ack];
}
/// <summary>
/// When true, a QoS 1 PUBLISH is answered with a PUBACK. False (the default) is a broker that takes
/// the PUBLISH and never acknowledges it — the stalled-broker shape a write deadline exists for.
/// </summary>
public bool AckPublish { get; set; }
/// <summary>Every PUBLISH received, in arrival order, as <c>(topic, payload)</c>.</summary>
public System.Collections.Concurrent.ConcurrentQueue<(string Topic, byte[] Payload)> Published { get; } = new();
/// <summary>Handles one complete packet; returns false when the connection must close.</summary>
private async Task<bool> HandlePacketAsync(NetworkStream stream, byte[] packet)
{
var packetType = packet[0] & 0xF0;
if (packetType == 0x10)
{
await stream.WriteAsync(new byte[] { 0x20, 0x02, 0x00, ConnAckReturnCode }, _cts.Token);
// A real broker closes the socket straight after refusing, which is what drives
// MQTTnet's DisconnectedAsync. Mirroring it keeps the fake honest.
return ConnAckReturnCode == ReturnCodeAccepted;
}
if (packetType == 0xC0)
{
await stream.WriteAsync(PingResp, _cts.Token);
}
else if (packetType == 0x80 && AnswerSubscribe && TryBuildSubAck(packet, packet.Length) is { } subAck)
{
await stream.WriteAsync(subAck, _cts.Token);
}
else if (packetType == 0x30)
{
var qos = (packet[0] >> 1) & 0x03;
var at = HeaderLength(packet);
var topicLength = (packet[at] << 8) | packet[at + 1];
var topic = System.Text.Encoding.UTF8.GetString(packet, at + 2, topicLength);
at += 2 + topicLength;
byte idHigh = 0, idLow = 0;
if (qos > 0)
{
idHigh = packet[at];
idLow = packet[at + 1];
at += 2;
}
Published.Enqueue((topic, packet[at..]));
if (qos == 1 && AckPublish)
{
await stream.WriteAsync(new byte[] { 0x40, 0x02, idHigh, idLow }, _cts.Token);
}
}
return true;
}
/// <summary>Length of a packet's fixed header (type byte + variable-length remaining length).</summary>
private static int HeaderLength(byte[] packet)
{
var at = 1;
while ((packet[at] & 0x80) != 0) at++;
return at + 1;
}
/// <summary>Removes one complete packet from the front of <paramref name="pending"/>, if one is there.</summary>
private static bool TryTakePacket(List<byte> pending, out byte[] packet)
{
packet = [];
if (pending.Count < 2) return false;
var at = 1;
var remaining = 0;
var multiplier = 1;
byte digit;
do
{
if (at >= pending.Count) return false;
digit = pending[at++];
remaining += (digit & 0x7F) * multiplier;
multiplier *= 128;
}
while ((digit & 0x80) != 0);
var total = at + remaining;
if (pending.Count < total) return false;
packet = pending.GetRange(0, total).ToArray();
pending.RemoveRange(0, total);
return true;
}
/// <summary>Total TCP connections accepted since construction — counts reconnect attempts.</summary>
public int AcceptedCount => Volatile.Read(ref _acceptedCount);
@@ -178,7 +269,8 @@ internal sealed class MiniBroker : IDisposable
private async Task ServeAsync(TcpClient client)
{
var buffer = new byte[1024];
var buffer = new byte[4096];
var pending = new List<byte>();
try
{
var stream = client.GetStream();
@@ -195,25 +287,22 @@ internal sealed class MiniBroker : IDisposable
continue;
}
var packetType = buffer[0] & 0xF0;
if (packetType == 0x10)
// Frame properly: several packets can share one TCP read (concurrent publishes do), and
// one packet can span two.
pending.AddRange(buffer.AsSpan(0, read).ToArray());
var close = false;
while (TryTakePacket(pending, out var packet))
{
await stream.WriteAsync(new byte[] { 0x20, 0x02, 0x00, ConnAckReturnCode }, _cts.Token);
// A real broker closes the socket straight after refusing, which is what drives
// MQTTnet's DisconnectedAsync. Mirroring it keeps the fake honest.
if (ConnAckReturnCode != ReturnCodeAccepted)
if (!await HandlePacketAsync(stream, packet))
{
close = true;
break;
}
}
else if (packetType == 0xC0)
if (close)
{
await stream.WriteAsync(PingResp, _cts.Token);
}
else if (packetType == 0x80 && AnswerSubscribe && TryBuildSubAck(buffer, read) is { } subAck)
{
await stream.WriteAsync(subAck, _cts.Token);
break;
}
}
}
@@ -288,6 +288,79 @@ public sealed class MqttDriverSessionLifecycleTests
driver.GetHealth().State.ShouldBe(DriverState.Healthy);
}
private static RawTagEntry WritableTag(string rawPath, string writeTopic) => new(
rawPath,
$$"""{"topic":"{{writeTopic}}/state","payloadFormat":"Scalar","dataType":"Float64","writeTopic":"{{writeTopic}}"}""",
WriteIdempotent: false);
/// <summary>
/// <b>R06:M-8.</b> A write batch used to publish strictly serially, one awaited QoS 1 round-trip per
/// request — against a stalled broker, N writes × the publish deadline. Different topics now publish
/// concurrently: all four PUBLISHes reach a broker that never PUBACKs well inside ONE 30 s deadline
/// (serial code would have sent exactly one). Cancelling the batch then fills every slot, so the
/// batch shape holds.
/// </summary>
[Fact]
public async Task AWriteBatch_PublishesDifferentTopicsConcurrently()
{
using var broker = new MiniBroker(); // never PUBACKs
var options = BrokerOptions(broker.Port) with
{
ConnectTimeoutSeconds = 30,
RawTags = [.. Enumerable.Range(0, 4).Select(i => WritableTag($"Plant/Mqtt/dev1/Sp{i}", $"cmd/sp{i}"))],
};
await using var driver = new MqttDriver(options, "d", null);
await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken);
using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
var write = driver.WriteAsync(
[.. Enumerable.Range(0, 4).Select(i => new WriteRequest($"Plant/Mqtt/dev1/Sp{i}", (double)i))],
cts.Token);
await WaitForAsync(() => broker.Published.Count == 4, "all four PUBLISHes to be in flight at once");
write.IsCompleted.ShouldBeFalse("no PUBACK has arrived, so nothing can have completed");
await cts.CancelAsync();
var results = await write.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken);
results.Count.ShouldBe(4);
results.ShouldAllBe(r => r.StatusCode == 0x80050000u); // BadCommunicationError, every slot filled
}
/// <summary>
/// Writes to the SAME topic in one batch stay in request order — last write wins at the device —
/// while the batch as a whole is concurrent. The broker PUBACKs here, so every slot is Good — and
/// those acks advance <c>LastSuccessfulRead</c> (writes feed health).
/// </summary>
[Fact]
public async Task AWriteBatch_KeepsSameTopicWritesInRequestOrder()
{
using var broker = new MiniBroker { AckPublish = true };
var options = BrokerOptions(broker.Port) with
{
RawTags = [WritableTag("Plant/Mqtt/dev1/Sp", "cmd/sp"), WritableTag("Plant/Mqtt/dev1/Other", "cmd/other")],
};
await using var driver = new MqttDriver(options, "d", null);
await driver.InitializeAsync(Json(options), TestContext.Current.CancellationToken);
driver.GetHealth().LastSuccessfulRead.ShouldBeNull("nothing has answered yet");
var results = await driver.WriteAsync(
[
new WriteRequest("Plant/Mqtt/dev1/Sp", 1.0),
new WriteRequest("Plant/Mqtt/dev1/Other", 9.0),
new WriteRequest("Plant/Mqtt/dev1/Sp", 2.0),
new WriteRequest("Plant/Mqtt/dev1/Sp", 3.0),
],
TestContext.Current.CancellationToken);
results.ShouldAllBe(r => r.StatusCode == 0u);
broker.Published.Where(p => p.Topic == "cmd/sp")
.Select(p => System.Text.Encoding.UTF8.GetString(p.Payload))
.ShouldBe(["1", "2", "3"]);
// A PUBACK is the broker answering, so writes feed the evidence clock the comms-loss watchdog reads.
driver.GetHealth().LastSuccessfulRead.ShouldNotBeNull();
}
private static async Task WaitForAsync(Func<bool> condition, string what)
{
var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10);