fix(historian): QuestDB final-review fix wave (I-1, M-1, M-3, M-5, M-8, M-9)

- I-1: QuestDbConnection caps Npgsql Maximum Pool Size at 24 (DefaultMaxPoolSize) when the connection string does not set it; an explicit value is kept. The effective value is logged once at Information. - M-1: QuestDbTagCatalog.EnsureAsync waits for the tag table's sequenced commits and re-checks before inserting, so it no longer overwrites a provisioner row (unit/description) that is committed but not yet applied. Integration test reproduces the clobber against real QuestDB (falsified). - M-3: the pre-1970 drop Warning is rate-limited per tag (once per 10 min); DroppedPre1970 still counts every sample. - M-5: QuestDbHistorianProvider is IAsyncDisposable/IDisposable and disposes the shared Npgsql pool; Program.cs registers the provider through a factory and every seam resolves it from the container, so DI disposes it at shutdown. - M-8, M-9: docs fixes (HistoryUpdate transaction wording; folded duplicate HistoryUpdate paragraph). Pool-size docs updated for the new default cap. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
2026-10-11 03:35:35 -04:00
parent e177a50046
commit 8d82d049e9
11 changed files with 309 additions and 30 deletions
@@ -92,6 +92,39 @@ public sealed class ProvisioningTests(QuestDbFixture fx)
        (await TagAsync(c, "k")).ShouldBe(((byte)DriverDataType.Float64, "kg", "mass"));
    }

+    /// <summary>
+    ///     Final review M-1: the catalog's first sighting of a tag can run while the provisioner's row is committed but
+    ///     not yet applied (its WAL-visibility window). An unconditional catalog INSERT then lands after it and the
+    ///     dedup key replaces the provisioner's row with one carrying no unit/description. The provisioner here does
+    ///     not wait (VisibilityTimeout 0) and has its own catalog, so every catalog Ensure runs inside that window.
+    /// </summary>
+    [Fact]
+    public async Task A_catalog_Ensure_inside_the_provisioners_visibility_window_keeps_its_metadata()
+    {
+        fx.SkipIfUnavailable();
+        var o = fx.Options(fx.UniquePrefix());
+        await using var c = new QuestDbConnection(o);
+        var b = new QuestDbSchemaBootstrapper(c, o);
+        await b.EnsureAsync(CancellationToken.None);
+        var noWait = new QuestDbWalVisibility(c, TimeSpan.Zero);
+        var provisioner = new QuestDbTagProvisioner(c, b, new QuestDbTagCatalog(c, b, noWait), noWait);
+        var catalog = new QuestDbTagCatalog(c, b, new QuestDbWalVisibility(c, o.VisibilityTimeout));
+
+        const int Tags = 25;
+        for (var i = 0; i < Tags; i++)
+        {
+            var name = $"race{i}";
+            (await provisioner.EnsureTagsAsync([new(name, DriverDataType.Float64, "kg", $"mass {i}")], CancellationToken.None))
+                .Ensured.ShouldBe(1);
+            await catalog.EnsureAsync(name, 1, CancellationToken.None); // the recorder's first value for the tag
+        }
+
+        await new QuestDbWalVisibility(c, o.VisibilityTimeout).WaitAsync(c.TagTable, CancellationToken.None);
+        for (var i = 0; i < Tags; i++)
+            (await TagAsync(c, $"race{i}")).ShouldBe(((byte)DriverDataType.Float64, "kg", $"mass {i}"), $"race{i}");
+        (await CountAsync(c)).ShouldBe(Tags);
+    }
+
    [Fact]
    public async Task A_connection_failure_is_a_Failed_tally_not_an_exception()
    {
@@ -137,4 +137,48 @@ public sealed class QuestDbConnectionTests
        c.EventTable.ShouldBe("p_event");
        c.TagTable.ShouldBe("p_tag");
    }
+
+    // ---- pool size (final review I-1) -------------------------------------------------------------------------------
+
+    private static int PoolOf(string cs)
+    {
+        using var c = new QuestDbConnection(new QuestDbHistorianOptions { ConnectionString = cs });
+        return c.EffectiveMaxPoolSize;
+    }
+
+    [Fact]
+    public void An_unset_Maximum_Pool_Size_is_capped_at_24() =>
+        PoolOf("Host=127.0.0.1;Port=1").ShouldBe(24);
+
+    [Fact]
+    public void The_default_cap_constant_is_24() => QuestDbConnection.DefaultMaxPoolSize.ShouldBe(24);
+
+    [Theory]
+    [InlineData("Host=127.0.0.1;Port=1;Maximum Pool Size=10", 10)]
+    [InlineData("Host=127.0.0.1;Port=1;Maximum Pool Size=80", 80)]
+    [InlineData("Host=127.0.0.1;Port=1;maximum pool size=30", 30)]
+    [InlineData("Host=127.0.0.1;Port=1;MaxPoolSize=80", 80)]
+    public void An_explicit_Maximum_Pool_Size_is_preserved(string cs, int expected) =>
+        PoolOf(cs).ShouldBe(expected);
+
+    [Fact]
+    public void The_effective_pool_size_is_logged_once_at_Information()
+    {
+        var log = new InfoLogger();
+        using var c = new QuestDbConnection(new QuestDbHistorianOptions { ConnectionString = "Host=127.0.0.1;Port=1" }, log);
+        log.Messages.Count.ShouldBe(1);
+        log.Messages[0].ShouldContain("24");
+    }
+
+    private sealed class InfoLogger : Microsoft.Extensions.Logging.ILogger
+    {
+        public List<string> Messages { get; } = [];
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true;
+        public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId, TState state,
+            Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            if (logLevel == Microsoft.Extensions.Logging.LogLevel.Information) Messages.Add(formatter(state, exception));
+        }
+    }
 }
