Improve hypertable DML performance with chunk exclusion#9315
Improve hypertable DML performance with chunk exclusion#9315
Conversation
45bfb0e to
15758a1
Compare
2e19fc6 to
2981e5d
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
5f34e38 to
4773fb5
Compare
| { | ||
| if (IS_UPDL_CMD(context->rootquery)) | ||
| { | ||
| if (rti == (Index) query->resultRelation) | ||
| rte_mark_for_expansion(rte); | ||
| } | ||
| else if (query->resultRelation == 0) | ||
| rte_mark_for_expansion(rte); | ||
| } |
There was a problem hiding this comment.
Let's explain this new logic in the comments (i.e. what every branch condition means and why we handle only these cases and not others).
src/import/allpaths.c
Outdated
| Node *childvar = (Node *) lfirst(childvars); | ||
|
|
||
| if (IsA(parentvar, Var)) | ||
| /* Only process Vars that belong to the parent rel */ |
There was a problem hiding this comment.
This seems like a general change not related to update/delete specifically -- why did it become necessary? The check below should probably filter out the different relations already.
There was a problem hiding this comment.
It is a direct port from upstream allpaths.c. https://github.com/postgres/postgres/blob/c2c1962a64b547412c88fa2728e4fa35e65f4c90/src/backend/optimizer/path/allpaths.c#L1193
Without this fix, The tests failing with the "WARNING: problem in alloc set MessageContext: req size > alloc size for chunk" in Debug mode PG18.2 with --enable-cassert:
- cagg_invalidation — UPDATE on inval_update table triggered the warning
- cagg_watermark — also showed the warning during UPDATE/DELETE
- triggers — also showed the warning during UPDATE/DELETE
Here is a simple test reproducing the above warning without the fix on debug builds
CREATE EXTENSION timescaledb;
DROP TABLE IF EXISTS transition_test;
CREATE OR REPLACE FUNCTION test_trigger() RETURNS TRIGGER AS $$
BEGIN RAISE WARNING 'trigger fired'; RETURN NULL; END;
$$ LANGUAGE plpgsql;
CREATE TABLE transition_test(time timestamptz NOT NULL);
SELECT create_hypertable('transition_test','time');
INSERT INTO transition_test VALUES ('2020-01-10');
-- These trigger "problem in alloc set MessageContext: req size > alloc size"
UPDATE transition_test SET time = '2020-01-12' WHERE time = '2020-01-11';
There was a problem hiding this comment.
Oh, it's for the ROWID_VAR = -4. I think it only reproduces with update/delete/merge, this is why we're not hitting it now. I'm not sure, maybe it makes sense to update the entire function to the latest PG version? Just to have a consistent shapshot and make sure we're not missing anything when changing it piece-wise.
There was a problem hiding this comment.
Yes, now I synched the latest code.
There was a problem hiding this comment.
Let's update it separately. I'd like to move in small steps to see precisely when we're breaking something. I'll do it myself here: #9354
| } | ||
| } | ||
|
|
||
| if (ts >= TS_TIMESTAMP_END) |
There was a problem hiding this comment.
Here too -- general change not immediately related to update/delete, is this something we're missing on the select paths too?
There was a problem hiding this comment.
Without this fix, extreme date/timestamp values (e.g. year 294247) would error instead of clamping to INT64_MIN/MAX during chunk exclusion.
cagg_union_view was the failing test. It inserts '294247-01-01' into timestamp/timestamptz hypertables, then CALL refresh_continuous_aggregate('timestamp_agg', NULL, NULL) errors with "timestamp out of
range".
DELETE FROM test_large_ts WHERE time < '294247-01-02' simple DELETE with an overflowing timestamp in the WHERE clause reproduces ERROR: timestamp out of range.
There was a problem hiding this comment.
Let's try to reproduce this with pure SELECT and fix separately then. This PR touches a very important part of code, so it would be good to keep all independent changes out of it.
There was a problem hiding this comment.
Sure, I could reproduce with pure SELECT too. PR is on the way! Thank you!
src/planner/expand_hypertable.c
Outdated
| /* Parent is not a leaf result rel; children are. */ | ||
| root->leaf_result_relids = bms_del_member(root->leaf_result_relids, rti); | ||
|
|
||
| foreach (lc, appinfos) |
There was a problem hiding this comment.
At this point, we're basically reimplementing all of the pieces of expand_single_inheritance_child, but in different places. Can we make this more straightforward and use this function entirely?
There was a problem hiding this comment.
@akuzm expand_single_inheritance_child is a static function. Do you mean to copy into our code and use it?
There was a problem hiding this comment.
Yes. This is a maintainability problem, but sometimes we have do this for static functions. We try to maintain a pure copy of the PG code w/o any changes, so that it's easier to keep up to date.
There was a problem hiding this comment.
It should go somewhere to the "import" dir like the similar ones.
There was a problem hiding this comment.
@akuzm expand_single_inheritance_child is imported into src/import/ts_inherit.c as ts_expand_single_inheritance_child.
There was a problem hiding this comment.
I'm going to try to merge this separately too: #9373
ac13d22 to
8d5de8d
Compare
8d5de8d to
0e3214f
Compare
|
Another thing that is relevant for this PR: while looking into it, I noticed that row mark handling is not implemented in our expansion. This is expected, since at the moment it's not used if there are row marks. The only exception was expansion for FKs, and I'm working on a fix for this: #9324 Given that you're not hitting test failures with this, probably the test coverage is missing -- can you add some tests for this? This case is pretty important. I'd expect the tests to fail w/o my fix and pass with it. |
Thinking more about this, it seems to be a different thing. Row marks are not used on the DML target relation. |
Postgres's standard inheritance expansion opens every chunk of a hypertable during UPDATE/DELETE planning, which causes severe performance degradation as chunk count grows. This patch routes UPDATE/DELETE through TimescaleDB's existing chunk exclusion code (HypertableRestrictInfo), expanding only chunks that match the query's WHERE clause — the same optimization already used for SELECT. The key change is in planner.c's RTE marking logic: UPDATE/DELETE target relations are now marked for TimescaleDB's custom expansion (via `rte_mark_for_expansion`), whereas previously they were left to Postgres. MERGE is intentionally excluded since our custom expansion only handles UPDATE/DELETE DML fixups. The per-chunk expansion in expand_hypertable.c is refactored to use a copied `expand_single_inheritance_child` (from PG's inherit.c), which handles RTE construction, AppendRelInfo creation, result relation registration, and row identity setup — replacing ~60 lines of hand-rolled equivalent code. Also fixes two pre-existing bugs exposed by this change: - ts_set_append_rel_size: missing varno check in width estimation caused out-of-bounds writes when ROWID_VAR entries were present in the parent rel's reltarget. Ported from upstream PG. - ts_time_value_to_internal_or_infinite: extreme date/timestamp values would error instead of clamping to INT64_MIN/MAX. Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
|
Looks like we don't have any queries that are affected by this in TSBench. But also no regressions: https://grafana.dev-us-east-1.ops.dev.timescale.com/d/fasYic_4z/compare-benchmark-runs?orgId=1&var-run1=5385&var-run2=5382&var-postgres=16&var-branch=All&var-threshold=0.02&var-use_historical_thresholds=true&var-threshold_expression=2.0%20%2A%20percentile_cont%280.90%29&var-exact_suite_version=true |
I didn't knew we have an infra to perform benchmarks. I will add one. Thank you. |
|
Left-side: 2.25.1 |
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
…te-delete-slowness
src/planner/planner.c
Outdated
| * MERGE is not marked: our custom expansion in | ||
| * expand_hypertable.c only handles UPDATE/DELETE | ||
| * DML fixups. MERGE is left to PG's standard |
There was a problem hiding this comment.
Should we enable it for MERGE too, or are there more problems with it?
There was a problem hiding this comment.
MERGE can also lead to INSERTS right? AFAIK, we have separate path for INSERT and I'm not really sure how to wire those together now.
I would rather simply focus on the UPDATE and DELETE in this PR.
There was a problem hiding this comment.
I'm not familiar with the details. The comment sounds a little confusing like this, I think it would be more straightforward if it mentioned some actual reason or just didn't mention it at all.
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
…te-delete-slowness
…te-delete-slowness
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
…te-delete-slowness
130c308 to
9ba1b77
Compare
…te-delete-slowness
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
7cb0964 to
a30845e
Compare
Signed-off-by: Arunprasad Rajkumar <ar.arunprasad@gmail.com>
Postgres's standard inheritance expansion opens every chunk of a hypertable during UPDATE/DELETE planning, which causes severe performance degradation as chunk count grows. This patch routes
UPDATE/DELETE through TimescaleDB's existing chunk exclusion code (HypertableRestrictInfo), expanding only chunks that match the query's WHERE clause — the same optimization already used for SELECT.
The key change is in planner.c's RTE marking logic: UPDATE/DELETE target relations are now marked for TimescaleDB's custom expansion (via rte_mark_for_expansion), whereas previously they were left to Postgres. MERGE is intentionally excluded since our custom expansion only handles UPDATE/DELETE DML fixups.
The per-chunk expansion in expand_hypertable.c is refactored to use a copied expand_single_inheritance_child (from PG's inherit.c), which handles RTE construction, AppendRelInfo creation, result relation registration, and row identity setup — replacing ~60 lines of hand-rolled equivalent code.
Also fixes one of pre-existing bug exposed by this change:
caused out-of-bounds writes when ROWID_VAR entries were present
in the parent rel's reltarget. Ported from upstream PG.
CPU profile before the fix(2.25.1):

CPU profile after the fix(2.26.0-dev):

Perf Profile Comparison: Chunk Exclusion for DML
Overall CPU Time
Key Hotspots Eliminated
The non optimized profile is dominated by planner overhead
pg_strtokstrtollAllocSetAllocrelation_excluded_by_constraintsexpression_tree_walkercopyObjectImplget_relation_infoSingle-Row UPDATE Benchmark: TimescaleDB 2.25.1 vs 2.26.0-dev
(id, time)Total Loop Time for 50 Single-Row UPDATEs Across 5 Chunks (ms)
Total Loop Time for 50 Single-Row UPDATEs Within 1 Chunk (ms)
Planning Time Per Query Comparison (ms)
Planning Buffer Hits Per Query
In 2.25.1 the compressed HT custom plan touches 2,609 shared buffers during planning (scanning all chunk metadata). This overhead is eliminated in 2.26.0-dev via chunk exclusion for DML.
Key Takeaways
Hypertable UPDATEs with custom plan are ~11x faster.
Planning time dropped from ~2.1ms to ~0.12ms per query. Chunk exclusion for DML now prunes irrelevant chunks during planning rather than scanning all 200.
Compressed Hypertable UPDATEs with custom plan are ~25x faster.
This was the worst case in 2.25.1 - each custom plan touched 2,609 shared buffers to inspect compressed chunk metadata. In 2.26.0-dev, chunk exclusion eliminates this entirely.
automode now correctly picks the fast custom plan.In 2.25.1,
automode chose generic plan for uncompressed HT (~53ms) but was stuck with expensive custom plan for compressed HT (~218ms). In 2.26.0-dev,autocorrectly uses custom plan at ~9ms for both, since the custom plan is now cheaper than the generic plan.force_generic_planis unchanged (~53ms).Generic plans skip per-query planning and scan all chunks at execution time. No change is expected here.
Logical Partition and Plain Table are unchanged.
The fix only affects hypertable code paths. These baselines confirm there is no regression.