Skip to content

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
erikdarlingdata merged 2 commits into
devfrom
fix/3653-local-clock-buckets
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

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_time with the service host's DateTime.UtcNow, and both providers keyed the hour-of-week baseline on it: the shared RobustTierScaffold's keyed CTE did EXTRACT(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's baseline_bucket metadata ("Tue 22:00") named a time nobody on that server keeps. The C# half of the same smear was the lookup: GetBaselineAsync keyed on analysisTime.Hour / analysisTime.DayOfWeek on 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 by server_id, bucket, collection_time, i.e. it materializes per-collection rows whose only time columns are the naive-UTC collection_time and an hourly time_bucket used for the min(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 after CacheTtl keys on local time.

What changed

One shared expression, BaselineLocalClock.LocalCollectionTimeSql:

(collection_time + (CASE WHEN collection_time < $4 THEN $5 ELSE $6 END) * INTERVAL '1' MINUTE)

is substituted for bare collection_time inside each provider's RobustTierScaffold (hour, dow AND the distinct-day date) and inside the two event arms. Every arm that ends in clean(collection_time, v) + the scaffold — the SQL Server arms, the legacy arms in GetLegacyBaselineQuery, and every PgTargetBaselineProvider* arm including the quarter-hour date_bin I/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.GetPriorOccurrencesSql already uses integer * INTERVAL '1' MINUTE in 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: $4 the single offset transition inside the window as naive UTC (the window end when there is none), $5/$6 the offset minutes before/after it. They come from the newest server_properties row carrying an offset — the LocalDataService.ServerInfo / GetPriorOccurrencesSql read shape, with V134's time_zone_id riding along — resolved once per compute by the new shared PerformanceMonitor.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.FindSystemTimeZoneById accepts the Windows id on Linux/macOS when ICU is present — verified present in mcr.microsoft.com/dotnet/aspnet:10.0, along with tzdata, and DOTNET_SYSTEM_GLOBALIZATION_INVARIANT unset), 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 no server_properties row and lands on that last arm today, by construction.

GetBaselineAsync looks 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's baseline_bucket label (BaselineContextFormatter, from the detector's baseline_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 TimeZoneInfo in 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 ZONE with an IANA name bound as text — rejected, not for infeasibility: DuckDB.NET 1.5.5 runs TIMESTAMP AT TIME ZONE 'UTC' AT TIME ZONE 'America/New_York' without an explicit LOAD 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

  • No migration rung, no aggregate change (V135 / Lite v63 unchanged). Nothing keyed is stored.
  • Does not touch BaselineMath.cs, BaselineBucket.cs, or any PgTarget*.cs file (the PostgreSQL-target analysis engine: v2/v3 slices and the v1 lanes' residue (from #3542) #3691 fence); the PgTarget arms inherit through PgBaselineProvider.RobustTierScaffold.
  • Does not give a PostgreSQL target a clock — none has a server_properties row, 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).
  • Does not change the result-set shape, the reader, the 6/8-column detection, or the $1..$3 bindings. A new arm that ends in clean(collection_time, v) + the scaffold needs no edit.
  • The clock read sits INSIDE the compute's existing classified try/catch on purpose (a store that cannot answer a one-row indexed read cannot answer the 30-day scan either); AnalysisShutdownResidueTests still 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..$3 while 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 bare EXTRACT(HOUR FROM collection_time) would run, silently, on UTC. The census tests in both test projects are the enforcement: every non-null GetBaselineQuery / GetLegacyBaselineQuery / PgTargetBaselineProvider.GetPgTargetBaselineQuery text either ends in the one scaffold or extracts from the one local-time expression, no bare EXTRACT(HOUR|DOW FROM collection_time) or collection_time::DATE survives, and every statement references $3..$6. Red-first: with the provider's LocalCollectionTime alias reverted to "collection_time" (members intact, $4..$6 still 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) and Lite.Tests/LocalClockBucketKeyTests.cs (8), the fixtures Erik asked for, both SKUs:

  1. UTC−5 offset-only, time_zone_id NULL: Tuesday 23:00Z → (18, Tue); BaselineContextFormatter names "Tue 18:00", not "Tue 23:00".
  2. Day rollover: Wednesday 03:00Z at UTC−5 → (22, Tue) — the dow is the local date's.
  3. DST straddle, zone id "Eastern Standard Time" (and the IANA spelling): a 30-day window over 2026-03-08 07:00Z resolves $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.
  4. The pure math's degradations: no transition → before == after (the ZONE's July answer, not a stale winter snapshot's); NULL zone + offset → fixed offset; NULL both → UTC; unresolvable zone id → offset fallback with the note logged once across repeated resolves and once more for a different id; a window that ends before it starts is refused.
  5. Through the real providers: Lite's BaselineProvider over the real DuckDB schema with a seeded server_properties row (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's PgBaselineProvider live (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's Darling PostgreSQL tests is the second execution.
  6. RobustBaselineDuckDbTests now runs the real RobustTierScaffold (made internal) 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's by_range on a plain-PG rig; 2 Timescale-gated skips), Lite 35/35 incl. all of BaselineProviderTests; DocCommentHygieneTests 77/77. All touched projects build with 0 warnings (Lite.Tests carries one pre-existing xUnit2000 on LogTailOverlapThresholdPinTests, untouched).

…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
@erikdarlingdata
erikdarlingdata merged commit 15b1a23 into dev Sep 19, 2026
6 of 7 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/3653-local-clock-buckets branch September 19, 2026 15:20
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant