Repository navigation
Hour-of-week baselines key on the target's local clock, not UTC: one shared step-function expression re-keys every arm through the scaffold, three bound clock parameters from server_properties (zone id, offset fallback, UTC when there is no row), the lookup through the same numbers — nothing keyed is stored, so re-bucketing is the next compute (#3653 item 12, Q6) - #3749
Merged
Merged
Conversation
…item 12, Q6) Both SKUs stamp collection_time with the service host's DateTime.UtcNow and keyed the hour-of-week baseline on it — EXTRACT(HOUR/DOW FROM collection_time) — so a "Tue 22:00" bucket pooled 17:00 and 18:00 on a UTC−5 server across a DST change, and the finding's baseline_bucket named an hour nobody on that server keeps. Erik's ruling: buckets key on the target's local clock; the UTC smear IS the DST defect. Nothing keyed is stored (every supply is per-collection rows; the key lives in the query text and a 1 h cache), so no rung and re-bucketing is automatic at the next compute. The change: one shared local-time expression — collection_time + (CASE WHEN collection_time < $4 THEN $5 ELSE $6 END) * INTERVAL '1' MINUTE, byte-identical on PostgreSQL and DuckDB — substituted for bare collection_time inside each provider's RobustTierScaffold (every arm ending in clean(collection_time, v) inherits it, PgTarget arms included) and in the two event-family arms; three parameters bound after the window bounds from the newest server_properties row (time_zone_id preferred, utc_offset_minutes fallback, 0/0 = UTC when there is no row); the C# lookup through the same three numbers. The pure math is one new shared file, Baselines/BaselineLocalClock.cs. Neither engine rejects a statement that ignores $4..$6 (measured), so a census test in each test project is the enforcement.
…repo scrub guard (short-name-ordinal shape) does not read 'Wednesday-03' / 'UTC-05' as a server name
This was referenced Sep 19, 2026
Closed
erikdarlingdata
added a commit
that referenced
this pull request
Sep 19, 2026
…ne setting, so a New York target's Tuesday-09:00 bucket is 09:00 in New York in every season (partial #3691, lane 20) (#3757) The clock read in PgBaselineProvider becomes a protected virtual seam (SQL Server body unchanged, byte for byte); PgTargetBaselineProvider overrides it once to read the TimeZone row of the latest pg_server_config snapshot at or before the window end, session-scoped sources excluded (the config facts' rule, copied), and hands the shared resolver the zone name. UTC is now the fallback for a target with no server-scoped snapshot, not the rule for the engine. Doc riders: the root doc names the $4..$6 bind the base has made since #3749 and states the setting-then-UTC rule; the TpsTrendKey prose in PgTargetScorer.Database.cs and PgTargetFactCollector.Database.cs no longer claims a co-fire reads the trend (nothing has since #3747).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial #3653 — item 12 (Q6)
Erik's ruling, verbatim: "Hour-of-week buckets key on the target's local clock; existing baselines re-bucket once. The UTC smear is the DST defect itself."
The mechanism
Both SKUs stamp
collection_timewith the service host'sDateTime.UtcNow, and both providers keyed the hour-of-week baseline on it: the sharedRobustTierScaffold'skeyedCTE didEXTRACT(HOUR FROM collection_time),EXTRACT(DOW FROM collection_time),collection_time::DATE, and the two event-family arms (blocking, deadlock) did the same by hand. A bucket the store called "Tuesday 22h" was therefore Tuesday 17:00 on a UTC−5 server in winter and Tuesday 18:00 on the same server in summer — the bucket pooled two different local hours, and the finding'sbaseline_bucketmetadata ("Tue 22:00") named a time nobody on that server keeps. The C# half of the same smear was the lookup:GetBaselineAsynckeyed onanalysisTime.Hour/analysisTime.DayOfWeekon a UTC analysis time.Nothing keyed is stored. Every baseline supply — the raw hypertables,
perfmon_interval_baseline,wait_stats_interval_baseline, the legacy pair,session_stats_baseline,query_stats_baseline,blocked_process_baseline,deadlock_baseline,memory_baseline— groups byserver_id, bucket, collection_time, i.e. it materializes per-collection rows whose only time columns are the naive-UTCcollection_timeand an hourlytime_bucketused for themin(bucket)reach probes and retention. The hour×dow key exists only in the query text at compute time and in the providers' 1-hour cache. So: no aggregate changes, no migration rung, and the "re-bucket once" the ruling asks for is automatic — the first compute afterCacheTtlkeys on local time.What changed
One shared expression,
BaselineLocalClock.LocalCollectionTimeSql:is substituted for bare
collection_timeinside each provider'sRobustTierScaffold(hour, dow AND the distinct-day date) and inside the two event arms. Every arm that ends inclean(collection_time, v)+ the scaffold — the SQL Server arms, the legacy arms inGetLegacyBaselineQuery, and everyPgTargetBaselineProvider*arm including the quarter-hourdate_binI/O grain (every real offset is a multiple of 15 minutes, so a bin and its rows shift into the same local hour) — inherits the local key with zero edits of its own. The expression is the store's existing shared dialect (PgFindingStore.GetPriorOccurrencesSqlalready usesinteger * INTERVAL '1' MINUTEin both PG and DuckDB) and runs byte-identical on PostgreSQL 17 and DuckDB 1.5.5 (both measured).Three parameters follow the existing
$1 server_id, $2 window start, $3 analysis time:$4the single offset transition inside the window as naive UTC (the window end when there is none),$5/$6the offset minutes before/after it. They come from the newestserver_propertiesrow carrying an offset — theLocalDataService.ServerInfo/GetPriorOccurrencesSqlread shape, with V134'stime_zone_idriding along — resolved once per compute by the new sharedPerformanceMonitor.Analysis/Baselines/BaselineLocalClock.cs: the zone id when present and resolvable on the host (it is the only input that knows when the offset changed, which is the whole of the DST fix;TimeZoneInfo.FindSystemTimeZoneByIdaccepts the Windows id on Linux/macOS when ICU is present — verified present inmcr.microsoft.com/dotnet/aspnet:10.0, along with tzdata, andDOTNET_SYSTEM_GLOBALIZATION_INVARIANTunset), else the stored offset as a fixed shift (with one Information note per zone id per process: "not resolvable on this host; using the offset in force at the snapshot, which is off by an hour across a DST transition"), else 0/0 — UTC keying, exactly what every bucket was before. A PostgreSQL target has noserver_propertiesrow and lands on that last arm today, by construction.GetBaselineAsynclooks the analysis time up through the same three numbers (LocalClockWindow.LocalKey: local hour and the LOCAL date's day-of-week — Wednesday 03:00 UTC at UTC−5 is Tuesday 22h), cached beside the buckets so a cache hit and its lookup agree even if the server's stored clock changed since. The finding'sbaseline_bucketlabel (BaselineContextFormatter, from the detector'sbaseline_hour/baseline_dow) prints the bucket's key, so it becomes the local hour with no edit.Why this shape, and the one rejected
(B), piecewise offsets from
TimeZoneInfoin C# — chosen. A real zone changes offset at most once in any 30-day window (DST pairs are ~6 months apart; the closest pair in the current tz database is a Ramadan DST suspension ~35 days wide), so the zone over this window IS a step function with one step, and the SQL is engine-agnostic arithmetic with no tz dependency in either dialect, no Windows→IANA mapping in SQL, and ONE statement shape for the zone-present, offset-only, and no-row cases. It is exact by construction, and — the property that matters most — the SQL key and the C# lookup key are computed from the same three numbers, so they cannot disagree. The resolver scans the window hourly and bisects each change to the second; if it ever sees more than one step it keys the newest segment exactly and notes it once, rather than pooling silently.(A),
AT TIME ZONEwith an IANA name bound as text — rejected, not for infeasibility: DuckDB.NET 1.5.5 runsTIMESTAMP AT TIME ZONE 'UTC' AT TIME ZONE 'America/New_York'without an explicitLOAD icu(measured), so it was viable on Lite. Rejected because it needs the Windows-id → IANA mapping bound into SQL, PostgreSQL's own tz database on the store (a bring-your-own store's tzdata is not ours to keep current), a SECOND statement shape for the offset-only fallback (a zone name cannot express "fixed −300"), and two independent conversions (SQL's and C#'s lookup) that could drift. POSIX-style offset strings in PG also have the inverted sign, a trap (B) never meets.What it deliberately does NOT do
BaselineMath.cs,BaselineBucket.cs, or anyPgTarget*.csfile (the PostgreSQL-target analysis engine: v2/v3 slices and the v1 lanes' residue (from #3542) #3691 fence); the PgTarget arms inherit throughPgBaselineProvider.RobustTierScaffold.server_propertiesrow, so PG targets stay on UTC keying exactly as before until a clock source for them exists (reported as an out-of-lane finding on the issue).$1..$3bindings. A new arm that ends inclean(collection_time, v)+ the scaffold needs no edit.AnalysisShutdownResidueTestsstill pins exactly one catch.The finding this makes explicit
Neither engine enforces the new parameters. Measured on PostgreSQL 17 through Npgsql 9 (extended protocol) and on DuckDB 1.5.5: a statement that references only
$1..$3while six parameters are bound executes without complaint — Npgsql sends typed OIDs for every parameter in Parse and PG accepts declared-but-unreferenced parameters. So an arm that bypassed the scaffold and kept a bareEXTRACT(HOUR FROM collection_time)would run, silently, on UTC. The census tests in both test projects are the enforcement: every non-nullGetBaselineQuery/GetLegacyBaselineQuery/PgTargetBaselineProvider.GetPgTargetBaselineQuerytext either ends in the one scaffold or extracts from the one local-time expression, no bareEXTRACT(HOUR|DOW FROM collection_time)orcollection_time::DATEsurvives, and every statement references$3..$6. Red-first: with the provider'sLocalCollectionTimealias reverted to"collection_time"(members intact,$4..$6still bound), 3 Darling tests and 3 Lite tests go red — the census, the expression pin, and the behavioral tests where the EST and EDT rows split and the bucket collapses to Flat.Tests
Darling.Tests/LocalClockBucketKeyTests.cs(13 + 1 live) andLite.Tests/LocalClockBucketKeyTests.cs(8), the fixtures Erik asked for, both SKUs:time_zone_idNULL: Tuesday 23:00Z → (18, Tue);BaselineContextFormatternames "Tue 18:00", not "Tue 23:00".$4 = 2026-03-08 07:00,$5 = −300,$6 = −240; Tue 2026-02-24 22:00Z (EST) and Tue 2026-03-10 21:00Z (EDT) key to the SAME (17, Tue) where UTC keyed them 22h/21h; Tue 2026-03-10 22:00Z is (18, Tue); the lookup at 21:30Z is (17, Tue). Fall-back 2026-11-01 06:00Z is the same step the other way, and a row stamped exactly at the transition is on the new side, as the SQL's<says.BaselineProviderover the real DuckDB schema with a seededserver_propertiesrow (offset-only → (18, Tue) with the "Tue 18:00" label; zone id → the EST/EDT rows are one 12-sample Full bucket over two local days, the Wednesday-03:xxZ rows are (22, Tue); no row → UTC keying as before). Darling'sPgBaselineProviderlive (DARLING_TEST_PG): CPU through the scaffold (one (17, Tue) bucket, 12 samples, 2 local days, mean 52.5; the rollover rows at (22, Tue) with a Tuesday date), Blocking through the hand-EXTRACT arm read at the SQL level with the provider's bindings (local: one (17, Tue) row, 2 events over 2 days; UTC: two one-day rows at 22h and 21h — the smear in the six-column shape). Executed here against a PG 17 rig; CI'sDarling PostgreSQL testsis the second execution.RobustBaselineDuckDbTestsnow runs the realRobustTierScaffold(madeinternal) over its fixture instead of a hand copy that had already gone stale.All pre-existing pins the brief names ran green through mactest harnesses on this Mac: Darling 368 total / 365 green (the 1 red is
RetiredBaselineAggregateLiveTests.Sweep_…needing TimescaleDB'sby_rangeon a plain-PG rig; 2 Timescale-gated skips), Lite 35/35 incl. all ofBaselineProviderTests;DocCommentHygieneTests77/77. All touched projects build with 0 warnings (Lite.Testscarries one pre-existingxUnit2000onLogTailOverlapThresholdPinTests, untouched).