feat(battle-node): bound InboundTracker with watermark-guarded sliding window
Audit Md11 (part 1 of 2). Replace unbounded HashSet<long> _seen with a WindowSize=256 ring (HashSet + Queue, LRU eviction). The stale-below-window guard (pubSeq <= HighWaterMark - WindowSize) prevents window eviction from re-admitting old seqs as novel — the load-bearing invariant. pubSeq is client-monotonic and SIO retransmit horizons are seconds-scale, so 256 covers realistic retries by a wide margin. HighWaterMark semantics preserved (Gungnir still reports it). Tests: 5 new InboundTrackerTests covering below-window guard, evicted-seq rejection, within-window dedup after eviction, memory bound, and watermark monotonicity. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -4,17 +4,54 @@ namespace SVSim.BattleNode.Reliability;
|
||||
/// Per-session inbound-emit ledger. Dedupes the client's pubSeq so we never dispatch
|
||||
/// a retransmitted emit twice; ack-echo (via SIO callback) is the caller's job.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// State is bounded: the ledger keeps the most recent <see cref="WindowSize"/>
|
||||
/// pubSeqs in an LRU ring. Seqs below <c>HighWaterMark - WindowSize</c> are
|
||||
/// treated as stale-below-window and rejected without recording — this is what
|
||||
/// prevents window eviction from re-admitting an old seq as novel. The pubSeq is
|
||||
/// client-assigned monotonically; the bound is sized well above the realistic
|
||||
/// Socket.IO retransmit horizon, so legitimate retransmits always fall inside.
|
||||
/// </remarks>
|
||||
public sealed class InboundTracker
|
||||
{
|
||||
private readonly HashSet<long> _seen = new();
|
||||
/// <summary>Sliding-window size. Anything below <c>HighWaterMark - WindowSize</c> is dropped.</summary>
|
||||
public const int WindowSize = 256;
|
||||
|
||||
private readonly HashSet<long> _seen = new(WindowSize);
|
||||
private readonly Queue<long> _order = new(WindowSize);
|
||||
|
||||
/// <summary>Highest pubSeq observed so far. Reported via Gungnir for diagnostics.</summary>
|
||||
public long HighWaterMark { get; private set; }
|
||||
|
||||
/// <summary>Record an incoming pubSeq. Returns true if the caller should dispatch the envelope, false on duplicate.</summary>
|
||||
/// <summary>Record an incoming pubSeq. Returns true if the caller should dispatch the envelope, false on duplicate or stale-below-window.</summary>
|
||||
public bool Observe(long pubSeq)
|
||||
{
|
||||
if (pubSeq > HighWaterMark) HighWaterMark = pubSeq;
|
||||
return _seen.Add(pubSeq);
|
||||
// Stale-below-window guard. Required AFTER HighWaterMark is past the window,
|
||||
// otherwise an evicted ring entry would re-admit as novel.
|
||||
if (HighWaterMark > 0 && pubSeq <= HighWaterMark - WindowSize)
|
||||
return false;
|
||||
|
||||
if (pubSeq > HighWaterMark)
|
||||
{
|
||||
HighWaterMark = pubSeq;
|
||||
Record(pubSeq);
|
||||
return true;
|
||||
}
|
||||
|
||||
if (_seen.Contains(pubSeq))
|
||||
return false;
|
||||
Record(pubSeq);
|
||||
return true;
|
||||
}
|
||||
|
||||
private void Record(long pubSeq)
|
||||
{
|
||||
if (_order.Count >= WindowSize)
|
||||
{
|
||||
var evicted = _order.Dequeue();
|
||||
_seen.Remove(evicted);
|
||||
}
|
||||
_order.Enqueue(pubSeq);
|
||||
_seen.Add(pubSeq);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,4 +38,79 @@ public class InboundTrackerTests
|
||||
t.Observe(5);
|
||||
Assert.That(t.HighWaterMark, Is.EqualTo(5));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Observe_returns_false_for_pubSeq_below_high_water_minus_window()
|
||||
{
|
||||
// Drive watermark past WindowSize, then re-observe seq=1.
|
||||
// The watermark guard rejects it without consulting ring membership.
|
||||
var t = new InboundTracker();
|
||||
for (long i = 1; i <= InboundTracker.WindowSize + 10; i++) t.Observe(i);
|
||||
|
||||
Assert.That(t.Observe(pubSeq: 1), Is.False);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Evicted_seq_stays_rejected_by_watermark()
|
||||
{
|
||||
// Load-bearing invariant: even after seq=1 is evicted from the ring,
|
||||
// re-observing it must still return false because the watermark guard
|
||||
// catches `1 <= 257 - 256`. Otherwise window eviction would admit
|
||||
// an old seq as novel.
|
||||
var t = new InboundTracker();
|
||||
for (long i = 1; i <= InboundTracker.WindowSize; i++) t.Observe(i); // ring: {1..256}
|
||||
t.Observe(InboundTracker.WindowSize + 1); // evicts 1; watermark=257
|
||||
|
||||
Assert.That(t.Observe(pubSeq: 1), Is.False);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Within_window_dedup_still_fires_after_eviction()
|
||||
{
|
||||
var t = new InboundTracker();
|
||||
for (long i = 1; i <= InboundTracker.WindowSize; i++) t.Observe(i); // ring: {1..256}, watermark=256
|
||||
t.Observe(pubSeq: 300); // ring: {45..256, 300}, watermark=300
|
||||
|
||||
// 200 is in the ring → dedup catches it.
|
||||
Assert.That(t.Observe(pubSeq: 200), Is.False, "Within-window dedup still works after eviction.");
|
||||
|
||||
// 44 was evicted, but is below HighWaterMark - WindowSize = 44 → stale guard.
|
||||
Assert.That(t.Observe(pubSeq: 44), Is.False, "Below-window guard rejects evicted seqs.");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Memory_bound_stays_constant_after_many_observes()
|
||||
{
|
||||
// The dedup structures must not grow without bound. Internals are accessible
|
||||
// via the existing InternalsVisibleTo("SVSim.UnitTests"). Use reflection so
|
||||
// the test doesn't force the field-access modifier choice in production.
|
||||
var t = new InboundTracker();
|
||||
for (long i = 1; i <= 10_000; i++) t.Observe(i);
|
||||
|
||||
var seenField = typeof(InboundTracker).GetField("_seen",
|
||||
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
|
||||
var orderField = typeof(InboundTracker).GetField("_order",
|
||||
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
|
||||
Assert.That(seenField, Is.Not.Null);
|
||||
Assert.That(orderField, Is.Not.Null);
|
||||
|
||||
var seen = (System.Collections.Generic.IReadOnlyCollection<long>)seenField!.GetValue(t)!;
|
||||
var order = (System.Collections.Generic.IReadOnlyCollection<long>)orderField!.GetValue(t)!;
|
||||
Assert.That(seen.Count, Is.LessThanOrEqualTo(InboundTracker.WindowSize),
|
||||
$"_seen must stay <= WindowSize ({InboundTracker.WindowSize}).");
|
||||
Assert.That(order.Count, Is.LessThanOrEqualTo(InboundTracker.WindowSize),
|
||||
$"_order must stay <= WindowSize ({InboundTracker.WindowSize}).");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void HighWaterMark_is_not_moved_backward_by_eviction()
|
||||
{
|
||||
var t = new InboundTracker();
|
||||
for (long i = 1; i <= InboundTracker.WindowSize; i++) t.Observe(i);
|
||||
var beforeEviction = t.HighWaterMark;
|
||||
t.Observe(InboundTracker.WindowSize + 1); // eviction fires here
|
||||
|
||||
Assert.That(t.HighWaterMark, Is.GreaterThanOrEqualTo(beforeEviction));
|
||||
Assert.That(t.HighWaterMark, Is.EqualTo(InboundTracker.WindowSize + 1));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user