Skip to content

CI: reduce number of entries in join partners to reduce execution time#91470

Merged
ahmadov merged 3 commits intomasterfrom
ahmadov/fix-gh-86335
Dec 11, 2025
Merged

CI: reduce number of entries in join partners to reduce execution time#91470
ahmadov merged 3 commits intomasterfrom
ahmadov/fix-gh-86335

Conversation

@ahmadov
Copy link
Copy Markdown
Member

@ahmadov ahmadov commented Dec 4, 2025 •
edited
Loading

Test 02177_issue_31009 became flaky because of having a long execution time, so it hit Timeout. Reducing the number of entries in the join partners makes it faster while keeping the actual test case. The history shows that the test was failed always with Timeout.

Closes #86335

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Reduce number of entries in join partners to reduce execution time of the test 02177_issue_31009.

@ahmadov
Reduce number of entries in join partners to reduce execution time
d99ae2d 
Test `02177_issue_31009` became flaky because of having a long
execution time, so it hit `Timeout`. Reducing the number of entries in
the join partners make it faster while keeping the actual test case.
@ahmadov ahmadov requested a review from vdimir December 4, 2025 10:20
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Dec 4, 2025 •
edited by maxknv
Loading

Workflow [PR], commit [b112bc4]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_tsan, s3 storage, parallel) failure
03704_rmv_logging_internal_queries FAIL cidb, issue ISSUE EXISTS

@clickhouse-gh clickhouse-gh bot added the pr-ci label Dec 4, 2025
vdimir
vdimir reviewed Dec 4, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@vdimir vdimir left a comment •
edited
Loading

Choose a reason for hiding this comment

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

When test times out, do we swap joined tables (it can be checked via logs, there should table size estimations there)? Why do we swap in some cases but not in others?

UPD: Ah, I see that number of rows was changed. Why do we need to disable swap still if ratio remains same? Also the fact that test times out may indicate performance degradation, otherwise it's not clear why is started to happen.

@ahmadov
Copy link
Copy Markdown
Member Author

ahmadov commented Dec 8, 2025

When test times out, do we swap joined tables (it can be checked via logs, there should table size estimations there)? Why do we swap in some cases but not in others?

UPD: Ah, I see that number of rows was changed. Why do we need to disable swap still if ratio remains same? Also the fact that test times out may indicate performance degradation, otherwise it's not clear why is started to happen.

yes, also numbers are changed. tbh I'm not sure if disabling the swap would affect the test. Therefore, I'd recommend to keep as it is since it verifies the original issue. However, if you recommend to disable the swap, I'm fine.

@ahmadov ahmadov requested a review from vdimir December 10, 2025 14:42
vdimir
vdimir reviewed Dec 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@vdimir vdimir left a comment

Choose a reason for hiding this comment

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

It may be the case than reduced test does not reproduce original issue #31009 . So, not sure if test makes sense at all

@vdimir
Copy link
Copy Markdown
Member

vdimir commented Dec 10, 2025

Maybe we can identify some randomized setting that makes test to execute longer?

@ahmadov
Copy link
Copy Markdown
Member Author

ahmadov commented Dec 10, 2025 •
edited
Loading

Maybe we can identify some randomized setting that makes test to execute longer?

@vdimir I tried to use the exact settings from failed builds (example). But couldn't reproduce the issue - execution time was consistent and fast (<1s) on my machine. One reason could be max_insert_threads was set to a lower value in some failed builds, therefore I just made it unlimited since we are inserting ~5M rows. I'd merge it and monitor for a few weeks.

@ahmadov
Copy link
Copy Markdown
Member Author

ahmadov commented Dec 11, 2025

@ahmadov ahmadov enabled auto-merge December 11, 2025 14:47
@ahmadov ahmadov added this pull request to the merge queue Dec 11, 2025
Merged via the queue into master with commit 666b310 Dec 11, 2025
128 of 130 checks passed
@ahmadov ahmadov deleted the ahmadov/fix-gh-86335 branch December 11, 2025 14:59
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdimir vdimir vdimir approved these changes

Assignees

@vdimir vdimir

Labels

pr-ci pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test 02177_issue_31009 is flaky

3 participants

@ahmadov @vdimir @robot-ch-test-poll2