@@ -76,6 +76,23 @@ public sealed class QuestDbHistorianProviderTests
        Field(w, "_wal").ShouldBeSameAs(Field(alarm, "_wal"));
    }

+    [Fact]
+    public async Task Disposing_the_provider_disposes_the_shared_pool_and_is_idempotent()
+    {
+        var sp = new NullSp();
+        var p = Make();
+        var conn = (QuestDbConnection)Field(p.CreateDataSource(sp), "_conn")!;
+        var pool = (Npgsql.NpgsqlDataSource)Field(conn, "_dataSource")!;
+        await p.DisposeAsync();
+        Should.Throw<ObjectDisposedException>(() => pool.CreateConnection().Open());
+        await p.DisposeAsync();
+        p.Dispose();
+        Should.Throw<ObjectDisposedException>(() => p.CreateDataSource(sp));
+    }
+
+    [Fact]
+    public async Task Disposing_an_unused_provider_is_a_no_op() => await Make().DisposeAsync();
+
    [Fact]
    public void The_constructor_rejects_null_options() =>
        Should.Throw<ArgumentNullException>(() => new QuestDbHistorianProvider(null!));
@@ -0,0 +1,71 @@
+using Microsoft.Extensions.Logging;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.QuestDb.Tests;
+
+/// <summary>
+///     The pre-1970 drop Warning is rate-limited per tag (final review M-3) while the counter still counts every dropped
+///     sample. All-pre-1970 batches return before any QuestDB call, so the connection points at a closed port.
+/// </summary>
+public sealed class QuestDbHistorianValueWriterTests
+{
+    private static readonly DateTime FileTimeZero = new(1601, 1, 1, 0, 0, 0, DateTimeKind.Utc);
+
+    private static (QuestDbHistorianValueWriter W, ListLogger Log, Action<TimeSpan> Advance) Make()
+    {
+        var opts = new QuestDbHistorianOptions { ConnectionString = "Host=127.0.0.1;Port=1;Timeout=1" };
+        var conn = new QuestDbConnection(opts);
+        var boot = new QuestDbSchemaBootstrapper(conn, opts);
+        var wal = new QuestDbWalVisibility(conn, opts.VisibilityTimeout);
+        var now = new DateTime(2026, 10, 11, 12, 0, 0, DateTimeKind.Utc);
+        var log = new ListLogger();
+        var w = new QuestDbHistorianValueWriter(conn, new QuestDbTagCatalog(conn, boot, wal), wal, log, () => now);
+        return (w, log, d => now += d);
+    }
+
+    private static HistorizationValue[] Pre1970(int n) =>
+        Enumerable.Range(0, n).Select(i => new HistorizationValue(FileTimeZero.AddSeconds(i), HistorianValue.FromDouble(i), 0)).ToArray();
+
+    [Fact]
+    public async Task The_pre_1970_warning_is_logged_once_per_tag_per_interval_but_every_drop_is_counted()
+    {
+        var (w, log, advance) = Make();
+
+        (await w.WriteLiveValuesAsync("a", Pre1970(2), CancellationToken.None)).ShouldBeTrue();
+        advance(TimeSpan.FromSeconds(2));
+        (await w.WriteLiveValuesAsync("a", Pre1970(3), CancellationToken.None)).ShouldBeTrue();
+        advance(QuestDbHistorianValueWriter.Pre1970WarningInterval - TimeSpan.FromSeconds(3));
+        (await w.WriteLiveValuesAsync("a", Pre1970(1), CancellationToken.None)).ShouldBeTrue();
+
+        log.Warnings.ShouldBe(1);
+        w.DroppedPre1970.ShouldBe(6);
+
+        // A different tag has its own budget.
+        (await w.WriteLiveValuesAsync("b", Pre1970(1), CancellationToken.None)).ShouldBeTrue();
+        log.Warnings.ShouldBe(2);
+
+        // Once the interval has passed since the last Warning for "a", it warns again.
+        advance(TimeSpan.FromSeconds(1));
+        (await w.WriteLiveValuesAsync("a", Pre1970(1), CancellationToken.None)).ShouldBeTrue();
+        log.Warnings.ShouldBe(3);
+        w.DroppedPre1970.ShouldBe(8);
+    }
+
+    [Fact]
+    public void The_interval_is_ten_minutes() =>
+        QuestDbHistorianValueWriter.Pre1970WarningInterval.ShouldBe(TimeSpan.FromMinutes(10));
+
+    private sealed class ListLogger : ILogger
+    {
+        private int _warnings;
+        public int Warnings => Volatile.Read(ref _warnings);
+        public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
+        public bool IsEnabled(LogLevel logLevel) => true;
+        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
+        {
+            if (logLevel == LogLevel.Warning) Interlocked.Increment(ref _warnings);
+        }
+    }
+}