diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b304ce6..b9615210 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ 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 +2660,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 a0ead6e6..9fa2e9a6 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 50b3f979..2af09cf4 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 =