-
Notifications
You must be signed in to change notification settings - Fork 915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cagg_migrate_to_time_bucket during update #7763
base: main
Are you sure you want to change the base?
Fix cagg_migrate_to_time_bucket during update #7763
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7763 +/- ##
==========================================
+ Coverage 80.06% 81.95% +1.88%
==========================================
Files 190 247 +57
Lines 37181 45460 +8279
Branches 9450 11360 +1910
==========================================
+ Hits 29770 37255 +7485
- Misses 2997 3735 +738
- Partials 4414 4470 +56 ☔ View full report in Codecov by Sentry. |
58ce092
to
860cb3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I just left some questions for things I am not sure about.
b18e3ca
to
ceee4ea
Compare
In timescale#6837 we added a migration for CAggs using `time_bucket_ng` to use the new version of `time_bucket` that support orign and/or offset. We made a mistake because we place the code to migrate existing CAggs using `time_bucket_ng` to `time_bucket` in the update script but this code should be placed instead on post-update.sql script where we can call functions from the new extension version loaded.
ceee4ea
to
8cfacf1
Compare
-- Migrate existing CAggs using time_bucket_ng to time_bucket | ||
DO $$ | ||
DECLARE | ||
cagg_name regclass; | ||
BEGIN | ||
FOR cagg_name IN | ||
SELECT | ||
pg_catalog.format('%I.%I', user_view_schema, user_view_name)::regclass | ||
FROM | ||
_timescaledb_catalog.continuous_agg | ||
JOIN _timescaledb_catalog.continuous_aggs_bucket_function USING (mat_hypertable_id) | ||
WHERE | ||
bucket_func::text LIKE '%time_bucket_ng%' | ||
LOOP | ||
CALL _timescaledb_functions.cagg_migrate_to_time_bucket(cagg_name); | ||
END LOOP; | ||
END | ||
$$; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We apparently made a decision to do this inside an update script some time in the past. However, I am unsure how expensive the operation is to update the continuous aggregates. This could potentially block an upgrade for a long time if it has to move a lot of data around, or create weird errors if the script is not properly written.
Are we sure this script is not going to block updates for production cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We apparently made a decision to do this inside an update script some time in the past. However, I am unsure how expensive the operation is to update the continuous aggregates. This could potentially block an upgrade for a long time if it has to move a lot of data around, or create weird errors if the script is not properly written.
This function replace the time_bucket_ng
by time_bucket
in the cagg query tree... it is very cheap.
Are we sure this script is not going to block updates for production cases?
This change is exactly to don't block updates cause now it is blocking with the following error:
TimescaleDB: 2025-02-17 10:54:00 UTC [425]: [67b3153a.1a9-3] 2313713420 postgres@tsdb,app=deployer [XX000] STATEMENT: ALTER EXTENSION "timescaledb" UPDATE TO "2.18.1";
TimescaleDB: 2025-02-17 10:54:00 UTC [425]: [67b3153a.1a9-1] 2313713420 postgres@tsdb,app=deployer [XX000] ERROR: tried calling catalog_get when extension isn't loaded
TimescaleDB: 2025-02-17 10:54:00 UTC [425]: [67b3153a.1a9-2] 2313713420 postgres@tsdb,app=deployer [XX000] CONTEXT: SQL statement "CALL _timescaledb_functions.cagg_migrate_to_time_bucket(cagg_name)"
I've found a problem and don't have enough bandwidth now to work on it ... so I'll revisit it after finish the batch refresh stuff.
In #6837 we added a migration for CAggs using
time_bucket_ng
to use the new version oftime_bucket
that support orign and/or offset.We made a mistake because we place the code to migrate existing CAggs using
time_bucket_ng
totime_bucket
in the update script but this code should be placed instead on post-update.sql script where we can call functions from the new extension version loaded.Disable-check: force-changelog-file