Skip to content

Fix flaky cagg_hierarchical_concurrent_refresh - #10349

Merged
melihmutlu merged 1 commit into
timescale:mainfrom
melihmutlu:cagg_hier_flaky_fix
Jul 31, 2026
Merged

Fix flaky cagg_hierarchical_concurrent_refresh#10349
melihmutlu merged 1 commit into
timescale:mainfrom
melihmutlu:cagg_hier_flaky_fix

Conversation

@melihmutlu

Copy link
Copy Markdown
Member

Synchronize with a lock on cagg_6h's materialization hypertable instead. L1 cagg refresh is blocked on the lock mid-txn 3 after deleting its processed log entries but before the commit. While L2 moves and process its log entries concurrently, blocking only when it starts materializing from cagg_6h.

@melihmutlu
melihmutlu requested a review from a team July 28, 2026 14:47
@github-actions
github-actions Bot requested a review from dbeck July 28, 2026 14:52
@github-actions

Copy link
Copy Markdown

@dbeck, @svenklemm: please review this pull request.

Powered by pull-review

@github-actions
github-actions Bot requested a review from svenklemm July 28, 2026 14:52
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@melihmutlu
melihmutlu force-pushed the cagg_hier_flaky_fix branch from e582aa4 to 7547859 Compare July 28, 2026 16:56
Comment thread tsl/test/isolation/specs/cagg_hierarchical_concurrent_refresh.spec
@melihmutlu
melihmutlu force-pushed the cagg_hier_flaky_fix branch 2 times, most recently from 9a083dd to 1cdd927 Compare July 31, 2026 12:50
step unlock_6h_mat_table:
ROLLBACK;

<waiting ...>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why we see waiting here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it waits for a previous refresh to complete, "unlock_6h_mat_table"("L2_refresh_full"). Maybe that's not necessary.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That dependency just makes sure that unlock happens only after L2_refresh_full starts, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was to have the same output order in every run. But that wasn't needed for unlock_6h_mat_table, so I removed it.

@melihmutlu
melihmutlu force-pushed the cagg_hier_flaky_fix branch from 1cdd927 to 87af8e0 Compare July 31, 2026 13:14
cagg_1d|Mon Jan 05 00:00:00 2026 UTC|infinity
cagg_6h|-infinity |Wed Dec 31 23:59:59.999999 2025 UTC
cagg_6h|Fri Jan 02 00:00:00 2026 UTC|Sat Jan 03 23:59:59.999999 2026 UTC
cagg_6h|Fri Jan 02 00:00:00 2026 UTC|Fri Jan 02 23:59:59.999999 2026 UTC

@gayyappan gayyappan Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see this diff. I believe this is executing in batches and can cause non-deterministic behavior again. Could you check if we need to force setting as num_batches=0

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it runs in batches. But the diff is because there is an additional refresh in the beginning (L1b_refresh_jan3) to generate some invalidations for L2 refresh. So Jan 3 is already refreshed which wasn't the case previously.

# Once L1 is blocked in txn3 and still holding its lock on the materialization invalidation
# log, since it hasn't committed yet, run L2's refresh. L2 should get through txn1 and txn2
# without any issue despite that lock. Once L2 also hits the block in txn3, release it.
#

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some more comments are helpful here:
L1_refresh_jan1 refreshes Jan1. But is blocked in Txn3 and so changes are not yet materialized when L2_refresh_full is in Txn2. So Jan1 changes are not visible to L2 and it only updates data for Jan3.

@melihmutlu
melihmutlu force-pushed the cagg_hier_flaky_fix branch from 87af8e0 to 862548f Compare July 31, 2026 13:51

@gayyappan gayyappan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request for an additional comment

@melihmutlu
melihmutlu enabled auto-merge (squash) July 31, 2026 13:53
@melihmutlu
melihmutlu disabled auto-merge July 31, 2026 13:54
Synchronize with a lock on cagg_6h's materialization hypertable instead.
L1 cagg refresh is blocked on the lock mid-txn 3 after deleting its
processed log entries but before the commit. While L2 moves and process
its log entries concurrently, blocking only when it starts materializing
from cagg_6h.
@melihmutlu
melihmutlu force-pushed the cagg_hier_flaky_fix branch from 862548f to 5688c6b Compare July 31, 2026 13:57
@melihmutlu
melihmutlu enabled auto-merge (squash) July 31, 2026 13:57
@melihmutlu melihmutlu added the force-auto-backport Automatically backport this PR or fix of this issue, even if it's not marked as "bug" label Jul 31, 2026
@melihmutlu
melihmutlu merged commit 62575c3 into timescale:main Jul 31, 2026
76 of 78 checks passed
@melihmutlu
melihmutlu deleted the cagg_hier_flaky_fix branch July 31, 2026 15:05
timescale-automation pushed a commit that referenced this pull request Jul 31, 2026
Synchronize with a lock on cagg_6h's materialization hypertable instead.
L1 cagg refresh is blocked on the lock mid-txn 3 after deleting its
processed log entries but before the commit. While L2 moves and process
its log entries concurrently, blocking only when it starts materializing
from cagg_6h.

(cherry picked from commit 62575c3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported-2.29.x force-auto-backport Automatically backport this PR or fix of this issue, even if it's not marked as "bug"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants