From cfbc0b3a96a861dbc8365778c75db90cc9094ca1 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:31:21 -0400 Subject: [PATCH 1/3] AG latency: commit-time columns, drain-time estimates, primary-side perfmon counters (#991) ag_database_replica_states gains six appended columns: - last_commit_time / last_hardened_time / last_redone_time / last_received_time, the four DMV timestamps the reference query skips. Unlike secondary_lag_seconds these are directly comparable across replicas, which the primary-vs-secondary commit-time lag math needs. - est_redo_completion_time_min / est_send_drain_time_min, computed server-side as queue / rate / 60 with both guards the raw expression needs: `* 1.0` stops BIGINT/BIGINT integer division flooring a sub-minute drain to zero, NULLIF(rate, 0) stops the divide-by-zero an idle or suspended replica raises (which fails the whole cycle, not one column). NULL means "no drain rate" and is never coerced to 0, which would read as "drains instantly". The estimates are computed per row at the sample's own instant rather than composed later, because a ratio of two window aggregates is not the average of the per-sample ratios and the two diverge worst exactly when rates swing. Both are Gauge compose measures, duration family, minutes. PerfmonStatsCollector's whitelist gains Transaction Delay and Mirrored Write Transactions/sec - the primary side of commit latency, their ratio being the average delay per mirrored transaction. Zero new schema. Both verified live on SQL2022: they exist with no AGs configured, sit on SQLServer:Database Replica, and occur exactly once server-wide, so the counter_name-only filter cannot collide. Corrects a doc claim shipped in #1688. It restated MS Learn's assertion that secondary_lag_seconds reads 0 while data movement is suspended; a Docker AG fixture measured the inverse on SQL Server 2022 in a CLUSTER_TYPE = NONE group - 0 while movement is ACTIVE and caught up, accruing monotonically once suspended (0 to 62s across a 60s SUSPEND_FROM_USER, back to 0 on resume). A suspended replica does not hide as zero lag. Also documents two further measured quirks: log_send_queue_size goes NULL while suspended while redo_queue_size freezes at its last value, and collecting from a secondary yields a one-row self-view because sys.dm_hadr_* carries only the local replica there. Store migration V36 appends the columns additively instead of widening V34, because V34's CREATE TABLE IF NOT EXISTS is a no-op on an already-migrated store - editing it would leave every existing store six columns short while fresh installs got them. The schema pin now reconstructs the current shape from V34 + V36 and compares it to the generator; both failure modes confirmed by planted defects. Version 36 because 35 is claimed by the concurrent AG-alerts work. THIRD_PARTY_NOTICES.md now credits Hannah Vernon's SqlServerAgMonitor alongside the existing collector-header attribution. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 + Darling/Darling.Tests/DarlingComposeTests.cs | 4 +- .../DarlingObservabilityTests.cs | 18 +++- .../Darling.Tests/DarlingServerTagsTests.cs | 2 +- .../Darling.Tests/PgSchemaGeneratorTests.cs | 70 +++++++++++++-- .../Darling.Tests/ViewerDataServiceTests.cs | 2 +- .../Compose/MeasureCatalog.cs | 21 +++++ .../PgMigrations.cs | 24 +++++ .../StorageVersion.cs | 2 +- .../ViewerDataService.cs | 16 +++- Lite.Tests/AgCollectorDefinitionTests.cs | 87 ++++++++++++++++++- Lite.Tests/GoldenCollectorSchema.cs | 8 +- ...nAndDmvBlockingCollectorDefinitionTests.cs | 9 +- .../AgDatabaseReplicaStatesCollector.cs | 73 ++++++++++++++-- .../PerfmonStatsCollector.cs | 8 ++ THIRD_PARTY_NOTICES.md | 37 ++++++++ 16 files changed, 352 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 286bdc59c..5c5cae3ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **AG latency: commit-time columns, drain-time estimates, and the primary-side perfmon counters** ([#1695]) - completes the Availability Group latency picture [#1688] started. `ag_database_replica_states` gains **four DMV timestamps the reference query skips** (`last_commit_time`, `last_hardened_time`, `last_redone_time`, `last_received_time`) - unlike `secondary_lag_seconds` these are directly comparable across replicas, which is what the canonical primary-vs-secondary commit-time lag math needs - plus **two server-computed drain-time estimates**, `est_redo_completion_time_min` and `est_send_drain_time_min` (queue / rate / 60). Both estimates carry the two guards the raw expression needs: `* 1.0` stops BIGINT-over-BIGINT integer division flooring a sub-minute drain to zero, and `NULLIF(rate, 0)` stops the divide-by-zero an idle or suspended replica would otherwise raise - which would fail the whole cycle, not one column. A NULL estimate honestly means "no drain rate" and is never coerced to 0, which would read as "drains instantly". They are computed per row at the sample's own instant rather than composed later as a ratio of two window averages, because avg(queue)/avg(rate) is not the average of the per-sample ratios and the two diverge worst exactly when rates swing. Both are Gauge compose measures (duration family, native minutes). **Two perfmon counters** join the existing whitelist - `Transaction Delay` and `Mirrored Write Transactions/sec` - carrying the PRIMARY side of commit latency (their ratio is the average delay per mirrored transaction); zero new schema, they ride `perfmon_stats`. Verified on SQL2022 that both exist even with no AGs configured, live on `SQLServer:Database Replica`, and occur exactly once server-wide, so the collector's counter_name-only filter cannot collide. Together with `HADR_SYNC_COMMIT`, which already flows through `wait_stats`, sync-commit pressure is now composable end to end: primary-side delay, secondary-side queues and rates, drain estimates, and the wait itself. **A shipped doc claim is corrected against a live AG.** [#1688] restated MS Learn's assertion that `secondary_lag_seconds` reads 0 while data movement is suspended; a Docker AG fixture measured the inverse on SQL Server 2022 in a `CLUSTER_TYPE = NONE` group - it reads 0 while movement is ACTIVE and caught up, and accrues monotonically once suspended (0 to 62 s across a 60 s `SUSPEND_FROM_USER`, back to 0 on resume). So a suspended replica does not hide as zero lag and a lag threshold fires on its own; reading `is_suspended` alongside explains WHY lag is climbing rather than catching lag that is masked. Two further measured quirks are now documented: `log_send_queue_size` goes NULL while suspended while `redo_queue_size` FREEZES at its last value (so a redo-queue reading on a suspended replica is stale, not current), and collecting from a SECONDARY yields a one-row self-view because `sys.dm_hadr_*` carries only the local replica there - a complete AG picture requires collecting from the primary. Store migration V36 appends the six columns additively rather than widening V34 in place, because V34's `CREATE TABLE IF NOT EXISTS` is a no-op on an already-migrated store and editing it would silently leave every existing store short the new columns while fresh installs got them; the schema pin now reconstructs the current shape from V34 + V36 and compares it to the generator, with both failure modes confirmed by planted defects. Hannah Vernon's SqlServerAgMonitor is now credited in `THIRD_PARTY_NOTICES.md` alongside the existing collector-header attribution. - **AG collection: document the second grant it needs, and make the lag trap filterable** ([#1691]) - review follow-ups to [#1688]. **The grant is the one that matters in the field.** Both AG collectors join the `sys.availability_groups` / `sys.availability_replicas` CATALOG VIEWS to the `sys.dm_hadr_*` DMVs, and while the DMVs are covered by the `VIEW SERVER STATE` the product asks for, [the catalog views require `VIEW ANY DEFINITION`](https://learn.microsoft.com/en-us/sql/database-engine/availability-groups/windows/monitor-availability-groups-transact-sql) - which catalog views enforce by HIDING ROWS, not by raising an error. So on a fully configured AG cluster a monitoring login with only the documented grant returned zero rows, which is exactly what an AG-less server returns: the collectors would have looked healthy forever while collecting nothing, with no error anywhere to notice. Now called out in both READMEs (the Lite/Darling grant script, the Darling permission table's If-missing column) and in both collector headers, along with the fingerprint that identifies it if it is ever worth detecting automatically - the DMV returning rows while the catalog view returns none is unambiguous, and `SERVERPROPERTY('IsHadrEnabled')` is readable by every login. **The documented lag trap is now actionable instead of just documented**: `secondary_lag_seconds` reads 0 rather than NULL while data movement is suspended, so a suspended replica charts as perfectly healthy - but nothing exposed the suspension state to a panel, so the misread the code comment warned about was unavoidable. `synchronization_state_desc` and `suspend_reason_desc` are now compose dimensions, so a lag panel can filter suspended replicas out or group by suspend reason. (`is_suspended` itself cannot be a dimension: the compiler binds filter values as text, which would not match a boolean column.) **And the V34 migration is now genuinely pinned.** Its test asserted only that each column NAME appeared somewhere in the DDL - a V34 with the columns reordered, `is_local` typed `text`, or a spurious `NOT NULL` passed every test, while a comment claimed the shape was pinned elsewhere. It now compares the whole generated `CREATE TABLE` via the existing `Migrations_JobHistoryAndAgentStatus_MatchGeneratedFreshShape` idiom; detection power confirmed by planting `is_local text` and watching it go red. That test's long-standing rationale was corrected too - it blamed a "positional binary COPY", but `PgCollectorRowWriter.CopyCommandFor` emits a named column list so Postgres binds by name; names and types are the real hazard on that side, and the positional appender is Lite's. - **Availability Group health collection in Lite + Darling** ([#1688]) - closes #991 and the AG item of #1606, the one real coverage gap the Datadog DBM comparison turned up. Two new shared collectors, both server-scope and both zero-cost on a server without Always On: **`ag_replica_states`** (replica grain - role, operational/connected state, recovery and synchronization health, availability and failover mode, endpoint URL) from `sys.availability_replicas` joined to `sys.availability_groups` and `sys.dm_hadr_availability_replica_states`, and **`ag_database_replica_states`** (database grain - synchronization state, log send and redo queue sizes, send and redo rates, secondary lag, suspension state and reason, and both LSNs) from `sys.dm_hadr_database_replica_states`. The metric surface follows Hannah Vernon's [SqlServerAgMonitor](https://github.com/HannahVernon/SqlServerAgMonitor) (MIT), attributed in both collector headers. `WHERE COALESCE(is_distributed, 0) = 0` keeps distributed-AG container rows out while member AGs still appear; drilling into a DAG's remote members needs a connection per member and is out of scope. Runs everywhere except Azure SQL DB, which has no AG surface - an AG-less on-prem server is deliberately NOT gated off, it collects and stores zero rows, so turning Always On on later starts producing data with no configuration change. Three details that are easy to get wrong and are pinned by test: the queues, rates and lag are **instantaneous gauges, not counters**, so neither collector touches the delta framework and the compose measures are Gauge (avg/min/max, never SUM - summing a backlog over a window is a category error); `last_hardened_lsn` / `last_commit_lsn` are `numeric(25, 0)`, **wider than BIGINT**, so they are converted server-side and stored as text rather than silently overflowing; and every column is read null-tolerantly, because under WSFC quorum loss `sys.availability_replicas` serves only locally cached metadata and `endpoint_url` is documented NULL. Both apps schedule it per minute with 30-day retention (the grain at which a lag or queue spike is still visible), and it is in all three cadence presets (per-minute on Aggressive and Balanced, 5 min on Low-Impact). **Custom Views v2** gets five Gauge measures on the database grain - send queue, redo queue, send rate, redo rate and secondary lag - under an Availability Groups category, sliceable and groupable by AG, database and replica plus the synchronization/suspend state (and the universal server dimension), which lights up send-rate-vs-redo-rate on a dual axis and worst-lag-per-replica out of the box. Only the database-grain table carries measures - the replica-grain table is all state strings with nothing numeric to aggregate, so it is stored for the coming viewer tab rather than queryable from Custom Views today. Darling store migration V34; Lite's storage registers itself off the collector catalog. Collection only in this first cut: no viewer tab and no failover / sync-fell-behind alerts yet, both tracked as follow-ups, and both apps' viewer-coverage ratchets carry the two tables as explicitly tracked debt so the tab cannot be forgotten. That ratchet also got a real fix along the way - it text-scans the viewer's reader layer for a table name, and naming a collector table as a schema-version probe sentinel (as V34 does) made the table read as "already covered", silently exempting it; the scan now strips the probe's `information_schema` lines, which retroactively hardens the same pin for `long_query_completions`. - **A Docker Availability Group fixture, and what it found about the AG DMVs** ([#1689]) - the AG DMVs return nothing at all on a standalone instance, so AG collection has never had anything to be validated against. `tools/ag-fixture` stands up two SQL Server 2022 containers as a clusterless (`CLUSTER_TYPE = NONE`) availability group - shared endpoint certificate, mirroring endpoints on 5022, automatically-seeded `AgFixtureDb` - in one idempotent `setup.ps1`, with a write-load script to make the queues and rates move and a suspend/resume recipe to fault it. Primary on `localhost,14331`, secondary on `localhost,14332`. Both AG collector queries (#991) were run against it verbatim and validated, including a suspend fault; evidence is recorded in `tools/ag-fixture/VALIDATION.md`. Three behaviors it surfaced that the DMV documentation does not prepare you for: **a secondary reports only itself** - `sys.availability_replicas` holds every replica on both nodes, but `sys.dm_hadr_availability_replica_states` and `sys.dm_hadr_database_replica_states` hold only the local one on a secondary, so a complete AG picture requires collecting from the primary and monitoring only a secondary yields a one-row self-view; **`log_send_queue_size` reads NULL while data movement is suspended** rather than growing, with `redo_queue_size` frozen at its last value, so a send-queue threshold is blind to a suspended secondary and `is_suspended`/`suspend_reason_desc` are the signal for it; and **`secondary_lag_seconds` accrues while suspended** (measured 0 -> 15 -> 31 -> 46 -> 62 across a 60-second suspend, back to 0 on resume), which is the inverse of MS Learn's documented "this value shows as 0 if the data movement is suspended" - it reads 0 when movement is active and caught up, not when it is suspended. Measured on `16.0.4265.3`, clusterless AG only. The fixture is deliberately sized to share a machine with a VM fleet rather than to perform: 1 CPU and 2 GB per container with the engine held to 1536 MB, no SQL Agent, 64 MB database files, and a periodic overwriting log backup in the write-load script - without which a single 90-second run took the log to 1.6 GB, since an AG database must stay in FULL recovery and cannot truncate its own log. Measured steady-state usage is recorded alongside the rest of the evidence. @@ -1657,4 +1658,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1680]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1680 [#1688]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1688 [#1691]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1691 +[#1695]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1695 [#1690]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1690 diff --git a/Darling/Darling.Tests/DarlingComposeTests.cs b/Darling/Darling.Tests/DarlingComposeTests.cs index ba1903456..c1d2c535c 100644 --- a/Darling/Darling.Tests/DarlingComposeTests.cs +++ b/Darling/Darling.Tests/DarlingComposeTests.cs @@ -857,7 +857,7 @@ composer SUM a backlog over a window and report a number that means nothing. */ .Where(m => string.Equals(m.SourceTable, "ag_database_replica_states", StringComparison.Ordinal)) .ToList(); - Assert.Equal(5, agMeasures.Count); + Assert.Equal(7, agMeasures.Count); foreach (var measure in agMeasures) { @@ -871,7 +871,7 @@ composer SUM a backlog over a window and report a number that means nothing. */ } Assert.Equal( - new[] { "ag_log_send_queue", "ag_log_send_rate", "ag_redo_queue", "ag_redo_rate", "ag_secondary_lag" }, + new[] { "ag_est_redo_drain_min", "ag_est_send_drain_min", "ag_log_send_queue", "ag_log_send_rate", "ag_redo_queue", "ag_redo_rate", "ag_secondary_lag" }, agMeasures.Select(m => m.Key).OrderBy(k => k, StringComparer.Ordinal).ToArray()); } diff --git a/Darling/Darling.Tests/DarlingObservabilityTests.cs b/Darling/Darling.Tests/DarlingObservabilityTests.cs index 232c5af3a..a87cfe2d4 100644 --- a/Darling/Darling.Tests/DarlingObservabilityTests.cs +++ b/Darling/Darling.Tests/DarlingObservabilityTests.cs @@ -32,9 +32,12 @@ public sealed class DarlingObservabilityTests private const int TestServerId = -424242; [Fact] - public void MigrationScripts_ThirtyFourVersions_V33ConnectionAlertOptIns_V34AgCollectors() + public void MigrationScripts_V34AgCollectors_V36AgLatencyColumns() { - Assert.Equal(34, PgMigrations.Scripts.Count); + /* 35 scripts, not 36: version 35 is deliberately absent here — it belongs to the concurrent + AG-alerts work, and the ladder is a list of the versions THIS branch defines, not a dense + range. The applier runs pending versions in order, so a gap is harmless. */ + Assert.Equal(35, PgMigrations.Scripts.Count); Assert.Equal(1, PgMigrations.Scripts[0].Version); Assert.Equal(2, PgMigrations.Scripts[1].Version); Assert.Equal(3, PgMigrations.Scripts[2].Version); @@ -69,7 +72,8 @@ public void MigrationScripts_ThirtyFourVersions_V33ConnectionAlertOptIns_V34AgCo Assert.Equal(32, PgMigrations.Scripts[31].Version); Assert.Equal(33, PgMigrations.Scripts[32].Version); Assert.Equal(34, PgMigrations.Scripts[33].Version); - Assert.Equal(34, StorageVersion.SchemaVersion); + Assert.Equal(36, PgMigrations.Scripts[34].Version); + Assert.Equal(36, StorageVersion.SchemaVersion); /* V34 (#991) creates the two Availability Group collector tables. Schema-qualified collect.* and CREATE TABLE IF NOT EXISTS, per the file's additive-create idiom (V29): a no-op on a fresh store @@ -84,6 +88,14 @@ The full column-for-column equality against PgSchemaGenerator.CreateTable is pin Assert.Contains("last_hardened_lsn text", v34, StringComparison.Ordinal); Assert.Contains("last_commit_lsn text", v34, StringComparison.Ordinal); + /* V36 (#991 addendum) widens the V34 database-grain table additively. Identity only here; the + column-for-column reconstruction of V34 + V36 against the generator is pinned by + PgSchemaGeneratorTests.Migrations_JobHistoryAndAgentStatus_MatchGeneratedFreshShape. */ + var v36 = PgMigrations.Scripts[34].Sql; + Assert.Equal("ag-latency-columns", PgMigrations.Scripts[34].Name); + Assert.Contains("ALTER TABLE collect.ag_database_replica_states", v36, StringComparison.Ordinal); + Assert.Contains("ADD COLUMN IF NOT EXISTS est_send_drain_time_min double precision", v36, StringComparison.Ordinal); + /* V26 (#1506) adds the generic webhook channel's four columns to the V17 control-plane table. Schema-qualified config.* and IF NOT EXISTS, per the file's additive-ALTER idiom. */ var v26 = PgMigrations.Scripts[25].Sql; diff --git a/Darling/Darling.Tests/DarlingServerTagsTests.cs b/Darling/Darling.Tests/DarlingServerTagsTests.cs index 424436636..3388b69c7 100644 --- a/Darling/Darling.Tests/DarlingServerTagsTests.cs +++ b/Darling/Darling.Tests/DarlingServerTagsTests.cs @@ -34,7 +34,7 @@ public void V32_IsSchemaQualified_AndV33_IsRegisteredLast() and is what tracks the build version. */ var v33 = PgMigrations.Scripts.Single(s => s.Version == 33); Assert.Equal("connection-alert-refire", v33.Name); - Assert.Equal(34, PgMigrations.Scripts[^1].Version); + Assert.Equal(36, PgMigrations.Scripts[^1].Version); Assert.Equal(StorageVersion.SchemaVersion, PgMigrations.Scripts[^1].Version); Assert.Contains("ALTER TABLE config.config_alert_settings", v33.Sql, StringComparison.Ordinal); Assert.Contains("notify_connection_down_at_startup boolean NOT NULL DEFAULT false", v33.Sql, StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs index 9ade3afa3..044370c81 100644 --- a/Darling/Darling.Tests/PgSchemaGeneratorTests.cs +++ b/Darling/Darling.Tests/PgSchemaGeneratorTests.cs @@ -7,6 +7,7 @@ */ using System; +using System.Collections.Generic; using System.Linq; using PerformanceMonitor.Collectors; using PerformanceMonitor.Darling.Storage; @@ -420,17 +421,74 @@ static string CollectQualified(ICollectorSchemaInfo schema) Assert.Contains(CollectQualified(AgentStatusCollector.Instance), v25, StringComparison.Ordinal); Assert.Contains("CREATE INDEX IF NOT EXISTS idx_agent_status_time ON collect.agent_status(server_id, collection_time);", v25, StringComparison.Ordinal); - /* V34 (#991) creates BOTH Availability Group tables in one migration body — same contract, so it - is pinned here rather than by a name-presence check. The weaker `Assert.Contains(column.Name)` - sweep this replaces would have passed a V34 with the columns reordered, is_local typed text, or - a spurious NOT NULL: exactly the drifts that make an upgraded store's physical shape differ from - a fresh one. Compares the WHOLE generated CREATE TABLE, so shape is pinned, not vocabulary. */ + /* V34 (#991) creates BOTH Availability Group tables in one migration body — same contract. The + weaker `Assert.Contains(column.Name)` sweep this replaces would have passed a V34 with the + columns reordered, is_local typed text, or a spurious NOT NULL: exactly the drifts that make an + upgraded store's physical shape differ from a fresh one. */ var v34 = Lf(PgMigrations.Scripts.Single(m => m.Version == 34).Sql); Assert.Contains(CollectQualified(AgReplicaStatesCollector.Instance), v34, StringComparison.Ordinal); Assert.Contains("CREATE INDEX IF NOT EXISTS idx_ag_replica_states_time ON collect.ag_replica_states(server_id, collection_time);", v34, StringComparison.Ordinal); - Assert.Contains(CollectQualified(AgDatabaseReplicaStatesCollector.Instance), v34, StringComparison.Ordinal); Assert.Contains("CREATE INDEX IF NOT EXISTS idx_ag_database_replica_states_time ON collect.ag_database_replica_states(server_id, collection_time);", v34, StringComparison.Ordinal); + + /* The database-grain table is the one case where a single migration is NOT the whole story: V34 + created its first 15 payload columns and V36 (#991 addendum) appended 6 more, so an upgraded + store's shape is V34 + V36 and only their SUM can equal the generator's current output. + Reconstruct that here rather than weakening the pin to name-presence — generate the historical + 15-column shape and assert V34 matches it exactly, then assert V36 appends the remaining columns + in order with the generator's own types. Together those two prove fresh == upgraded. */ + var currentColumns = AgDatabaseReplicaStatesCollector.Instance.PayloadColumns; + const int V34ColumnCount = 15; + + Assert.Contains( + CollectQualified(new TruncatedSchema(AgDatabaseReplicaStatesCollector.Instance, V34ColumnCount)), + v34, + StringComparison.Ordinal); + + var v36 = Lf(PgMigrations.Scripts.Single(m => m.Version == 36).Sql); + + foreach (var column in currentColumns.Skip(V34ColumnCount)) + { + var generatedType = Lf(PgSchemaGenerator.CreateTable(new TruncatedSchema(AgDatabaseReplicaStatesCollector.Instance, currentColumns.Count))) + .Split('\n') + .Single(l => l.TrimStart().StartsWith(column.Name + " ", StringComparison.Ordinal)) + .Trim() + .TrimEnd(','); + + Assert.Contains($"ADD COLUMN IF NOT EXISTS {generatedType}", v36, StringComparison.Ordinal); + } + + /* And V34 must NOT have been widened in place: its CREATE TABLE IF NOT EXISTS is a no-op on a store + that already ran it, so editing V34 instead of adding V36 would leave every migrated store short + the new columns while fresh installs got them. */ + foreach (var appended in currentColumns.Skip(V34ColumnCount)) + { + Assert.DoesNotContain($" {appended.Name} ", v34, StringComparison.Ordinal); + } + } + + /// + /// A collector's schema surface with its payload truncated to the first N columns — lets a test + /// generate the HISTORICAL shape a migration froze, so an additive migration can be pinned as + /// "V-old created these, V-new appended those, and together they equal today's generated table". + /// + private sealed class TruncatedSchema : ICollectorSchemaInfo + { + private readonly ICollectorSchemaInfo _inner; + + public TruncatedSchema(ICollectorSchemaInfo inner, int columnCount) + { + _inner = inner; + PayloadColumns = inner.PayloadColumns.Take(columnCount).ToList(); + } + + public string Name => _inner.Name; + public string TargetTable => _inner.TargetTable; + public bool IncludesCollectionId => _inner.IncludesCollectionId; + public string PrefixIdColumnName => _inner.PrefixIdColumnName; + public string PrefixTimeColumnName => _inner.PrefixTimeColumnName; + public IReadOnlyList PayloadColumns { get; } + public bool AppliesTo(CollectorTargetInfo target) => _inner.AppliesTo(target); } [Fact] diff --git a/Darling/Darling.Tests/ViewerDataServiceTests.cs b/Darling/Darling.Tests/ViewerDataServiceTests.cs index 1ac715d9c..b085c5a08 100644 --- a/Darling/Darling.Tests/ViewerDataServiceTests.cs +++ b/Darling/Darling.Tests/ViewerDataServiceTests.cs @@ -456,7 +456,7 @@ public void RequiredStoreSchemaVersion_TracksTheBuildSchemaVersion_AndTheProbeCo the connect-time gate refuse to open the viewer against a perfectly healthy store. */ Assert.Equal( ViewerDataService.RequiredStoreSchemaVersion, - ViewerDataService.MapProbedSchemaVersion(true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true)); + ViewerDataService.MapProbedSchemaVersion(true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true, true)); } } diff --git a/Darling/PerformanceMonitor.Darling.Service/Compose/MeasureCatalog.cs b/Darling/PerformanceMonitor.Darling.Service/Compose/MeasureCatalog.cs index 85e3598da..f6c58da2a 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Compose/MeasureCatalog.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Compose/MeasureCatalog.cs @@ -1094,6 +1094,27 @@ suspended replica stops masquerading as a healthy one. ── */ DefaultTimeAgg = ComposeAggregate.Max, ValidAggs = GaugeAggs, AllowedDimensions = AgDatabaseDims, }, + /* The two drain-time ESTIMATES (#991 addendum). Computed server-side per row, at the sample's own + instant, rather than composed here as a queue/rate ratio — and that is the whole point: a ratio + of two window AGGREGATES (avg queue ÷ avg rate) is not the average of the per-sample ratios, and + the two diverge badly exactly when rates swing, which is when anyone is looking. Storing the + per-row ratio and averaging THAT is the honest read. NULL where no rate exists (idle, suspended, + caught up); NULL is never coerced to 0, which would read as "drains instantly". */ + new ComposeMeasure + { + Key = "ag_est_redo_drain_min", DisplayName = "AG estimated redo drain time", Category = CatAvailabilityGroups, SourceTable = "ag_database_replica_states", + Archetype = MeasureArchetype.Gauge, Column = "est_redo_completion_time_min", + NativeUnit = "min", DefaultUnit = "min", UnitFamily = FamilyDuration, + DefaultTimeAgg = ComposeAggregate.Max, ValidAggs = GaugeAggs, AllowedDimensions = AgDatabaseDims, + }, + new ComposeMeasure + { + Key = "ag_est_send_drain_min", DisplayName = "AG estimated send drain time", Category = CatAvailabilityGroups, SourceTable = "ag_database_replica_states", + Archetype = MeasureArchetype.Gauge, Column = "est_send_drain_time_min", + NativeUnit = "min", DefaultUnit = "min", UnitFamily = FamilyDuration, + DefaultTimeAgg = ComposeAggregate.Max, ValidAggs = GaugeAggs, AllowedDimensions = AgDatabaseDims, + }, + /* ═══════════ Same-source ratios (design §2c) — the bread-and-butter derived metrics ═══════════ */ /* Supporting execution-count scalars the "average per execution" ratios divide by (query_stats had no executions measure; procedure_stats already exposes proc_executions). */ diff --git a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs index 3266cfbae..e5aa3a5e6 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs @@ -78,6 +78,7 @@ public Migration(int version, string name, string sql) new Migration(32, "server-tags", V32Sql), new Migration(33, "connection-alert-refire", V33Sql), new Migration(34, "availability-group-collectors", V34Sql), + new Migration(36, "ag-latency-columns", V36Sql), }; /// @@ -545,6 +546,29 @@ secondary_lag_seconds bigint CREATE INDEX IF NOT EXISTS idx_ag_database_replica_states_time ON collect.ag_database_replica_states(server_id, collection_time);"; + /// + /// V36 — the AG latency columns (#991 addendum): the four commit/hardened/redone/received timestamps + /// the reference project's query skips, plus the two server-computed drain-time estimates + /// (queue ÷ rate ÷ 60, guarded against BIGINT integer division and a zero rate). + /// APPENDED, never inserted, and deliberately a SEPARATE migration rather than an edit to V34: + /// V34's CREATE TABLE IF NOT EXISTS is a no-op on a store that already ran it, so widening V34 + /// in place would silently leave every already-migrated store (the field box included) six columns + /// short while fresh installs got them — the exact drift the fresh-vs-upgraded shape pin exists to + /// catch. ADD COLUMN IF NOT EXISTS appends physically, matching the order + /// emits for a fresh store, so both provenances end up + /// column-for-column identical (pinned by PgSchemaGeneratorTests, which reconstructs the current + /// shape from V34 + V36 and compares it to the generator). + /// Version 36, not 35: 35 is claimed by the concurrent AG-alerts work. + /// + private const string V36Sql = @" +ALTER TABLE collect.ag_database_replica_states + ADD COLUMN IF NOT EXISTS last_commit_time timestamp, + ADD COLUMN IF NOT EXISTS last_hardened_time timestamp, + ADD COLUMN IF NOT EXISTS last_redone_time timestamp, + ADD COLUMN IF NOT EXISTS last_received_time timestamp, + ADD COLUMN IF NOT EXISTS est_redo_completion_time_min double precision, + ADD COLUMN IF NOT EXISTS est_send_drain_time_min double precision;"; + /// /// V9 — the FinOps copy-parity fields that were user-input config or previously live-only: /// server_properties gains the three inventory columns the shared ServerPropertiesCollector now diff --git a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs index 97aecec70..7bf326553 100644 --- a/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs +++ b/Darling/PerformanceMonitor.Darling.Storage/StorageVersion.cs @@ -16,5 +16,5 @@ namespace PerformanceMonitor.Darling.Storage; /// public static class StorageVersion { - public const int SchemaVersion = 34; + public const int SchemaVersion = 36; } diff --git a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs index 7aa762a8d..46bcd95b0 100644 --- a/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs +++ b/Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs @@ -395,7 +395,8 @@ OR NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'timescaledb') EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'custom_views'), EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'server_tags'), EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'config_alert_settings' AND column_name = 'notify_connection_down_at_startup'), - EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'ag_database_replica_states')"; + EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'ag_database_replica_states'), + EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'ag_database_replica_states' AND column_name = 'est_send_drain_time_min')"; /// The store schema version this viewer build requires — the highest migration it knows /// (). The connect-time gate blocks a store below this. @@ -416,7 +417,7 @@ OR NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'timescaledb') await using var reader = await command.ExecuteReaderAsync(cancellationToken); if (await reader.ReadAsync(cancellationToken)) { - return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17)); + return MapProbedSchemaVersion(reader.GetBoolean(0), reader.GetBoolean(1), reader.GetBoolean(2), reader.GetBoolean(3), reader.GetBoolean(4), reader.GetBoolean(5), reader.GetBoolean(6), reader.GetBoolean(7), reader.GetBoolean(8), reader.GetBoolean(9), reader.GetBoolean(10), reader.GetBoolean(11), reader.GetBoolean(12), reader.GetBoolean(13), reader.GetBoolean(14), reader.GetBoolean(15), reader.GetBoolean(16), reader.GetBoolean(17), reader.GetBoolean(18)); } return null; @@ -441,8 +442,17 @@ OR NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'timescaledb') /// is unit-tested without a live store; any schema bump past the newest arm trips the pinning test that keeps /// this in step with . /// - internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false) + internal static int MapProbedSchemaVersion(bool hasConfigControlPlane, bool hasAlertDeliveryOverride, bool hasAnalysisState, bool hasAlertTuningKnobs, bool hasDefaultTraceEvents, bool hasIndexObjectStatsLatestIndex, bool hasCollectionLogHypertableOrPlainPg, bool hasJobHistory, bool hasAgentStatus, bool hasGenericWebhook, bool hasDeadlocksDatabaseName, bool hasQueryStoreReplicaRole, bool hasLongQueryCompletions, bool hasWebDashboardConfig, bool hasCustomViews, bool hasServerTags, bool hasConnectionRefireKnobs = false, bool hasAgCollectors = false, bool hasAgLatencyColumns = false) { + /* V36 (#991 addendum, AG latency columns): engine-agnostic COLUMN-existence sentinel — V36 only + widens the V34 table, so the table-existence arm below cannot distinguish the two. Newest-first. + (Version 35 is the concurrent AG-alerts migration; a store at 35 without these columns falls + through to the V34 arm, which is correct — it is below what this build requires either way.) */ + if (hasAgLatencyColumns) + { + return 36; + } + /* V34 (#991 Availability Group collectors): engine-agnostic table-existence sentinel, newest-first arm. The AG database-grain collector table exists only at V34 or later. (Named only in the probe SQL, deliberately not repeated here: ViewerCollectorCoverageTests scans this layer by substring diff --git a/Lite.Tests/AgCollectorDefinitionTests.cs b/Lite.Tests/AgCollectorDefinitionTests.cs index 35e522481..b6f3a3cb2 100644 --- a/Lite.Tests/AgCollectorDefinitionTests.cs +++ b/Lite.Tests/AgCollectorDefinitionTests.cs @@ -77,7 +77,13 @@ ORDER BY is_suspended = hdrs.is_suspended, suspend_reason_desc = hdrs.suspend_reason_desc, availability_mode_desc = ar.availability_mode_desc, - secondary_lag_seconds = hdrs.secondary_lag_seconds + secondary_lag_seconds = hdrs.secondary_lag_seconds, + last_commit_time = hdrs.last_commit_time, + last_hardened_time = hdrs.last_hardened_time, + last_redone_time = hdrs.last_redone_time, + last_received_time = hdrs.last_received_time, + est_redo_completion_time_min = CONVERT(float, (hdrs.redo_queue_size * 1.0 / NULLIF(hdrs.redo_rate, 0)) / 60.0), + est_send_drain_time_min = CONVERT(float, (hdrs.log_send_queue_size * 1.0 / NULLIF(hdrs.log_send_rate, 0)) / 60.0) FROM sys.dm_hadr_database_replica_states AS hdrs JOIN sys.availability_replicas AS ar ON hdrs.replica_id = ar.replica_id @@ -206,6 +212,12 @@ public void DatabasePayloadColumns_AreInAppendOrder_WithTheDeclaredTypes() "suspend_reason_desc", "availability_mode_desc", "secondary_lag_seconds", + "last_commit_time", + "last_hardened_time", + "last_redone_time", + "last_received_time", + "est_redo_completion_time_min", + "est_send_drain_time_min", }, columns.Select(c => c.Name).ToArray()); @@ -221,6 +233,34 @@ public void DatabasePayloadColumns_AreInAppendOrder_WithTheDeclaredTypes() { Assert.Equal(CollectorColumnType.BigInt, columns.Single(c => c.Name == numeric).Type); } + + /* The four DMV timestamps the reference query skips. */ + foreach (var time in new[] { "last_commit_time", "last_hardened_time", "last_redone_time", "last_received_time" }) + { + Assert.Equal(CollectorColumnType.Timestamp, columns.Single(c => c.Name == time).Type); + } + + /* The drain-time estimates are CONVERT(float, ...) server-side — Double, not Decimal: without the + explicit CONVERT the expression's numeric type would come back as decimal and GetDouble throws. */ + foreach (var estimate in new[] { "est_redo_completion_time_min", "est_send_drain_time_min" }) + { + Assert.Equal(CollectorColumnType.Double, columns.Single(c => c.Name == estimate).Type); + } + } + + [Fact] + public void DrainTimeEstimates_GuardIntegerDivisionAndZeroRate() + { + /* Two guards, both load-bearing, both easy to drop in a reformat: `* 1.0` stops BIGINT / BIGINT + integer division (which would floor a 0.4-minute drain to 0), and NULLIF(rate, 0) stops the + divide-by-zero that an idle or suspended replica — rate 0 — would otherwise raise, failing the + whole collection cycle rather than one column. */ + var text = AgDatabaseReplicaStatesCollector.Instance.BuildQuery(CollectorTestContext.Make(new RecordingCollectorDeltaCalculator())).Text; + + Assert.Contains("(hdrs.redo_queue_size * 1.0 / NULLIF(hdrs.redo_rate, 0)) / 60.0", text, StringComparison.Ordinal); + Assert.Contains("(hdrs.log_send_queue_size * 1.0 / NULLIF(hdrs.log_send_rate, 0)) / 60.0", text, StringComparison.Ordinal); + Assert.Contains("est_redo_completion_time_min = CONVERT(float,", text, StringComparison.Ordinal); + Assert.Contains("est_send_drain_time_min = CONVERT(float,", text, StringComparison.Ordinal); } [Fact] @@ -272,6 +312,12 @@ public async Task DatabaseReadAsync_MapsColumns() "37000000045600001", "37000000045600002", 4096L, 2048L, 512L, 256L, false, DBNull.Value, "SYNCHRONOUS_COMMIT", 12L, + new DateTime(2026, 7, 26, 12, 0, 0), new DateTime(2026, 7, 26, 12, 0, 1), + new DateTime(2026, 7, 26, 11, 59, 55), new DateTime(2026, 7, 26, 12, 0, 2), + /* The two drain estimates are computed SERVER-side, so the fake supplies them directly and + they need not agree with the queue/rate columns above. Deliberately different values so a + swapped mapping between the two fails rather than passing on a coincidence. */ + 8.0d, 0.5d, }); var context = CollectorTestContext.Make(new RecordingCollectorDeltaCalculator()); @@ -294,6 +340,13 @@ public async Task DatabaseReadAsync_MapsColumns() Assert.Null(row.SuspendReasonDesc); Assert.Equal("SYNCHRONOUS_COMMIT", row.AvailabilityModeDesc); Assert.Equal(12L, row.SecondaryLagSeconds); + Assert.Equal(new DateTime(2026, 7, 26, 12, 0, 0), row.LastCommitTime); + Assert.Equal(new DateTime(2026, 7, 26, 12, 0, 1), row.LastHardenedTime); + Assert.Equal(new DateTime(2026, 7, 26, 11, 59, 55), row.LastRedoneTime); + Assert.Equal(new DateTime(2026, 7, 26, 12, 0, 2), row.LastReceivedTime); + + Assert.Equal(8.0d, row.EstRedoCompletionTimeMin); + Assert.Equal(0.5d, row.EstSendDrainTimeMin); } [Fact] @@ -301,7 +354,9 @@ public async Task DatabaseReadAsync_ToleratesNulls() { /* Every payload column of sys.dm_hadr_database_replica_states is documented nullable — last_hardened_lsn is explicitly NULL on an async-commit primary, and the queues/rates/lag - are null on a replica that is not reporting. */ + are null on a replica that is not reporting. The two drain estimates are ALWAYS null on this + shape: NULLIF(rate, 0) makes a null or zero rate produce a null estimate rather than a + divide-by-zero, which is the whole point of the guard. */ using var reader = new FakeCollectorDataReader( new object[] { @@ -309,6 +364,8 @@ are null on a replica that is not reporting. */ DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, "ASYNCHRONOUS_COMMIT", DBNull.Value, + DBNull.Value, DBNull.Value, DBNull.Value, DBNull.Value, + DBNull.Value, DBNull.Value, }); var context = CollectorTestContext.Make(new RecordingCollectorDeltaCalculator()); @@ -326,6 +383,14 @@ are null on a replica that is not reporting. */ Assert.Null(row.IsSuspended); Assert.Null(row.SuspendReasonDesc); Assert.Null(row.SecondaryLagSeconds); + Assert.Null(row.LastCommitTime); + Assert.Null(row.LastHardenedTime); + Assert.Null(row.LastRedoneTime); + Assert.Null(row.LastReceivedTime); + + /* NULL, not 0 — an un-drainable queue must never read as "drains instantly". */ + Assert.Null(row.EstRedoCompletionTimeMin); + Assert.Null(row.EstSendDrainTimeMin); } [Fact] @@ -359,14 +424,28 @@ public void WritePayload_EmitsPayloadOrder_AndTakesNoDeltas() new object?[] { "AG1", "NODE1", "PRIMARY", "ONLINE", "CONNECTED", "ONLINE", "HEALTHY", "SYNCHRONOUS_COMMIT", "AUTOMATIC", null }, replicaWriter.Values); + /* Modeled on a real measured sample from the Docker AG fixture: a SUSPEND_FROM_USER replica 62 s + into suspension. Note the lag is 62, NOT 0 — the docs claim lag reads 0 while suspended, and the + live instance does the opposite. A send-drain estimate of null is the matching reality: the send + queue goes NULL while suspended, so there is nothing to divide. */ var databaseWriter = new RecordingCollectorRowWriter(); AgDatabaseReplicaStatesCollector.Instance.WritePayload( - new AgDatabaseReplicaStatesCollector.Row("AG1", "Orders", "NODE2", false, "SYNCHRONIZING", "1", "2", 4096L, 2048L, 512L, 256L, true, "SUSPEND_FROM_USER", "SYNCHRONOUS_COMMIT", 0L), + new AgDatabaseReplicaStatesCollector.Row( + "AG1", "Orders", "NODE2", false, "SYNCHRONIZING", "1", "2", 4096L, 2048L, 512L, 256L, true, "SUSPEND_FROM_USER", "SYNCHRONOUS_COMMIT", 62L, + new DateTime(2026, 7, 26, 12, 0, 0), new DateTime(2026, 7, 26, 12, 0, 1), + new DateTime(2026, 7, 26, 11, 59, 55), new DateTime(2026, 7, 26, 12, 0, 2), + 8.0d, null), databaseWriter, context); Assert.Equal( - new object?[] { "AG1", "Orders", "NODE2", false, "SYNCHRONIZING", "1", "2", 4096L, 2048L, 512L, 256L, true, "SUSPEND_FROM_USER", "SYNCHRONOUS_COMMIT", 0L }, + new object?[] + { + "AG1", "Orders", "NODE2", false, "SYNCHRONIZING", "1", "2", 4096L, 2048L, 512L, 256L, true, "SUSPEND_FROM_USER", "SYNCHRONOUS_COMMIT", 62L, + new DateTime(2026, 7, 26, 12, 0, 0), new DateTime(2026, 7, 26, 12, 0, 1), + new DateTime(2026, 7, 26, 11, 59, 55), new DateTime(2026, 7, 26, 12, 0, 2), + 8.0d, null, + }, databaseWriter.Values); /* The queues, rates and lag are instantaneous gauges, NOT counters: neither collector may diff --git a/Lite.Tests/GoldenCollectorSchema.cs b/Lite.Tests/GoldenCollectorSchema.cs index 26f8250cf..d63afdb9a 100644 --- a/Lite.Tests/GoldenCollectorSchema.cs +++ b/Lite.Tests/GoldenCollectorSchema.cs @@ -880,7 +880,13 @@ endpoint_url VARCHAR is_suspended BOOLEAN, suspend_reason_desc VARCHAR, availability_mode_desc VARCHAR, - secondary_lag_seconds BIGINT + secondary_lag_seconds BIGINT, + last_commit_time TIMESTAMP, + last_hardened_time TIMESTAMP, + last_redone_time TIMESTAMP, + last_received_time TIMESTAMP, + est_redo_completion_time_min DOUBLE, + est_send_drain_time_min DOUBLE )", }; diff --git a/Lite.Tests/PerfmonAndDmvBlockingCollectorDefinitionTests.cs b/Lite.Tests/PerfmonAndDmvBlockingCollectorDefinitionTests.cs index 7f5321114..72fa5a4ad 100644 --- a/Lite.Tests/PerfmonAndDmvBlockingCollectorDefinitionTests.cs +++ b/Lite.Tests/PerfmonAndDmvBlockingCollectorDefinitionTests.cs @@ -24,10 +24,17 @@ public sealed class PerfmonStatsCollectorDefinitionTests [Fact] public void DefaultCounterList_IsTheCuratedParityContract() { - Assert.Equal(59, PerfmonStatsCollector.DefaultCounters.Count); + Assert.Equal(61, PerfmonStatsCollector.DefaultCounters.Count); Assert.Contains("Batch Requests/sec", PerfmonStatsCollector.DefaultCounters); Assert.Contains("Number of Deadlocks/sec", PerfmonStatsCollector.DefaultCounters); Assert.Contains("Wait for the worker", PerfmonStatsCollector.DefaultCounters); + + /* #991: the two SQLServer:Database Replica counters that carry the PRIMARY side of AG commit + latency. Their ratio (delay per mirrored transaction) is the number sync-commit conversations + are actually about, and neither name occurs on any other perfmon object, so the collector's + counter_name-only filter is safe without an object_name predicate. */ + Assert.Contains("Transaction Delay", PerfmonStatsCollector.DefaultCounters); + Assert.Contains("Mirrored Write Transactions/sec", PerfmonStatsCollector.DefaultCounters); } [Fact] diff --git a/PerformanceMonitor.Collectors/AgDatabaseReplicaStatesCollector.cs b/PerformanceMonitor.Collectors/AgDatabaseReplicaStatesCollector.cs index cb7ce8a66..644d88ec6 100644 --- a/PerformanceMonitor.Collectors/AgDatabaseReplicaStatesCollector.cs +++ b/PerformanceMonitor.Collectors/AgDatabaseReplicaStatesCollector.cs @@ -6,6 +6,7 @@ * Licensed under the MIT License. See LICENSE file in the project root for full license information. */ +using System; using System.Collections.Generic; using System.Data.Common; using System.Threading; @@ -33,15 +34,38 @@ namespace PerformanceMonitor.Collectors; /// Deriving a byte distance between them is ANALYSIS, deliberately left to a reader. /// /// secondary_lag_seconds is 2016+ and the repo floor IS 2016, so it is referenced directly -/// with no version branch. MS Learn documents it reading 0 — not NULL — while data movement -/// is SUSPENDED, so a suspended replica presents as zero lag: any later lag analysis has to read -/// is_suspended alongside it or it will under-report the worst case. Both columns are collected here -/// so that reading is possible. +/// with no version branch. MEASURED BEHAVIOR, which contradicts the docs: MS Learn says it "shows as 0 +/// if the data movement is suspended", but on a live SQL Server 2022 (16.0.4265.3) CLUSTER_TYPE = NONE +/// AG it does the inverse — it reads 0 while movement is ACTIVE and caught up, and accrues monotonically +/// once SUSPENDED (0 → 15 → 31 → 46 → 62 s across a 60 s SUSPEND_FROM_USER, back to 0 on resume). So a +/// suspended replica does NOT hide as zero lag, and a lag threshold fires on its own. Read is_suspended +/// alongside it to explain WHY lag is climbing, not to catch lag that is being masked. (Validated on a +/// clusterless AG on one build; WSFC untested, so treat the doc sentence as unreliable rather than +/// inverted-everywhere.) +/// +/// Two more measured quirks of the SUSPENDED state, both relevant to anyone thresholding these: +/// log_send_queue_size goes NULL rather than growing, while redo_queue_size FREEZES at its last value — +/// so a redo-queue reading on a suspended replica is stale, not current. +/// +/// GRAIN WARNING: on a SECONDARY, sys.dm_hadr_database_replica_states carries only the LOCAL +/// replica's rows, so this INNER JOIN narrows to a one-row self-view even though +/// sys.availability_replicas holds every replica. A complete AG picture requires collecting from the +/// PRIMARY; monitoring only a secondary yields that replica's own view and nothing about its peers. /// /// synchronization_state_desc's values are space-separated (NOT SYNCHRONIZING), unlike /// the replica-grain synchronization_health_desc's underscores (NOT_HEALTHY) — stored /// verbatim as the DMV reports them. /// +/// The four *_time columns are the commit/hardened/redone/received timestamps the reference +/// project skips; they are what the canonical primary-vs-secondary commit-time lag math needs, and +/// unlike secondary_lag_seconds they are directly comparable across replicas. The two +/// est_*_time_min columns are drain-time ESTIMATES computed server-side: queue ÷ rate ÷ 60. Both +/// guard the two ways the raw expression misbehaves — * 1.0 stops BIGINT ÷ BIGINT integer +/// division, and NULLIF(rate, 0) stops the divide-by-zero an idle or suspended replica would +/// otherwise raise. A NULL estimate therefore means "no drain rate" (idle, suspended, or caught up), +/// which is the honest answer; it is deliberately never coerced to 0, which would read as "drains +/// instantly" — the exact opposite of the truth. +/// /// PERMISSIONS: this query joins the same two AG catalog views, which require VIEW ANY /// DEFINITION and hide rows rather than erroring without it — see /// for the full trap and the fingerprint that identifies it. @@ -69,7 +93,13 @@ public readonly record struct Row( bool? IsSuspended, string? SuspendReasonDesc, string? AvailabilityModeDesc, - long? SecondaryLagSeconds); + long? SecondaryLagSeconds, + DateTime? LastCommitTime, + DateTime? LastHardenedTime, + DateTime? LastRedoneTime, + DateTime? LastReceivedTime, + double? EstRedoCompletionTimeMin, + double? EstSendDrainTimeMin); private const string QueryText = @" SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; @@ -89,7 +119,13 @@ public readonly record struct Row( is_suspended = hdrs.is_suspended, suspend_reason_desc = hdrs.suspend_reason_desc, availability_mode_desc = ar.availability_mode_desc, - secondary_lag_seconds = hdrs.secondary_lag_seconds + secondary_lag_seconds = hdrs.secondary_lag_seconds, + last_commit_time = hdrs.last_commit_time, + last_hardened_time = hdrs.last_hardened_time, + last_redone_time = hdrs.last_redone_time, + last_received_time = hdrs.last_received_time, + est_redo_completion_time_min = CONVERT(float, (hdrs.redo_queue_size * 1.0 / NULLIF(hdrs.redo_rate, 0)) / 60.0), + est_send_drain_time_min = CONVERT(float, (hdrs.log_send_queue_size * 1.0 / NULLIF(hdrs.log_send_rate, 0)) / 60.0) FROM sys.dm_hadr_database_replica_states AS hdrs JOIN sys.availability_replicas AS ar ON hdrs.replica_id = ar.replica_id @@ -138,6 +174,15 @@ ORDER BY new CollectorColumn("suspend_reason_desc", CollectorColumnType.Varchar), new CollectorColumn("availability_mode_desc", CollectorColumnType.Varchar), new CollectorColumn("secondary_lag_seconds", CollectorColumnType.BigInt), + /* Appended, never inserted (#991 addendum): an upgraded store gets these via ALTER TABLE ADD + COLUMN, which appends physically, so a fresh store's generated shape only matches if they + are last here too. */ + new CollectorColumn("last_commit_time", CollectorColumnType.Timestamp), + new CollectorColumn("last_hardened_time", CollectorColumnType.Timestamp), + new CollectorColumn("last_redone_time", CollectorColumnType.Timestamp), + new CollectorColumn("last_received_time", CollectorColumnType.Timestamp), + new CollectorColumn("est_redo_completion_time_min", CollectorColumnType.Double), + new CollectorColumn("est_send_drain_time_min", CollectorColumnType.Double), }; public override async ValueTask> ReadAsync(DbDataReader reader, CollectorContext context, CancellationToken cancellationToken) @@ -161,7 +206,13 @@ public override async ValueTask> ReadAsync(DbDataReader reader, Collec IsSuspended: reader.IsDBNull(11) ? null : reader.GetBoolean(11), SuspendReasonDesc: reader.IsDBNull(12) ? null : reader.GetString(12), AvailabilityModeDesc: reader.IsDBNull(13) ? null : reader.GetString(13), - SecondaryLagSeconds: reader.IsDBNull(14) ? null : reader.GetInt64(14))); + SecondaryLagSeconds: reader.IsDBNull(14) ? null : reader.GetInt64(14), + LastCommitTime: reader.IsDBNull(15) ? null : reader.GetDateTime(15), + LastHardenedTime: reader.IsDBNull(16) ? null : reader.GetDateTime(16), + LastRedoneTime: reader.IsDBNull(17) ? null : reader.GetDateTime(17), + LastReceivedTime: reader.IsDBNull(18) ? null : reader.GetDateTime(18), + EstRedoCompletionTimeMin: reader.IsDBNull(19) ? null : reader.GetDouble(19), + EstSendDrainTimeMin: reader.IsDBNull(20) ? null : reader.GetDouble(20))); } return rows; @@ -184,6 +235,12 @@ public override void WritePayload(Row row, ICollectorRowWriter writer, Collector .Value(row.IsSuspended) /* is_suspended BOOLEAN */ .Value(row.SuspendReasonDesc) /* suspend_reason_desc VARCHAR */ .Value(row.AvailabilityModeDesc) /* availability_mode_desc VARCHAR */ - .Value(row.SecondaryLagSeconds); /* secondary_lag_seconds BIGINT (s) */ + .Value(row.SecondaryLagSeconds) /* secondary_lag_seconds BIGINT (s) */ + .Value(row.LastCommitTime) /* last_commit_time TIMESTAMP */ + .Value(row.LastHardenedTime) /* last_hardened_time TIMESTAMP */ + .Value(row.LastRedoneTime) /* last_redone_time TIMESTAMP */ + .Value(row.LastReceivedTime) /* last_received_time TIMESTAMP */ + .Value(row.EstRedoCompletionTimeMin) /* est_redo_completion_time_min DOUBLE (min) */ + .Value(row.EstSendDrainTimeMin); /* est_send_drain_time_min DOUBLE (min) */ } } diff --git a/PerformanceMonitor.Collectors/PerfmonStatsCollector.cs b/PerformanceMonitor.Collectors/PerfmonStatsCollector.cs index 3fd3a8638..afc03bee6 100644 --- a/PerformanceMonitor.Collectors/PerfmonStatsCollector.cs +++ b/PerformanceMonitor.Collectors/PerfmonStatsCollector.cs @@ -103,6 +103,14 @@ private PerfmonStatsCollector() /* Wait counters */ "Network IO waits", "Wait for the worker", + /* Availability Group counters (SQLServer:Database Replica object, #991). Both names are unique + to that object, so the collector's counter_name-only filter picks them up without an + object_name predicate. Transaction Delay / Mirrored Write Transactions/sec is the average + primary-side commit delay per mirrored transaction — the primary-side half of the AG latency + picture the ag_database_replica_states gauges cover from the secondary side. Absent on an + instance without AGs, which is simply fewer rows. */ + "Transaction Delay", + "Mirrored Write Transactions/sec", }; public override string Name => "perfmon_stats"; diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index b0298aedf..5a61436e1 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -159,6 +159,42 @@ Full license: https://github.com/microsoft/vscode-mssql/blob/main/LICENSE --- +## SqlServerAgMonitor + +**Author**: Hannah Vernon +**Repository**: https://github.com/HannahVernon/SqlServerAgMonitor +**License**: MIT License + +The Availability Group health collector's DMV metric surface (replica states + database replica states) follows SqlServerAgMonitor's monitoring queries. + +### License Text + +MIT License + +Copyright (c) 2026 Hannah Vernon + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +Full license: https://github.com/HannahVernon/SqlServerAgMonitor/blob/main/LICENSE + +--- + ## Acknowledgments Performance Monitor would not be possible without the excellent work of: @@ -168,6 +204,7 @@ Performance Monitor would not be possible without the excellent work of: - **Brent Ozar Unlimited** for the First Responder Kit and comprehensive SQL Server diagnostics - **Microsoft** for vscode-mssql execution plan operator icons used in the Plan Viewer +- **Hannah Vernon** for SqlServerAgMonitor, whose queries shape the Availability Group health collector We are grateful for their contributions to the SQL Server community and their commitment to open-source software. From cc8ec59d39141733362befc94bf36b771b2b64f7 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:43:11 -0400 Subject: [PATCH 2/3] Make the live-store version assertion gap-tolerant The end-to-end Postgres test asserted COUNT(*) FROM darling_schema_version == StorageVersion.SchemaVersion. That identity only holds while migration versions are DENSE from 1, so the first concurrently-developed pair of migrations broke it: V35 (AG alerts) and V36 (AG latency) are being built on separate branches, and V36 alone leaves a temporary gap. The gap is inert to the applier - MigrateAsync applies every script whose version exceeds MAX(version) and never assumes contiguity - so the proxy was the only thing that cared. Replaced with the two invariants it was conflating, which together are strictly stronger: MAX(version) == SchemaVersion (the store reached this build's version, the same expression MigrateAsync itself reads) and COUNT(*) == PgMigrations.Scripts.Count (every script ran, one stamped row apiece, so a silently skipped script still fails). Co-Authored-By: Claude Fable 5 --- .../DarlingAnalysisStoreTests.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Darling/Darling.Tests/DarlingAnalysisStoreTests.cs b/Darling/Darling.Tests/DarlingAnalysisStoreTests.cs index ad33d5049..78eaff824 100644 --- a/Darling/Darling.Tests/DarlingAnalysisStoreTests.cs +++ b/Darling/Darling.Tests/DarlingAnalysisStoreTests.cs @@ -8,6 +8,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Threading.Tasks; using Npgsql; @@ -239,12 +240,22 @@ public async Task EndToEnd_FindingStoreRoundTrip_AgainstDevPostgres() /* Migrations are idempotent — an older store comes up to current, a current store no-ops. */ await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); + /* Two separate invariants, because the single COUNT(*) == SchemaVersion check they replace conflated + them: it only holds while migration versions are DENSE from 1, so the first concurrently-developed + pair of migrations (V35 alerts / V36 AG latency, built on separate branches) broke it with a + temporary gap that is completely inert to the applier — MigrateAsync applies every script whose + version exceeds MAX(version) and never assumes contiguity. Assert what actually matters instead, + which is also strictly stronger than the old proxy. */ + using (var maxVersion = new NpgsqlCommand("SELECT COALESCE(MAX(version), 0) FROM darling_schema_version", connection)) + { + /* The store reached the version this build knows — the same expression MigrateAsync reads. */ + Assert.Equal(StorageVersion.SchemaVersion, Convert.ToInt32(await maxVersion.ExecuteScalarAsync(TestContext.Current.CancellationToken), CultureInfo.InvariantCulture)); + } + using (var versions = new NpgsqlCommand("SELECT COUNT(*) FROM darling_schema_version", connection)) { - /* One row per applied migration, so the count tracks the current schema version. Reference the - constant rather than a literal — this was hardcoded to a since-superseded number (15) and, as a - live-only assertion CI skips, silently rotted until the schema reached V17. */ - Assert.Equal((long)StorageVersion.SchemaVersion, await versions.ExecuteScalarAsync(TestContext.Current.CancellationToken)); + /* And EVERY script ran, one stamped row apiece — so a script silently skipped still fails here. */ + Assert.Equal((long)PgMigrations.Scripts.Count, await versions.ExecuteScalarAsync(TestContext.Current.CancellationToken)); } /* Clear leftovers from an earlier aborted run so the assertions below are deterministic. */ From 224df8f108d2326762447ad1e28c599d3dc3083c Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:44:25 -0400 Subject: [PATCH 3/3] Add the AG Health seed notebook, and make the template drift guard cover every template (#991) The fifth Custom Views v2 seed, built on the AG measures from #1688/#1695. Five panels in diagnostic order: 1. Secondary lag over time, grouped by replica. 2. Dual-axis line: log send rate vs redo rate on their own axes - the panel that answers which side is the bottleneck. 3. Stacked redo-queue series by database. 4. Stat tile: worst estimated redo drain. 5. Top-10 bar of send-queue backlog by database. The prose between panels carries the two things that make the numbers readable: point the view at the PRIMARY (sys.dm_hadr_* on a secondary only carries the local replica, so a secondary-scoped view is a one-row self-view), and a blank drain estimate means no drain rate - idle, caught up, or suspended - not that it drains instantly. Adds the two template helpers the existing seeds never needed: overlay support on tsPanel (dual-axis, legal only on an ungrouped line/area) and a scalar statPanel. This is the first seed exercising dual-axis, stacked and stat modes end to end. The brief asked for span 2 on the lag panel; notebooks are single-column documents and the renderer strips span, so it is omitted rather than stored as a field nothing honors. Also closes a hole in the templates drift guard. It hand-mirrors each template's panels so a MeasureCatalog change that breaks a seed fails in CI, but nothing checked the mirror COVERED every template - a sixth template added without a mirror entry would have gone silently unvalidated until it 400'd in a browser. It now reads the template keys out of notebook.js and requires the mirror to match exactly. Verified by renaming a key and watching it go red, and the new mirror was checked against the real module output by executing notebook.js under node and diffing the emitted panel JSON. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 + Darling/Darling.Tests/DarlingComposeTests.cs | 49 ++++++++ .../wwwroot/js/notebook.js | 107 ++++++++++++++++++ 3 files changed, 158 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c5cae3ee..49101a9e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Darling Web: an "AG Health" seed notebook** ([#1699]) - the fifth Custom Views v2 seed template, built on the Availability Group measures from [#1688] and [#1695]. Five panels in diagnostic order: secondary lag over time grouped by replica; a **dual-axis** line putting log send rate against redo rate on their own axes (the one panel that answers "which side is the bottleneck" - send outpacing redo means the secondary is receiving faster than it can replay, and failover time is growing); a **stacked** redo-queue series by database; a **stat tile** for the worst estimated redo drain; and a top-10 bar of send-queue backlog by database. The prose between panels carries the two things that make the numbers readable rather than merely present: point the view at the PRIMARY (a secondary only ever sees its own replica in `sys.dm_hadr_*`, so a secondary-scoped view is a one-row self-view), and a blank drain estimate means there is no drain rate - idle, caught up, or suspended - not that it drains instantly. Also adds the two template helpers the existing seeds never needed, `overlay` support on the time-series builder and a scalar `statPanel`, so this is the first seed exercising dual-axis, stacked and stat modes end to end. **The templates drift-guard got a real hole closed along the way**: it hand-mirrors each template's panels for validation, but nothing checked the mirror COVERED every template, so a sixth template added without a mirror entry would have gone silently unvalidated until it 400'd in someone's browser. It now reads the template keys out of `notebook.js` and requires the mirror to match exactly, verified by renaming a key and watching it go red. - **AG latency: commit-time columns, drain-time estimates, and the primary-side perfmon counters** ([#1695]) - completes the Availability Group latency picture [#1688] started. `ag_database_replica_states` gains **four DMV timestamps the reference query skips** (`last_commit_time`, `last_hardened_time`, `last_redone_time`, `last_received_time`) - unlike `secondary_lag_seconds` these are directly comparable across replicas, which is what the canonical primary-vs-secondary commit-time lag math needs - plus **two server-computed drain-time estimates**, `est_redo_completion_time_min` and `est_send_drain_time_min` (queue / rate / 60). Both estimates carry the two guards the raw expression needs: `* 1.0` stops BIGINT-over-BIGINT integer division flooring a sub-minute drain to zero, and `NULLIF(rate, 0)` stops the divide-by-zero an idle or suspended replica would otherwise raise - which would fail the whole cycle, not one column. A NULL estimate honestly means "no drain rate" and is never coerced to 0, which would read as "drains instantly". They are computed per row at the sample's own instant rather than composed later as a ratio of two window averages, because avg(queue)/avg(rate) is not the average of the per-sample ratios and the two diverge worst exactly when rates swing. Both are Gauge compose measures (duration family, native minutes). **Two perfmon counters** join the existing whitelist - `Transaction Delay` and `Mirrored Write Transactions/sec` - carrying the PRIMARY side of commit latency (their ratio is the average delay per mirrored transaction); zero new schema, they ride `perfmon_stats`. Verified on SQL2022 that both exist even with no AGs configured, live on `SQLServer:Database Replica`, and occur exactly once server-wide, so the collector's counter_name-only filter cannot collide. Together with `HADR_SYNC_COMMIT`, which already flows through `wait_stats`, sync-commit pressure is now composable end to end: primary-side delay, secondary-side queues and rates, drain estimates, and the wait itself. **A shipped doc claim is corrected against a live AG.** [#1688] restated MS Learn's assertion that `secondary_lag_seconds` reads 0 while data movement is suspended; a Docker AG fixture measured the inverse on SQL Server 2022 in a `CLUSTER_TYPE = NONE` group - it reads 0 while movement is ACTIVE and caught up, and accrues monotonically once suspended (0 to 62 s across a 60 s `SUSPEND_FROM_USER`, back to 0 on resume). So a suspended replica does not hide as zero lag and a lag threshold fires on its own; reading `is_suspended` alongside explains WHY lag is climbing rather than catching lag that is masked. Two further measured quirks are now documented: `log_send_queue_size` goes NULL while suspended while `redo_queue_size` FREEZES at its last value (so a redo-queue reading on a suspended replica is stale, not current), and collecting from a SECONDARY yields a one-row self-view because `sys.dm_hadr_*` carries only the local replica there - a complete AG picture requires collecting from the primary. Store migration V36 appends the six columns additively rather than widening V34 in place, because V34's `CREATE TABLE IF NOT EXISTS` is a no-op on an already-migrated store and editing it would silently leave every existing store short the new columns while fresh installs got them; the schema pin now reconstructs the current shape from V34 + V36 and compares it to the generator, with both failure modes confirmed by planted defects. Hannah Vernon's SqlServerAgMonitor is now credited in `THIRD_PARTY_NOTICES.md` alongside the existing collector-header attribution. - **AG collection: document the second grant it needs, and make the lag trap filterable** ([#1691]) - review follow-ups to [#1688]. **The grant is the one that matters in the field.** Both AG collectors join the `sys.availability_groups` / `sys.availability_replicas` CATALOG VIEWS to the `sys.dm_hadr_*` DMVs, and while the DMVs are covered by the `VIEW SERVER STATE` the product asks for, [the catalog views require `VIEW ANY DEFINITION`](https://learn.microsoft.com/en-us/sql/database-engine/availability-groups/windows/monitor-availability-groups-transact-sql) - which catalog views enforce by HIDING ROWS, not by raising an error. So on a fully configured AG cluster a monitoring login with only the documented grant returned zero rows, which is exactly what an AG-less server returns: the collectors would have looked healthy forever while collecting nothing, with no error anywhere to notice. Now called out in both READMEs (the Lite/Darling grant script, the Darling permission table's If-missing column) and in both collector headers, along with the fingerprint that identifies it if it is ever worth detecting automatically - the DMV returning rows while the catalog view returns none is unambiguous, and `SERVERPROPERTY('IsHadrEnabled')` is readable by every login. **The documented lag trap is now actionable instead of just documented**: `secondary_lag_seconds` reads 0 rather than NULL while data movement is suspended, so a suspended replica charts as perfectly healthy - but nothing exposed the suspension state to a panel, so the misread the code comment warned about was unavoidable. `synchronization_state_desc` and `suspend_reason_desc` are now compose dimensions, so a lag panel can filter suspended replicas out or group by suspend reason. (`is_suspended` itself cannot be a dimension: the compiler binds filter values as text, which would not match a boolean column.) **And the V34 migration is now genuinely pinned.** Its test asserted only that each column NAME appeared somewhere in the DDL - a V34 with the columns reordered, `is_local` typed `text`, or a spurious `NOT NULL` passed every test, while a comment claimed the shape was pinned elsewhere. It now compares the whole generated `CREATE TABLE` via the existing `Migrations_JobHistoryAndAgentStatus_MatchGeneratedFreshShape` idiom; detection power confirmed by planting `is_local text` and watching it go red. That test's long-standing rationale was corrected too - it blamed a "positional binary COPY", but `PgCollectorRowWriter.CopyCommandFor` emits a named column list so Postgres binds by name; names and types are the real hazard on that side, and the positional appender is Lite's. - **Availability Group health collection in Lite + Darling** ([#1688]) - closes #991 and the AG item of #1606, the one real coverage gap the Datadog DBM comparison turned up. Two new shared collectors, both server-scope and both zero-cost on a server without Always On: **`ag_replica_states`** (replica grain - role, operational/connected state, recovery and synchronization health, availability and failover mode, endpoint URL) from `sys.availability_replicas` joined to `sys.availability_groups` and `sys.dm_hadr_availability_replica_states`, and **`ag_database_replica_states`** (database grain - synchronization state, log send and redo queue sizes, send and redo rates, secondary lag, suspension state and reason, and both LSNs) from `sys.dm_hadr_database_replica_states`. The metric surface follows Hannah Vernon's [SqlServerAgMonitor](https://github.com/HannahVernon/SqlServerAgMonitor) (MIT), attributed in both collector headers. `WHERE COALESCE(is_distributed, 0) = 0` keeps distributed-AG container rows out while member AGs still appear; drilling into a DAG's remote members needs a connection per member and is out of scope. Runs everywhere except Azure SQL DB, which has no AG surface - an AG-less on-prem server is deliberately NOT gated off, it collects and stores zero rows, so turning Always On on later starts producing data with no configuration change. Three details that are easy to get wrong and are pinned by test: the queues, rates and lag are **instantaneous gauges, not counters**, so neither collector touches the delta framework and the compose measures are Gauge (avg/min/max, never SUM - summing a backlog over a window is a category error); `last_hardened_lsn` / `last_commit_lsn` are `numeric(25, 0)`, **wider than BIGINT**, so they are converted server-side and stored as text rather than silently overflowing; and every column is read null-tolerantly, because under WSFC quorum loss `sys.availability_replicas` serves only locally cached metadata and `endpoint_url` is documented NULL. Both apps schedule it per minute with 30-day retention (the grain at which a lag or queue spike is still visible), and it is in all three cadence presets (per-minute on Aggressive and Balanced, 5 min on Low-Impact). **Custom Views v2** gets five Gauge measures on the database grain - send queue, redo queue, send rate, redo rate and secondary lag - under an Availability Groups category, sliceable and groupable by AG, database and replica plus the synchronization/suspend state (and the universal server dimension), which lights up send-rate-vs-redo-rate on a dual axis and worst-lag-per-replica out of the box. Only the database-grain table carries measures - the replica-grain table is all state strings with nothing numeric to aggregate, so it is stored for the coming viewer tab rather than queryable from Custom Views today. Darling store migration V34; Lite's storage registers itself off the collector catalog. Collection only in this first cut: no viewer tab and no failover / sync-fell-behind alerts yet, both tracked as follow-ups, and both apps' viewer-coverage ratchets carry the two tables as explicitly tracked debt so the tab cannot be forgotten. That ratchet also got a real fix along the way - it text-scans the viewer's reader layer for a table name, and naming a collector table as a schema-version probe sentinel (as V34 does) made the table read as "already covered", silently exempting it; the scan now strips the probe's `information_schema` lines, which retroactively hardens the same pin for `long_query_completions`. @@ -1659,4 +1660,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1688]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1688 [#1691]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1691 [#1695]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1695 +[#1699]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1699 [#1690]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1690 diff --git a/Darling/Darling.Tests/DarlingComposeTests.cs b/Darling/Darling.Tests/DarlingComposeTests.cs index c1d2c535c..5f1d48c71 100644 --- a/Darling/Darling.Tests/DarlingComposeTests.cs +++ b/Darling/Darling.Tests/DarlingComposeTests.cs @@ -8,8 +8,11 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Text.Json.Nodes; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Npgsql; @@ -1550,6 +1553,27 @@ are mirrored here verbatim (the markdown prose is abbreviated — ValidateDefini "{\"type\":\"markdown\",\"text\":\"## Top memory clerks\"}," + "{\"type\":\"panel\",\"source\":\"memory_clerks\",\"viz\":\"bar\",\"topN\":10,\"title\":\"Top memory clerks\",\"groupBy\":[\"clerk_type\"],\"measure\":\"clerk_memory_mb\",\"aggregate\":\"max\",\"unit\":\"mb\"}," + "{\"type\":\"markdown\",\"text\":\"RESOURCE_SEMAPHORE notes\"}]}"), + + /* #991: the AG Health seed. Exercises three shapes the other four seeds do not — a dual-axis + overlay (ungrouped line + second measure), a stacked time series, and a scalar stat tile — + so this entry is also the drift guard for those panel modes staying valid. */ + ("ag-health", + "{\"kind\":\"notebook\",\"cells\":[" + + "{\"type\":\"markdown\",\"text\":\"# Availability Group health\"}," + + "{\"type\":\"markdown\",\"text\":\"## 1. How far behind is each secondary?\"}," + + "{\"type\":\"panel\",\"source\":\"ag_database_replica_states\",\"viz\":\"line\",\"timeBucket\":\"hour\",\"title\":\"Secondary lag by replica\",\"measure\":\"ag_secondary_lag\",\"aggregate\":\"max\",\"unit\":\"s\",\"groupBy\":[\"replica_server_name\"]}," + + "{\"type\":\"markdown\",\"text\":\"Reading the lag panel\"}," + + "{\"type\":\"markdown\",\"text\":\"## 2. Send vs redo: which side is the bottleneck?\"}," + + "{\"type\":\"panel\",\"source\":\"ag_database_replica_states\",\"viz\":\"line\",\"timeBucket\":\"hour\",\"title\":\"Log send rate vs redo rate\",\"measure\":\"ag_log_send_rate\",\"aggregate\":\"avg\",\"unit\":\"kb\",\"overlay\":{\"measure\":\"ag_redo_rate\",\"aggregate\":\"avg\",\"unit\":\"kb\"}}," + + "{\"type\":\"markdown\",\"text\":\"Reading the rate panel\"}," + + "{\"type\":\"markdown\",\"text\":\"## 3. Where is the redo backlog?\"}," + + "{\"type\":\"panel\",\"source\":\"ag_database_replica_states\",\"viz\":\"stacked\",\"timeBucket\":\"hour\",\"title\":\"Redo queue by database\",\"measure\":\"ag_redo_queue\",\"aggregate\":\"max\",\"unit\":\"kb\",\"groupBy\":[\"database_name\"]}," + + "{\"type\":\"markdown\",\"text\":\"## 4. How long until the redo queue clears?\"}," + + "{\"type\":\"panel\",\"source\":\"ag_database_replica_states\",\"viz\":\"stat\",\"title\":\"Estimated redo drain (worst, minutes)\",\"measure\":\"ag_est_redo_drain_min\",\"aggregate\":\"max\",\"unit\":\"min\"}," + + "{\"type\":\"markdown\",\"text\":\"Blank means no drain rate\"}," + + "{\"type\":\"markdown\",\"text\":\"## 5. Which databases are backing up on the send side?\"}," + + "{\"type\":\"panel\",\"source\":\"ag_database_replica_states\",\"viz\":\"bar\",\"topN\":10,\"title\":\"Log send queue by database\",\"groupBy\":[\"database_name\"],\"measure\":\"ag_log_send_queue\",\"aggregate\":\"max\",\"unit\":\"kb\"}," + + "{\"type\":\"markdown\",\"text\":\"Next steps\"}]}"), }; foreach (var (name, definition) in templates) @@ -1557,6 +1581,31 @@ are mirrored here verbatim (the markdown prose is abbreviated — ValidateDefini var result = DarlingWebEndpoints.ValidateDefinition(definition); Assert.True(result.IsValid, $"seed template '{name}' failed validation: {result.Error}"); } + + /* The hole a hand-written mirror always has: it protects the templates it happens to list. Read the + REAL key list out of notebook.js and require the mirror to cover exactly it, so a sixth template + added without a mirror entry fails here instead of silently going unvalidated until it 400s in + someone's browser. Source-scanned rather than executed — Darling.Tests has no JS runtime, and the + same read-the-source idiom already backs the cross-app preset pin. */ + var declaredKeys = NotebookTemplateKeys(); + Assert.NotEmpty(declaredKeys); + Assert.Equal(declaredKeys, templates.Select(t => t.Name).OrderBy(k => k, StringComparer.Ordinal).ToArray()); + } + + /// Every key: declared in NOTEBOOK_TEMPLATES (wwwroot/js/notebook.js), sorted. + /// key: appears nowhere else in that file, so a plain line scan is unambiguous. + private static string[] NotebookTemplateKeys([CallerFilePath] string thisFile = "") + { + var testDir = Path.GetDirectoryName(thisFile)!; + var notebookJs = Path.GetFullPath(Path.Combine( + testDir, "..", "PerformanceMonitor.Darling.Service", "wwwroot", "js", "notebook.js")); + + Assert.True(File.Exists(notebookJs), $"notebook.js not found at {notebookJs} (did the frontend move?)"); + + return Regex.Matches(File.ReadAllText(notebookJs), @"^\s*key:\s*""([^""]+)""", RegexOptions.Multiline) + .Select(m => m.Groups[1].Value) + .OrderBy(k => k, StringComparer.Ordinal) + .ToArray(); } /* ─────────────── #1665: availability-gated routing + the partial-window notice ─────────────── */ diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/notebook.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/notebook.js index c94274e2f..31e5bc608 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/notebook.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/notebook.js @@ -438,11 +438,21 @@ function tsPanel(o) { const cell = { type: "panel", source: o.source, viz: o.viz || "line", timeBucket: o.timeBucket || "hour", title: o.title }; applyMetric(cell, o); if (Array.isArray(o.groupBy) && o.groupBy.length) cell.groupBy = o.groupBy; + /* The #1606 dual-axis second measure. Only legal on an UNGROUPED line/area (the server rejects it + anywhere else), so a template using it must not also set groupBy. */ + if (o.overlay) cell.overlay = o.overlay; if (Array.isArray(o.annotations) && o.annotations.length) cell.annotations = o.annotations; if (Array.isArray(o.thresholds) && o.thresholds.length) cell.thresholds = o.thresholds; return cell; } +/** A single-value (stat tile) panel cell: no timeBucket and no topN is what makes it scalar mode. */ +function statPanel(o) { + const cell = { type: "panel", source: o.source, viz: o.viz || "stat", title: o.title }; + applyMetric(cell, o); + return cell; +} + /** A ranked (top-N) panel cell. */ function rankedPanel(o) { const cell = { type: "panel", source: o.source, viz: o.viz || "bar", topN: o.topN || 10, title: o.title, groupBy: o.groupBy }; @@ -732,4 +742,101 @@ export const NOTEBOOK_TEMPLATES = [ }; }, }, + { + key: "ag-health", + label: "AG Health", + description: "Availability Group replication health: secondary lag, send vs redo throughput, queue backlogs, and drain estimates.", + make() { + return { + name: "AG Health", + description: "Availability Group replication health and latency.", + definition: { + kind: NOTEBOOK_KIND, + cells: [ + md( + "# Availability Group health\n\n" + + "How far behind the secondaries are, and why. Set the server and time range at the top to scope every " + + "panel — **point them at the PRIMARY**: `sys.dm_hadr_*` only carries the local replica's rows on a " + + "secondary, so a secondary-scoped view shows that one replica's self-view and nothing about its peers.\n\n" + + "Reading order: lag says *how bad*, the rate and queue panels say *why*, and the drain estimate says " + + "*how long until it clears at the current rate*." + ), + md("## 1. How far behind is each secondary?"), + tsPanel({ + title: "Secondary lag by replica", + source: "ag_database_replica_states", + measure: "ag_secondary_lag", + aggregate: "max", + unit: "s", + viz: "line", + groupBy: ["replica_server_name"], + }), + md( + "Lag climbing on one replica but not the others points at that node (network, disk, CPU); climbing on all " + + "of them points at the primary's log-generation rate. A replica whose data movement is **suspended** " + + "shows lag accruing here, so a spike is not automatically a performance problem — check the " + + "synchronization state before chasing it." + ), + md("## 2. Send vs redo: which side is the bottleneck?"), + tsPanel({ + title: "Log send rate vs redo rate", + source: "ag_database_replica_states", + measure: "ag_log_send_rate", + aggregate: "avg", + unit: "kb", + viz: "line", + overlay: { measure: "ag_redo_rate", aggregate: "avg", unit: "kb" }, + }), + md( + "Both are KB/second. Send outpacing redo means the secondary is receiving faster than it can replay — the " + + "redo queue grows and failover time with it. Redo at or above send means the secondary is keeping up. " + + "(Ungrouped by design: a dual-axis panel plots one series per axis.)" + ), + md("## 3. Where is the redo backlog?"), + tsPanel({ + title: "Redo queue by database", + source: "ag_database_replica_states", + measure: "ag_redo_queue", + aggregate: "max", + unit: "kb", + viz: "stacked", + groupBy: ["database_name"], + }), + md("## 4. How long until the redo queue clears?"), + statPanel({ + title: "Estimated redo drain (worst, minutes)", + source: "ag_database_replica_states", + measure: "ag_est_redo_drain_min", + aggregate: "max", + unit: "min", + }), + md( + "Queue ÷ rate at each sample's own instant, so it reflects the rate that actually applied. **Blank means " + + "there is no drain rate** — idle, caught up, or suspended — not that it drains instantly." + ), + md("## 5. Which databases are backing up on the send side?"), + rankedPanel({ + title: "Log send queue by database", + source: "ag_database_replica_states", + measure: "ag_log_send_queue", + aggregate: "max", + unit: "kb", + viz: "bar", + groupBy: ["database_name"], + topN: 10, + }), + md( + "### Next steps\n\n" + + "- Send-side backlog with a healthy redo rate is usually network or primary log throughput.\n" + + "- Redo-side backlog is the secondary: check its CPU, disk latency, and whether redo is single-threaded " + + "for that database.\n" + + "- For primary-side commit cost, chart `Transaction Delay` and `Mirrored Write Transactions/sec` from " + + "`perfmon_stats` — their ratio is the average delay per mirrored transaction — and cross-check " + + "`HADR_SYNC_COMMIT` in wait stats." + ), + ], + }, + }; + }, + }, ];