Stop query_stats from recording a false zero CPU when only the CPU delta is unknowable (#4394)

Darling/Darling.Tests/QueryStatsUnknownCpuLiveTests.cs

@@ -0,0 +1,145 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using Npgsql;
+using PerformanceMonitor.Collectors;
+using PerformanceMonitor.Darling.Storage;
+using PerformanceMonitor.Darling.Viewer;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4394 live pin: a <c>query_stats</c> row with <c>delta_worker_time = NULL</c> and a real
+/// <c>sample_interval_seconds</c> keeps its executions and duration through the hourly rollup, and the
+/// Viewer's <c>worker_time_per_second</c> math tolerates the NULL rather than erroring.
+///
+/// <para><b>#1776 own-store</b> — mints a scratch database, so it is deliberately NOT in the
+/// <c>live-postgres</c> collection.</para>
+/// </summary>
+public sealed class QueryStatsUnknownCpuLiveTests
+{
+    private const int ServerId = -439412;
+    private const string ServerName = "a4394-unknown-cpu";
+    private const string Db = "UnknownCpuDb";
+
+    private static readonly DateTime WindowStart = new(2026, 1, 5, 0, 0, 0, DateTimeKind.Unspecified);
+
+    [Fact]
+    public async Task AnUnknownCpuRow_KeepsItsExecutionsAndDuration_ThroughTheHourlyRollup_AndTheViewerMathTolerates_TheNull()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString),
+            "Set DARLING_TEST_PG to a Postgres connection string (with TimescaleDB installed) to run the live #4394 rollup test.");
+
+        var ct = TestContext.Current.CancellationToken;
+
+        await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct);
+        await using var connection = new NpgsqlConnection(scratch.ConnectionString);
+        await connection.OpenAsync(ct);
+        await PgMigrations.MigrateAsync(connection, ct);
+
+        var timescaleEnabled = await TimescaleSupport.TryEnableAsync(connection, null, ct);
+        Assert.SkipWhen(!timescaleEnabled, "The live #4394 rollup test needs TimescaleDB.");
+        await TimescaleSupport.ConvertToHypertablesAsync(connection, null, ct);
+
+        await using (var stop = new NpgsqlCommand("SELECT _timescaledb_functions.stop_background_workers()", connection))
+        {
+            await stop.ExecuteNonQueryAsync(ct);
+        }
+
+        await DarlingMcpTestData.RegisterServerAsync(connection, ServerId, ServerName, ct);
+        await TimescaleSupport.EnsureContinuousAggregatesAsync(connection, null, ct);
+
+        var windowEnd = WindowStart.AddDays(1);
+        var bodySucceeded = false;
+        try
+        {
+            /* One normal row: real CPU, real interval. One #4394 row: NULL CPU (unknowable), but a real
+               interval and real executions/elapsed — the shape #4394's fix produces for the same query
+               hash on a later collection pass whose worker counter reset while exec/elapsed did not. */
+            await PlantAsync(connection, ct, WindowStart.AddHours(1), workerTime: 400_000L, elapsedTime: 500_000L, executions: 3L, intervalSeconds: 60);
+            await PlantAsync(connection, ct, WindowStart.AddHours(2), workerTime: null, elapsedTime: 5_000_000L, executions: 3L, intervalSeconds: 60);
+
+            await RefreshAsync(connection, TimescaleSupport.QueryStatsIntervalHourlyView, WindowStart, windowEnd.AddHours(1), ct);
+
+            await using var rollup = new NpgsqlCommand(
+                "SELECT sum(execution_count_sum), sum(elapsed_time_sum), sum(worker_time_sum) " +
+                "FROM collect.query_stats_interval_hourly WHERE server_id = $1", connection);
+            rollup.Parameters.AddWithValue(ServerId);
+            await using var reader = await rollup.ExecuteReaderAsync(ct);
+            Assert.True(await reader.ReadAsync(ct));
+
+            /* The NULL-CPU row's executions (3) and elapsed (5,000,000) are INCLUDED — the hourly view's
+               filter is on sample_interval_seconds, not on delta_worker_time, and sum()/NULL simply skips
+               the NULL. Worker sum equals the normal row's 400,000 ALONE: the unknown row contributes
+               nothing to it because it truly is unknown, not zero. */
+            Assert.Equal(6L, reader.GetInt64(0));
+            Assert.Equal(5_500_000L, reader.GetInt64(1));
+            Assert.Equal(400_000L, reader.GetInt64(2));
+            await reader.CloseAsync();
+
+            /* The Viewer's worker_time_per_second math: a normal row divides cleanly; the NULL-CPU row
+               must produce NULL rather than erroring (NULLIF/division against a NULL numerator is NULL in
+               SQL, never a throw — this proves it against the real store and the real query, not by
+               argument). */
+            await using var viewerProbe = new NpgsqlCommand(
+                "SELECT CAST(delta_worker_time AS double precision) / NULLIF(sample_interval_seconds, 0) / 1000.0 AS worker_time_per_second " +
+                "FROM collect.query_stats WHERE server_id = $1 ORDER BY collection_time", connection);
+            viewerProbe.Parameters.AddWithValue(ServerId);
+            await using var viewerReader = await viewerProbe.ExecuteReaderAsync(ct);
+
+            Assert.True(await viewerReader.ReadAsync(ct));
+            Assert.False(viewerReader.IsDBNull(0));
+            Assert.Equal(6.6667, viewerReader.GetDouble(0), 3);
+
+            Assert.True(await viewerReader.ReadAsync(ct));
+            Assert.True(viewerReader.IsDBNull(0), "the NULL-CPU row's worker_time_per_second must be NULL, not an error and not a fabricated zero");
+
+            bodySucceeded = true;
+        }
+        finally
+        {
+            await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (cleanup, cleanupCt) =>
+                await DarlingMcpTestData.ExecAsync(cleanup, cleanupCt, "DELETE FROM query_stats WHERE server_id = $1", ServerId));
+        }
+    }
+
+    private static async Task PlantAsync(
+        NpgsqlConnection connection, CancellationToken ct, DateTime at, long? workerTime, long elapsedTime, long executions, int intervalSeconds)
+    {
+        await using var insert = new NpgsqlCommand(@"
+INSERT INTO collect.query_stats
+    (collection_id, collection_time, server_id, server_name, database_name, query_hash, sql_handle,
+     delta_worker_time, delta_elapsed_time, delta_execution_count, sample_interval_seconds)
+VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)", connection);
+        insert.Parameters.AddWithValue(CollectionIdGenerator.Next());
+        insert.Parameters.AddWithValue(DarlingMcpTestData.TruncateToSeconds(at));
+        insert.Parameters.AddWithValue(ServerId);
+        insert.Parameters.AddWithValue(ServerName);
+        insert.Parameters.AddWithValue(Db);
+        insert.Parameters.AddWithValue("0xUNKNOWNCPU");
+        insert.Parameters.AddWithValue("0xSH1");
+        insert.Parameters.AddWithValue((object?)workerTime ?? DBNull.Value);
+        insert.Parameters.AddWithValue(elapsedTime);
+        insert.Parameters.AddWithValue(executions);
+        insert.Parameters.AddWithValue(intervalSeconds);
+        await insert.ExecuteNonQueryAsync(ct);
+    }
+
+    private static async Task RefreshAsync(NpgsqlConnection connection, string view, DateTime from, DateTime to, CancellationToken ct)
+    {
+        await using var refresh = new NpgsqlCommand($"CALL refresh_continuous_aggregate('collect.{view}'::regclass, $1::timestamp, $2::timestamp)", connection);
+        refresh.Parameters.AddWithValue(from);
+        refresh.Parameters.AddWithValue(to);
+        await refresh.ExecuteNonQueryAsync(ct);
+    }
+}

Darling/Darling.Tests/QueryStatsUnknownCpuTests.cs

@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System.Linq;
+using PerformanceMonitor.Collectors;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4394: a <c>query_stats</c> row whose worker (CPU) counter's own delta call returned interval 0 — a
+/// first sighting, a plan reset, or a gap past the policy — while the exec-count or elapsed-time
+/// counters (same row, same pass) returned a real, knowable delta used to write a false CPU of 0 over
+/// interval 0, and the interval-honest filter (<c>sample_interval_seconds IS DISTINCT FROM 0</c>) then
+/// discarded that row's real executions and duration along with the fabricated CPU. #2234 established
+/// interval 0 as the pairing for "no delta knowable"; <see cref="QueryStatsCollector.ResolveWorkerDelta"/>
+/// makes that hold per counter instead of per row.
+/// </summary>
+public sealed class QueryStatsUnknownCpuTests
+{
+    /// <summary>Worker (0, 0) — unknowable — with a real exec delta: CPU becomes NULL, interval comes from exec.</summary>
+    [Fact]
+    public void WorkerUnknown_ExecKnown_WritesNullCpuAndTheExecInterval()
+    {
+        var (workerDelta, intervalSeconds) = QueryStatsCollector.ResolveWorkerDelta(
+            workerDelta: 0, workerIntervalSeconds: 0, execIntervalSeconds: 60, elapsedIntervalSeconds: 0);
+
+        Assert.Null(workerDelta);
+        Assert.Equal(60, intervalSeconds);
+    }
+
+    /// <summary>Worker AND exec unknown, but elapsed is real: CPU becomes NULL, interval falls back to elapsed.</summary>
+    [Fact]
+    public void WorkerUnknown_ExecUnknown_ElapsedKnown_WritesNullCpuAndTheElapsedInterval()
+    {
+        var (workerDelta, intervalSeconds) = QueryStatsCollector.ResolveWorkerDelta(
+            workerDelta: 0, workerIntervalSeconds: 0, execIntervalSeconds: 0, elapsedIntervalSeconds: 60);
+
+        Assert.Null(workerDelta);
+        Assert.Equal(60, intervalSeconds);
+    }
+
+    /// <summary>
+    /// THE STAKEHOLDER'S CASE: a worker delta of 0 over a REAL interval (a query that ran but burned no
+    /// measurable CPU, e.g. blocked the whole window) is a MEASURED zero, not an unknowable one — it
+    /// must not become NULL just because it happens to equal zero.
+    /// </summary>
+    [Fact]
+    public void WorkerZeroOverARealInterval_KeepsTheMeasuredZero()
+    {
+        var (workerDelta, intervalSeconds) = QueryStatsCollector.ResolveWorkerDelta(
+            workerDelta: 0, workerIntervalSeconds: 60, execIntervalSeconds: 60, elapsedIntervalSeconds: 60);
+
+        Assert.Equal(0L, workerDelta);
+        Assert.Equal(60, intervalSeconds);
+    }
+
+    /// <summary>All three counters unknown (first sighting, reset, or gap across the board): unchanged — (0, 0).</summary>
+    [Fact]
+    public void AllCountersUnknown_StaysZeroAndZero()
+    {
+        var (workerDelta, intervalSeconds) = QueryStatsCollector.ResolveWorkerDelta(
+            workerDelta: 0, workerIntervalSeconds: 0, execIntervalSeconds: 0, elapsedIntervalSeconds: 0);
+
+        Assert.Equal(0L, workerDelta);
+        Assert.Equal(0, intervalSeconds);
+    }
+
+    /// <summary>A normal worker delta with a real interval passes through untouched.</summary>
+    [Fact]
+    public void ANormalWorkerDelta_PassesThroughUnchanged()
+    {
+        var (workerDelta, intervalSeconds) = QueryStatsCollector.ResolveWorkerDelta(
+            workerDelta: 100, workerIntervalSeconds: 60, execIntervalSeconds: 60, elapsedIntervalSeconds: 60);
+
+        Assert.Equal(100L, workerDelta);
+        Assert.Equal(60, intervalSeconds);
+    }
+
+    /// <summary>
+    /// Drives the real collector payload: a delta calculator that already knows the exec/elapsed keys
+    /// (a prior pass) but sees this row's WORKER key for the first time. On dev this writes a false CPU
+    /// of 0 at interval 0 and discards the row's real executions/elapsed under the interval-honest
+    /// filter; after the fix it writes CPU as NULL and takes the interval from the exec counter.
+    /// </summary>
+    [Fact]
+    public void ThroughTheCollectorPayload_AFirstSightingCpuWithKnownExecAndElapsed_WritesNullCpuAndARealInterval()
+    {
+        const string sqlHandle = "0x0102";
+        const string planHandle = "0x0304";
+        const int startOffset = 0;
+        const int endOffset = -1;
+        var deltaKey = $"{sqlHandle}:{startOffset}:{endOffset}:{planHandle}";
+
+        var deltas = new CollectorDeltaCalculator();
+        var t0 = new System.DateTime(2026, 9, 1, 12, 0, 0, System.DateTimeKind.Unspecified);
+        var t1 = t0.AddSeconds(60);
+
+        /* Establish exec and elapsed at pass one so pass two has a real, knowable delta for both — but
+           never touch the worker family, so its pass-two call is a genuine first sighting. */
+        deltas.CalculateDeltaWithSeriesAge(1, "query_stats_exec", deltaKey, 10, null, out _, t0, CollectorDeltaCalculator.DefaultMaxGapSeconds);
+        deltas.CalculateDeltaWithSeriesAge(1, "query_stats_elapsed", deltaKey, 5_000_000, null, out _, t0, CollectorDeltaCalculator.DefaultMaxGapSeconds);
+
+        var row = new QueryStatsCollector.Row
+        {
+            QueryHash = "queryhash",
+            ExecutionCount = 13,
+            TotalWorkerTime = 999,
+            TotalElapsedTime = 5_010_000,
+            SqlHandle = sqlHandle,
+            PlanHandle = planHandle,
+            StatementStartOffset = startOffset,
+            StatementEndOffset = endOffset,
+        };
+
+        var context = new CollectorContext
+        {
+            ServerId = 1,
+            ServerName = "target-a",
+            CollectionTime = t1,
+            Deltas = deltas,
+        };
+
+        var writer = new RecordingCollectorRowWriter();
+        QueryStatsCollector.Instance.WritePayload(row, writer, context);
+
+        var columnIndex = QueryStatsCollector.Instance.PayloadColumns
+            .Select((c, i) => (c.Name, i))
+            .ToDictionary(p => p.Name, p => p.i);
+
+        Assert.Null(writer.Values[columnIndex["delta_worker_time"]]);
+        Assert.Equal(3L, writer.Values[columnIndex["delta_execution_count"]]);
+        Assert.Equal(60, writer.Values[columnIndex["sample_interval_seconds"]]);
+    }
+
+    private sealed class RecordingCollectorRowWriter : ICollectorRowWriter
+    {
+        public System.Collections.Generic.List<object?> Values { get; } = new();
+        private ICollectorRowWriter Add(object? v) { Values.Add(v); return this; }
+        public ICollectorRowWriter Value(string? value) => Add(value);
+        public ICollectorRowWriter Value(long value) => Add(value);
+        public ICollectorRowWriter Value(long? value) => Add(value);
+        public ICollectorRowWriter Value(int value) => Add(value);
+        public ICollectorRowWriter Value(int? value) => Add(value);
+        public ICollectorRowWriter Value(short value) => Add(value);
+        public ICollectorRowWriter Value(short? value) => Add(value);
+        public ICollectorRowWriter Value(double value) => Add(value);
+        public ICollectorRowWriter Value(double? value) => Add(value);
+        public ICollectorRowWriter Value(decimal value) => Add(value);
+        public ICollectorRowWriter Value(decimal? value) => Add(value);
+        public ICollectorRowWriter Value(bool value) => Add(value);
+        public ICollectorRowWriter Value(bool? value) => Add(value);
+        public ICollectorRowWriter Value(System.DateTime value) => Add(value);
+        public ICollectorRowWriter Value(System.DateTime? value) => Add(value);
+        public ICollectorRowWriter NullValue() => Add(null);
+    }
+}