Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **A retention policy whose coverage list GROWS is now re-held, instead of purging past the new tier forever** ([#1877]) - the arming gate creates every retention policy paused and arms it only once every rollup beneath it already reaches as far back as the source does. It could only ever ARM. `add_retention_policy(if_not_exists => true)` returns -1 for a policy the store already has, so nothing paused one whose COVERAGE LIST had grown under it - and [#1869] was the first build to hand a new consumer to a gate that stores had already armed. Such a store logged `HELD PAUSED` for the interval layer on every start and kept purging it anyway. Nothing a read could reach was lost, because every window older than the new tier's floor routes to a tier that holds it; what was lost is reachable DEPTH, since the new day-grain daily could then only ever be backfilled as deep as the 7-day interval layer still held rather than as deep as the store's own history.

**The fix is a three-valued coverage verdict, and the third value is the whole of it.** The obvious form - "unsafe implies disarm" - is materially worse than the gap it closes, because the probe is deliberately fail-closed: any exception, timeout or transient failure answers "not safe". Under an arm-only gate that costs nothing, but a disarm-capable gate reading the same bool would let one bad probe on a busy store stop purging across every tier at once, and disk would grow until somebody noticed - trading a bounded depth cap for an unbounded disk risk. The probe now answers COVERED, SHORT or UNKNOWN, and only a positive SHORT measurement may stop a policy that is already running. UNKNOWN leaves the policy in whatever state it is already in: a newly created one stays paused exactly as before, and one this store already armed keeps purging. A consumer that exists and holds no rows is a SHORT measurement rather than an unknown - which is precisely the state a newly-added consumer is born in, and is what makes the re-hold fire on the upgrade it was written for.

**Release is the existing arming path, unchanged, so this is a gate and not a latch.** A re-held policy arms itself on the first start after `--backfill-rollups` carries the new consumer past the horizon, with no manual step, exactly as a first-time hold has always released. The catalog sweep's own drop still reads the same verdict through the same map, so the two purge paths cannot judge one drop differently ([#1784]) - and for the three raw tiers this removes a case where they already could, since an armed policy used to keep dropping the very chunks the sweep was refusing to touch. The per-policy warning now names the specific tier that is short instead of the whole coverage list, and the startup summary reports how many policies were left as-is because their coverage could not be read at all. Two narrower things are filed rather than folded in: the ordering this now depends on is still asserted by hand rather than derived from the policy list, so a future policy on a daily tier could reintroduce the risk from the other side ([#1905]), and a re-hold still reads identically to a first-time hold in the log even though only one of them stopped something that was running ([#1906]).

- **A cross-database lock now names the database the contended object actually lives in, from both blocking collectors** ([#1893]) - [#1876] made the two collectors' `contentious_object` labels agree at the incident identity, and corrected the `blocked_process_report` sentinel to name the lock RESOURCE's database rather than the blocked session's. The DMV snapshot side could not follow: its normalization runs in C# against a stored row whose only database name is `database_name`, which the collector writes as the blocked SESSION's database. A cross-database lock is held in one database by a session running in another, so for exactly that case the two sides still disagreed - the report row said `database: StackOverflow2013` where the snapshot row said `database: master` - and one contended object still raised two alerts.

**The resource's database id comes from `sys.dm_os_waiting_tasks.resource_description`, not from the wait resource.** MS Learn documents every lock resource type's description as carrying a `dbid=<db-id>` token - `keylock`, `pagelock`, `ridlock`, `objectlock`, `databaselock`, `filelock`, `extentlock`, `applicationlock`, `metadatalock`, `hobtlock` and `allocunitlock` - so a single parse covers every lock shape, where splitting `wait_resource` positionally would need a branch per resource type and would still miss the ones whose layout differs. `DB_NAME()` then runs in the same query, which is the whole reason this had to move server-side: the id was always sitting in the row, but nothing downstream could turn it into a name. Verified live on SQL Server 2022 against a genuine cross-database KEY lock, whose description reads `keylock hobtid=72057594045726720 dbid=11 id=lock... mode=X associatedObjectId=...`; the same blocking pair produced `Unresolved: key lock, database: pm1893_res` from both collectors, byte for byte, where before the fix the snapshot row carried the raw `KEY: 11:72057594045726720 (8194443284a0)` and could only ever have been named with the session's database.
Expand Down Expand Up @@ -129,7 +135,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **The Query Store DAILY numbers stopped over-counting by roughly 2x - the tier that is kept indefinitely is now the accurate one** ([#1869]) - the corrected rollups above dedup each Query Store interval within a collection HOUR, so an interval whose snapshots straddle an hour boundary produced two rows, each holding a cumulative value, and the daily sum counted it once per hour it was collected in rather than once. That is irreducible at the hourly grain - an interval genuinely collected across two hours has to appear in both - but not at the daily grain, and the daily tier is the one kept forever, so a permanent 2x there is a permanent lie. Two more continuous aggregates remove it: `query_store_stats_interval_daily` re-dedups the interval across the whole DAY with `last(execution_count, bucket)`, and `query_store_stats_daygrain_daily` collapses that to the same composer columns the other dailies carry, so every composed panel reads it with no change. Measured on a seeded store with the shape [#1869] published: a day whose true total is **515** reads **1,013** from the hour-grain daily - **1.97x** - and the new one reads 515.

**What it does NOT fix, because a mis-count is worth stating exactly.** An interval whose snapshots straddle MIDNIGHT is still counted once per collection DAY. It is the identical argument one grain up and equally irreducible there, but it is far smaller and the difference is structural rather than lucky: the collector fetches an interval while its `last_execution_time` keeps advancing, so a 60-minute interval is collected across an hour boundary almost always and a day boundary about once per 24 intervals - roughly a 4% over-count against the 97% removed. It is pinned by a test against a real store so the claim cannot quietly drift, and filed with the cost of the fifth aggregate that would shrink it at [#1879], along with the reason no finite dedup grain removes it outright. One upgrade note is filed the same way at [#1877]: the purge gate is arm-only, so a store whose interval-layer policy was already armed under [#1849] keeps purging while the new level catches up - nothing a read can reach is lost, but running `--backfill-rollups` promptly after upgrading is what lets the new daily reach back past 7 days.
**What it does NOT fix, because a mis-count is worth stating exactly.** An interval whose snapshots straddle MIDNIGHT is still counted once per collection DAY. It is the identical argument one grain up and equally irreducible there, but it is far smaller and the difference is structural rather than lucky: the collector fetches an interval while its `last_execution_time` keeps advancing, so a 60-minute interval is collected across an hour boundary almost always and a day boundary about once per 24 intervals - roughly a 4% over-count against the 97% removed. It is pinned by a test against a real store so the claim cannot quietly drift, and filed with the cost of the fifth aggregate that would shrink it at [#1879], along with the reason no finite dedup grain removes it outright. One upgrade note was filed the same way at [#1877] and is FIXED below in this same release: the purge gate could only arm, so a store whose interval-layer policy was already armed under [#1849] kept purging while the new level caught up. It is now re-held until the new level covers it, and running `--backfill-rollups` promptly after upgrading is what releases it.

**Three dailies now, best-first, each rung the same rule.** The day-grain daily is a NEW aggregate that starts empty, so it wins only where it has actually materialized the window; older windows fall to the corrected daily, and older ones still to the superseded original - each step comparative, so a store that has been backfilled never silently reads the coarser number. The hourly tier is untouched: there is no better hourly to route to.

Expand Down Expand Up @@ -2179,3 +2185,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1896]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1896
[#1898]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1898
[#1902]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1902
[#1905]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1905
[#1906]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1906
Loading
Loading