From ec20a08911c507cd23b88062aa6af1036b6fdf90 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:51:29 +0200 Subject: [PATCH 1/2] tempdb used% can no longer exceed 100 (#2169) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The viewer divides in-database FILEPROPERTY(SpaceUsed) by a total taken from sys.master_files.size — which records the size set at configuration and does NOT follow autogrowth for tempdb. A grown tempdb was measured against its startup size, so the ratio ran past 100%. The per-database probe already visits sys.database_files for SpaceUsed; it now also captures df.size, and the payload prefers that over master_files. Both operands come from one snapshot. COALESCE keeps the old source as fallback so a database whose probe failed still reports a total instead of dropping out of the grid. Scope: the on-prem/RDS/MI path. Azure SQL DB already read both numbers from sys.database_files, which is why the reporter's Managed Instance (it takes the cross-database path) hit this and a true Azure SQL DB would not. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 3 ++ .../DatabaseSizeCollectorDefinitionTests.cs | 52 +++++++++++++++++++ .../DatabaseSizeStatsCollector.cs | 18 +++++-- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b304ce68..f7691c9c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **tempdb no longer reports more than 100% used in FinOps Database Sizes** ([#2169], reported by @CatastropheOps) - the used percentage divided in-database usage by the size recorded in `sys.master_files`, which is the size set at configuration time and does not track autogrowth for tempdb. A tempdb that had grown was therefore measured against its startup size and rendered above 100%. The per-database probe now captures the file's current size in the same round trip it already makes for space-used, and the payload prefers it, so both halves of the ratio come from one snapshot; a database whose probe fails still falls back to the old source rather than vanishing from the grid. Affects the on-prem, RDS, and Managed Instance path - the Azure SQL Database path already read both numbers in-database. + - **Backing out of Custom Range no longer strands an open calendar - both apps** ([#2154], reported in #2153) - a DatePicker's calendar dropdown is a popup living outside the visual tree's visibility, so collapsing the pickers when the user switched back to a preset range left an already-open calendar floating on screen; the dropdowns now close explicitly alongside the collapse, in Lite's ServerTab and the Darling Viewer's twin alike. - **One wedged background task can no longer stop collection - in either app** ([#2148], reported on an Azure elastic pool minutes after upgrading) - Lite's collection ladder runs its steps sequentially, and while every step's exceptions were contained, nothing bounded a HANG: one stuck task (the new Query Store backfill was the prime suspect on the reporter's timeline) silently froze every collector, and the CPU chart going blank was just where it got noticed. Every backfill slice now runs under a PER-SERVER abandonment deadline with an in-flight guard (the scheduled-analysis idiom, extracted as a reusable primitive) in BOTH apps - Lite's tick and Darling's fleet loop share the exact shape: a wedged slice is abandoned so everything else continues, quarantined to ITS server only (never relaunched on top of itself, never blocking a neighbor), and self-restoring when the stuck task actually ends; Lite's fleet-wide connection check gets the same treatment. Abandonment logs at ERROR naming the issue, and an abandoned task's LATE exception - the one that explains the wedge - is surfaced instead of discarded. The deadlines are generous multiples of a single healthy slice, so those lines are always defect signals worth reporting. - **The retention purge retries once on a deadlock instead of wasting the cycle** ([#2143], caught live by the nightly's purge e2e) - drop_chunks can lose a deadlock to a TimescaleDB background job whose chunk locks clear within milliseconds of the abort; previously that one transient loss pushed the table to the row-by-row DELETE fallback (which can deadlock against the same partner) and burned the whole cycle. Now 40P01 earns exactly ONE immediate retry - a second deadlock in a row is standing contention, where the fallback-plus-next-cycle posture is right and a retry loop camped on a lock queue is not. Non-deadlock failures keep the original single-shot behavior. @@ -2659,5 +2661,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2154]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2154 [#2164]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2164 [#2170]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2170 +[#2169]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2169 [#2167]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2167 [#2138]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2138 diff --git a/Lite.Tests/DatabaseSizeCollectorDefinitionTests.cs b/Lite.Tests/DatabaseSizeCollectorDefinitionTests.cs index a0ead6e6c..9fa2e9a68 100644 --- a/Lite.Tests/DatabaseSizeCollectorDefinitionTests.cs +++ b/Lite.Tests/DatabaseSizeCollectorDefinitionTests.cs @@ -46,6 +46,58 @@ public void BuildQuery_OnPrem_SplicesExclusionAtBothSites_ParamsOnce() Assert.Equal("SO", Assert.Single(plan.Parameters).Value); } + [Fact] + public void OnPrem_TotalSize_PrefersTheInDatabaseCurrentSize_SoTempdbCannotExceed100Percent() + { + /* #2169: the viewer computes used% as used_size_mb / total_size_mb. Used comes from FILEPROPERTY + read INSIDE each database; total used to come from sys.master_files.size, which records the size + at configuration time and does NOT track autogrowth for tempdb. A grown tempdb therefore + reported current usage against its startup size and rendered above 100%. The probe now captures + the in-database current size in the SAME round trip, and the payload prefers it, so both operands + come from one snapshot. */ + var plan = DatabaseSizeStatsCollector.Instance.BuildQuery(new CollectorContext + { + ServerId = 1, + ServerName = "test-server", + CollectionTime = DateTime.UtcNow, + Deltas = s_deltas, + }); + + Assert.Contains("current_size_mb decimal(19,2) NULL", plan.Text, StringComparison.Ordinal); + Assert.Contains("INSERT #file_space (database_id, file_id, used_size_mb, current_size_mb)", plan.Text, StringComparison.Ordinal); + Assert.Contains("CONVERT(decimal(19,2), df.size * 8.0 / 1024.0)", plan.Text, StringComparison.Ordinal); + + /* The fallback is load-bearing: a database whose probe failed (mid-restore, permissions) still + reports a total from master_files rather than NULL, so it degrades in precision and never + disappears from the grid. */ + Assert.Contains("COALESCE(fs.current_size_mb, mf.size * 8.0 / 1024.0)", plan.Text, StringComparison.Ordinal); + + /* The stale source must no longer be the total on its own. */ + Assert.DoesNotContain("total_size_mb =" + Environment.NewLine + " CONVERT(decimal(19,2), mf.size * 8.0 / 1024.0),", plan.Text, StringComparison.Ordinal); + } + + [Fact] + public void AzureSqlDb_AlreadyUsedInDatabaseSizes_SoItNeverHadTheTempdbSkew() + { + /* The Azure SQL DB path reads BOTH size and SpaceUsed from sys.database_files in the connected + database, so its used% was always internally consistent — #2169 was specific to the path that + mixes master_files with in-database reads (on-prem, RDS, and Managed Instance, which honors the + cross-database reference and therefore takes that path). Pinned so a future refactor does not + 'unify' the two by moving Azure onto the stale source. */ + var plan = DatabaseSizeStatsCollector.Instance.BuildQuery(new CollectorContext + { + ServerId = 1, + ServerName = "test-server", + CollectionTime = DateTime.UtcNow, + Deltas = s_deltas, + Target = new CollectorTargetInfo { IsAzureSqlDb = true }, + }); + + Assert.Contains("total_size_mb =", plan.Text, StringComparison.Ordinal); + Assert.Contains("df.size * 8.0 / 1024.0", plan.Text, StringComparison.Ordinal); + Assert.DoesNotContain("sys.master_files", plan.Text, StringComparison.Ordinal); + } + [Fact] public void AzureDmvPermissionHint_ExplainsError300_OnAzureOnly() { diff --git a/PerformanceMonitor.Collectors/DatabaseSizeStatsCollector.cs b/PerformanceMonitor.Collectors/DatabaseSizeStatsCollector.cs index 50b3f9797..2af09cf48 100644 --- a/PerformanceMonitor.Collectors/DatabaseSizeStatsCollector.cs +++ b/PerformanceMonitor.Collectors/DatabaseSizeStatsCollector.cs @@ -73,7 +73,12 @@ CREATE TABLE #file_space ( database_id int NOT NULL, file_id int NOT NULL, - used_size_mb decimal(19,2) NULL + used_size_mb decimal(19,2) NULL, + /* #2169: the file's CURRENT size, read in-database alongside SpaceUsed. sys.master_files.size is the + size recorded at configuration time and does NOT track autogrowth for tempdb, so a grown tempdb + reported used (current) against total (startup) and produced a used% above 100. Every database + benefits — master_files can lag any autogrowth — but tempdb is where it is guaranteed to. */ + current_size_mb decimal(19,2) NULL ); /* #1851: every failure below used to die in an empty CATCH, so a database that was mid-restore or @@ -106,11 +111,12 @@ ORDER BY BEGIN BEGIN TRY SET @sql = N'EXECUTE ' + QUOTENAME(@db_name) + N'.sys.sp_executesql N'' -INSERT #file_space (database_id, file_id, used_size_mb) +INSERT #file_space (database_id, file_id, used_size_mb, current_size_mb) SELECT DB_ID(), df.file_id, - CONVERT(decimal(19,2), FILEPROPERTY(df.name, N''''SpaceUsed'''') * 8.0 / 1024.0) + CONVERT(decimal(19,2), FILEPROPERTY(df.name, N''''SpaceUsed'''') * 8.0 / 1024.0), + CONVERT(decimal(19,2), df.size * 8.0 / 1024.0) FROM sys.database_files AS df;'';'; EXECUTE sys.sp_executesql @sql; @@ -138,7 +144,11 @@ INSERT @probe_failures (name, error_text) file_name = mf.name, physical_name = mf.physical_name, total_size_mb = - CONVERT(decimal(19,2), mf.size * 8.0 / 1024.0), + /* #2169: in-database current size when the probe got it, else master_files. Both operands of the + used% the viewer computes then come from the SAME snapshot, so used can no longer exceed total + on a database whose files grew since configuration (tempdb, always). A probe that failed leaves + this NULL and falls back — worse precision, never a wrong ratio direction. */ + CONVERT(decimal(19,2), COALESCE(fs.current_size_mb, mf.size * 8.0 / 1024.0)), used_size_mb = fs.used_size_mb, auto_growth_mb = From df0c174872c3d8d7e932a42661cd044a6516ee08 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 11 Aug 2026 03:05:15 +0200 Subject: [PATCH 2/2] Changelog: drop the stray blank line between Fixed bullets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review nit — my insert left a blank line the surrounding list doesn't use. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7691c9c4..b9615210c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **tempdb no longer reports more than 100% used in FinOps Database Sizes** ([#2169], reported by @CatastropheOps) - the used percentage divided in-database usage by the size recorded in `sys.master_files`, which is the size set at configuration time and does not track autogrowth for tempdb. A tempdb that had grown was therefore measured against its startup size and rendered above 100%. The per-database probe now captures the file's current size in the same round trip it already makes for space-used, and the payload prefers it, so both halves of the ratio come from one snapshot; a database whose probe fails still falls back to the old source rather than vanishing from the grid. Affects the on-prem, RDS, and Managed Instance path - the Azure SQL Database path already read both numbers in-database. - - **Backing out of Custom Range no longer strands an open calendar - both apps** ([#2154], reported in #2153) - a DatePicker's calendar dropdown is a popup living outside the visual tree's visibility, so collapsing the pickers when the user switched back to a preset range left an already-open calendar floating on screen; the dropdowns now close explicitly alongside the collapse, in Lite's ServerTab and the Darling Viewer's twin alike. - **One wedged background task can no longer stop collection - in either app** ([#2148], reported on an Azure elastic pool minutes after upgrading) - Lite's collection ladder runs its steps sequentially, and while every step's exceptions were contained, nothing bounded a HANG: one stuck task (the new Query Store backfill was the prime suspect on the reporter's timeline) silently froze every collector, and the CPU chart going blank was just where it got noticed. Every backfill slice now runs under a PER-SERVER abandonment deadline with an in-flight guard (the scheduled-analysis idiom, extracted as a reusable primitive) in BOTH apps - Lite's tick and Darling's fleet loop share the exact shape: a wedged slice is abandoned so everything else continues, quarantined to ITS server only (never relaunched on top of itself, never blocking a neighbor), and self-restoring when the stuck task actually ends; Lite's fleet-wide connection check gets the same treatment. Abandonment logs at ERROR naming the issue, and an abandoned task's LATE exception - the one that explains the wedge - is surfaced instead of discarded. The deadlines are generous multiples of a single healthy slice, so those lines are always defect signals worth reporting. - **The retention purge retries once on a deadlock instead of wasting the cycle** ([#2143], caught live by the nightly's purge e2e) - drop_chunks can lose a deadlock to a TimescaleDB background job whose chunk locks clear within milliseconds of the abort; previously that one transient loss pushed the table to the row-by-row DELETE fallback (which can deadlock against the same partner) and burned the whole cycle. Now 40P01 earns exactly ONE immediate retry - a second deadlock in a row is standing contention, where the fallback-plus-next-cycle posture is right and a retry loop camped on a lock queue is not. Non-deadlock failures keep the original single-shot behavior.