docs(code): close-out comment fixes from the follow-ups review round

This commit is contained in:
Joseph Doherty
2026-08-17 04:32:53 -04:00
parent 57c66416d8
commit 7c5a843ace
5 changed files with 28 additions and 4 deletions
@@ -47,7 +47,9 @@ public static class OtOpcUaTelemetry
/// <c>PollGroupEngine.OverrunCount</c>. <c>Core.Abstractions</c> deliberately carries no metrics
/// dependency, so each poll driver wires this counter to the engine's <c>onOverrun</c> callback;
/// this is the production consumer that surface was always missing. Tagged
/// <c>driver.type</c> (a <c>DriverTypeNames</c> constant) + <c>driver.instance</c>.
/// <see cref="PollOverrunDriverTypeTag"/> (<c>driver_type</c>, a <c>DriverTypeNames</c> constant)
/// + <see cref="PollOverrunDriverInstanceTag"/> (<c>driver_instance</c>) — snake_case, per the
/// convention those two consts document.
/// A non-zero and climbing rate means the device cannot sustain its configured poll rate — the
/// loop is already running flat out.
/// </summary>
@@ -1036,6 +1036,9 @@ public sealed class ScriptedAlarmEngine : IDisposable
Task work;
try
{
// The null coalesce is defensive only: no current caller can return a null Task
// (every factory is an async lambda or returns a started Task). It costs nothing and
// keeps a future non-async factory from NREing the continuation wiring below.
work = factory() ?? Task.CompletedTask;
}
catch (Exception ex)
@@ -1057,6 +1060,9 @@ public sealed class ScriptedAlarmEngine : IDisposable
// so Dispose's WhenAll still observes (and logs) a background failure.
work.ContinueWith(t =>
{
// Removing BEFORE completing the placeholder is safe (not a window where Dispose could
// miss still-running work): this continuation only runs once the REAL task has already
// finished, so anything a concurrent Dispose could still be waiting for is done.
lock (_inFlightLock) { _inFlight.Remove(placeholder.Task); }
if (t.IsFaulted)
{
@@ -57,9 +57,11 @@ public sealed class SessionAclDecisionCache
/// <see cref="ConcurrentDictionary{TKey,TValue}.Count"/>, which takes every lock in the table — a
/// price the store path would pay on every miss to answer a question that only matters at the
/// ceiling. The counter lags — an insert is in its shard's dictionary a moment before it is booked
/// — but it does not drift: every entry is booked exactly once and unbooked exactly once, when the
/// shard holding it is sealed. See <see cref="SealedShard"/> for how that survives an eviction
/// racing an insert.
/// — but it does not drift: every <b>booked</b> entry is unbooked exactly once, when the shard
/// holding it is sealed. (Not every entry is booked: one whose <see cref="TryBookInsert"/> loses
/// the compare-exchange to a seal is never booked at all, because it is already unreachable in a
/// displaced shard.) See <see cref="SealedShard"/> for how that survives an eviction racing an
/// insert.
/// </remarks>
public const int MaxEntries = 100_000;
@@ -209,6 +211,12 @@ public sealed class SessionAclDecisionCache
{
// The outer bound: no single session is over its own ceiling, so there is nothing narrower to
// drop. Re-acquire the shard — ClearAll drops the shard objects themselves.
//
// Deliberately NOT elected: concurrent callers crossing the ceiling together all run ClearAll,
// and that is safe rather than merely tolerated. ClearAll displaces each shard with a per-shard
// TryRemove, so exactly one thread wins each shard and seals it exactly once; the losers just
// re-enumerate an emptying dictionary. The redundancy costs an enumeration, not correctness,
// and a CAS-elected clear would add hot-path state for a branch that is a rare fallback.
ClearAll();
shard = _shards.GetOrAdd(sessionKey, static _ => new Shard());
}
@@ -2119,6 +2119,9 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
//
// Guarded on IsRelational: the timeout is a relational-only concept and the extension throws
// on a non-relational provider (the in-memory provider the actor tests run against).
// ⚠️ Consequently the relational branch itself is UNTESTED in-suite — every actor test runs
// InMemory, so the suite only ever exercises the guard's false side. The 30 s bound is pinned
// by the EF 10.0.7 verification recorded above, not by a test.
if (db.Database.IsRelational())
db.Database.SetCommandTimeout(TimeSpan.FromSeconds(30));
@@ -32,6 +32,11 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests;
/// the same one-shot probe in <see cref="AdminUiSqlServer"/>, so a session probes once.
/// </para>
/// <para>
/// The probe being <b>one-shot</b> bounds what "stays green offline" guarantees: it covers SQL
/// being unreachable <i>before the run</i>. A connectivity blip <i>during</i> a run is never
/// re-probed, so the affected tests surface as <c>SqlException</c> <b>failures</b>, not skips.
/// </para>
/// <para>
/// Every test in the collection shares ONE database (migrating is the expensive part), so tests
/// must isolate themselves by seeding uniquely-named rows, or by clearing the table they measure.
/// xunit serialises a collection's tests, so a clear-then-seed is safe.