Repository navigation
Derive the TimescaleDB ensure-summary log from its own source (#1746) - #1747
Merged
Merged
Conversation
The line's counts were computed ({Ready}/{Total}, correctly reporting 8)
but the parenthetical text was hand-written and still enumerated the six
aggregates that existed before #1664 added the db-grain pair. An operator
mid-upgrade at a client site spent real time reconciling 8-against-6
before concluding it was not a bug.
Fixed by shape rather than by number: the names now come from the same
array the counts come from, so the summary quotes its own source and a
future pair cannot make it stale again. Replacing "3 hourly" with "4
hourly" would have been the same defect waiting to recur.
Swept the sibling summary lines for the same class. The retention,
hypertable and compression lines already interpolate computed counts and
interval constants, and the RetentionPolicyCount = 7 comment (three raw
plus four hourly) checks out against the policy array. This was the only
hardcoded enumeration.
Closes #1746
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes #1746. Reported from a client-site upgrade.
The defect
The ensure sweep's summary line got its counts right —
{Ready}/{Total}is computed from the aggregate array, so it correctly said 8 — while its parenthetical text was hand-written and still read:That enumeration predates #1664, which added
query_stats_db_hourly/query_stats_db_daily. So the line said 8 and then listed 6, and an operator mid-upgrade spent real time reconciling the two before concluding it wasn't a bug. False operator-facing text that cost more than the thing it was describing.The fix is the shape, not the number
Replacing
3 hourlywith4 hourlywould have been the same defect rearmed — correct until the next pair lands. The names are now derived from the same array the counts come from, so the summary quotes its own source and cannot drift from it:where
{Views}isstring.Join(", ", aggregates.Select(a => a.View)). Adding a ninth aggregate updates the log for free.Sibling sweep
Checked every operator-facing summary in
TimescaleSupportfor the same class:RawRetentionInterval/HourlyRetentionIntervalconstants. Clean.{Converted}/{Total},{Applied}/{Total},{Days}from constants. Clean.RetentionPolicyCount = 7and its "three raw tables plus four hourly CAGGs" comment — verified against the policy array (3 raw + 4 hourly). Accurate, left alone.The
DuckDbInitializerlines that enumeratequery_stats, procedure_stats, query_store_statsare historical migration messages describing what v2/v6/v13 each did — accurate for their own migration, not statements about current state. Left alone.So this was the only instance.
Tests
No test pinned the old text, which is part of why it drifted unnoticed.
Unrelated observation: dev currently builds with 3 warnings against its zero-warning policy —
CS8602atLite/MainWindow.AlertEngine.cs:262andxUnit2031atLite.Tests/AgAlertEvaluatorTests.cs:225, both from the AG-alerts work. Neither is in this diff; flagging rather than absorbing.🤖 Generated with Claude Code