Repository navigation
query_stats.creation_time is compared against a naive-UTC bound, so PARAMETER_SENSITIVITY is scored against the wrong plan population #2991
Description
query_stats.creation_time is compared against a naive-UTC bound, so PARAMETER_SENSITIVITY is scored against the wrong plan population on every non-UTC server
Confirmed live on the production fleet, not hypothetical. Every production SQL Server target reports utc_offset_minutes = -240 (UTC-4). The only target at 0 is a dev box. So this affects the whole monitored SQL Server fleet across both stores, not one server's configuration quirk.
The defect
Six sites compare a server-local column against a bound derived from DateTime.UtcNow:
Darling/PerformanceMonitor.Darling.Analysis/PgFactCollector.QueryPerf.csDarling/PerformanceMonitor.Darling.Analysis/PgDrillDownCollector.Queries.cs(two sites)Lite/Analysis/DuckDbFactCollector.QueryPerf.csLite/Analysis/DrillDownCollector.Queries.cs(two sites)
Each reads creation_time <= $n, where $n is AnalysisContext.TimeRangeStart — which both analysis services derive from DateTime.UtcNow. query_stats.creation_time is the monitored server's local clock (documented at QueryStatsCollector.cs:167).
Why the predicate exists, and what breaks
The predicate means "this plan was compiled before the analysis window opened." That is what makes a large min/max worker-time spread evidence of parameter sensitivity rather than of a plan that simply has not lived long enough to have seen varied parameters yet. It is the guard that separates a real signal from an artefact of plan age.
At UTC-4 the arithmetic works out as follows. A plan compiled at UTC instant T is stored with creation_time = T - 4h. The predicate therefore admits any plan where T - 4h <= W, i.e. T <= W + 4h, where W is the window start. On the default 4-hour window the window is [W, W + 4h] — so the predicate admits every plan compiled inside the window, which is precisely the population it exists to exclude.
A negative offset is the false-positive direction. Recently-compiled plans are admitted, and their partial-life worker-time spread is scored as if it were full-life variance. So PARAMETER_SENSITIVITY facts and the drill-downs hanging off them have been generated against the wrong population fleet-wide — over-reporting rather than silently suppressing.
For completeness, the other direction is worse but does not apply here: at a positive offset (say UTC+10) the predicate admits only plans compiled at least 10 hours before the window, so on a 4-hour window essentially nothing qualifies and the finding class silently stops being produced altogether. Any deployment east of UTC gets that behaviour instead.
Scope note
AnalysisContext.ServerUtcOffset exists but is TimeSpan.Zero in both live SKUs. The only non-zero assignment in the tree is in the retired Dashboard project, and its semantic is the inverse — it converts a server-local window back to UTC for persistence — so it is not the seam to reuse without deciding what it means here.
The retired Dashboard carries the same shape at Analysis/SqlServerFactCollector.QueryPerf.cs, Analysis/SqlServerDrillDownCollector.Queries.cs and Services/DatabaseService.QueryPerformance.Stats.cs. Out of scope unless that SKU is still shipped.
Not verified
The predicted effect on findings output has not been measured against stored analysis_findings rows — that comparison would confirm the over-reporting empirically and is the obvious next step, but it needs a store query rather than source reading. Nobody has checked whether the finding counts actually look inflated.
The fix direction is a per-server de-skew in SQL from the collected server_properties.utc_offset_minutes, which is the pattern already used elsewhere in this codebase. That is six SQL edits across two dialects and it changes what the detector emits, so it wants live verification rather than a source-only change.
Activity
De-skew query_stats.creation_time before comparing it to the analysis…
erikdarlingdata commentedon Sep 5, 2026
Closed by #2992 (
064a01656a). Closing by hand — this repository merges todev, so a closing keyword never fires.Both dialects now de-skew
creation_timefrom the collected offset before comparing it to the UTC window, verified against real PostgreSQL 17 and real DuckDB 1.5.5 on a five-plan fixture straddling the bound: at −240 the old form selected 5 where the correct answer is 3, at 0 both give 3, and at +600 the old form selected 1.Two corrections to this issue's own text, both from that measurement:
AnalyzeAsyncderives the window fromGetServerClockAsync()(SYSDATETIME()), so it compares server-local to server-local and is correct — which is also precisely whyServerUtcOffsetexists there with the inverse contract, making the trap this issue flagged real for a sharper reason than the name.Nothing asserted the buggy behaviour. Every seeded
creation_timein either suite was three days before the window with no collected offset, so all of them were offset-tolerant with a wide margin — the defect was invisible to the existing pins rather than pinned in place.StoreSqlClockDisciplineTestsis structurally blind to it:creation_timenever appears in a DDL string literal, so it is not in that scan's scraped vocabulary. And no cross-SKU guard existed at all — nothing compared Darling's analysis SQL to Lite's until now.compile_age_secondswas the tempting seam and is unreachable. The collector computes it for exactly this purpose but deliberately does not store it, so the collected offset is the only available route.Still not measured
The empirical confirmation this issue asked for has not been run — nobody has re-scored stored findings to show the counts were inflated as predicted. Three read-only queries are written and verified to parse against the real migrated schema; they need a store I cannot reach. The substantive one re-scores every
PARAMETER_SENSITIVITYfinding from the last 30 days both ways over that finding's own window and buckets the result. Prediction at a negative offset:new <= oldeverywhere, with a non-zero would-stop-firing count.