Refactor continuous aggregate materialization code #7505
Merged
Conversation
This file contains 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
fabriziomello
added
continuous_aggregate
tech-debt
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7505 +/- ##
==========================================
+ Coverage 80.06% 82.19% +2.12%
==========================================
Files 190 230 +40
Lines 37181 43250 +6069
Branches 9450 10875 +1425
==========================================
+ Hits 29770 35548 +5778
- Misses 2997 3385 +388
+ Partials 4414 4317 -97 ☔ View full report in Codecov by Sentry. |
fabriziomello
force-pushed
the
cagg_refactor_materialize
branch
5 times, most recently
from
6e70ed9 to
4c47fdb
Compare
|
@akuzm, @gayyappan: please review this pull request.
|
fabriziomello
force-pushed
the
cagg_refactor_materialize
branch
2 times, most recently
from
64bc413 to
94cdd5b
Compare
mkindahl
reviewed
Dec 6, 2024
There was a problem hiding this comment.
Would suggest that you modify the way plans are prepared as outlined below. Also one bug that will cause problems later (a missing "volatile"). Also a bunch of nits. but that is entirely your decision if you want to use them.
You might also switch to use function pointers and a dispatch table, which probably makes the code a little easier to maintain and also make it a little faster, but that is optional.
fabriziomello
force-pushed
the
cagg_refactor_materialize
branch
from
94cdd5b to
67a0e4c
Compare
mkindahl
approved these changes
Dec 10, 2024
gayyappan
approved these changes
Dec 10, 2024
fabriziomello
force-pushed
the
cagg_refactor_materialize
branch
2 times, most recently
from
da55567 to
c0e6efc
Compare
erimatnor
approved these changes
Dec 11, 2024
In addition to the code reorganization and simplification we changed it by splitting the query execution in multiple steps taking advantage of `SPI_prepare`, `SPI_execute_plan` and `SPI_freeplan`: * create_materialization_plan(PlanType) * execute_materialization_plan(PlanType) * free_materialization_plan(PlanType) This PR is in preparation for a following PR to execute the materialization in small batches to alleviate the I/O spikes when reading and writing many buckets.
fabriziomello
force-pushed
the
cagg_refactor_materialize
branch
from
c0e6efc to
050c9e4
Compare
fabriziomello
added a commit
to fabriziomello/timescaledb
that referenced
this pull request
Dec 11, 2024
In timescale#7505 we used PG_TRY() .. PG_FINALLY() but some CI configurations was not happy with it reporting "variable might be clobbered by longjmp". Fixed it by using PG_TRY() .. PG_CATCH() instead. https://github.com/timescale/timescaledb/actions/runs/12277548387/job/34257269473#step:13:206
akuzm
pushed a commit
that referenced
this pull request
Dec 12, 2024
In #7505 we used PG_TRY() .. PG_FINALLY() but some CI configurations was not happy with it reporting "variable might be clobbered by longjmp". Fixed it by using PG_TRY() .. PG_CATCH() instead. https://github.com/timescale/timescaledb/actions/runs/12277548387/job/34257269473#step:13:206 Disable-check: force-changelog-file
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.
Refactor continuous aggregate materialization code
In addition to the code reorganization and simplification we changed it by splitting the query execution in multiple steps taking advantage of
SPI_prepare,SPI_execute_planandSPI_freeplan:This PR is in preparation for a following PR to execute the materialization in small batches to alleviate the I/O spikes when reading and writing many buckets.
Disable-check: force-changelog-file