Fix flaky test_overcommit_tracker/test_user_overcommit#100567
Closed
alexey-milovidov wants to merge 1 commit intomasterfrom
Closed
Fix flaky test_overcommit_tracker/test_user_overcommit#100567alexey-milovidov wants to merge 1 commit intomasterfrom
alexey-milovidov wants to merge 1 commit intomasterfrom
Conversation
The test is probabilistic: it relies on memory pressure to kill queries with low `memory_overcommit_ratio_denominator` while sparing those with high ratio. Under MSan (which has ~3x memory overhead), a single attempt could kill all queries, causing the assertion to fail. Two changes: - Reduce `numbers(2500000)` to `numbers(1000000)` to lower per-query memory usage, making it more likely that B queries survive. - Add a retry loop (up to 5 attempts) since the test is inherently probabilistic. https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100404&sha=4e239671fdc3523725369f5ad6028b4a52c43b45&name_0=PR&name_1=Integration%20tests%20%28amd_msan%2C%204%2F6%29 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Workflow [PR], commit [edf15b4] Summary: ✅ AI ReviewSummaryThis PR reduces memory load and adds retries in Findings
ClickHouse Rules
Final Verdict
|
|
|
||
| USER_TEST_QUERY_A = "SELECT groupArray(number) FROM numbers(2500000) SETTINGS max_memory_usage_for_user=2000000000,memory_overcommit_ratio_denominator=1" | ||
| USER_TEST_QUERY_B = "SELECT groupArray(number) FROM numbers(2500000) SETTINGS max_memory_usage_for_user=2000000000,memory_overcommit_ratio_denominator=80000000" | ||
| USER_TEST_QUERY_A = "SELECT groupArray(number) FROM numbers(1000000) SETTINGS max_memory_usage_for_user=2000000000,memory_overcommit_ratio_denominator=1" |
Contributor
There was a problem hiding this comment.
numbers(...) from 2500000 to 1000000, but the test still asserts only that at least one B query finishes. With lower memory pressure, it can pass even when overcommit selection never happens (e.g. all queries succeed), so it may stop validating the intended behavior.
Please also assert that at least one low-priority A query is rejected with MEMORY_LIMIT_EXCEEDED in a successful attempt (or otherwise prove overcommit arbitration happened).
1 task
1 task
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fix flaky
test_overcommit_tracker/test_user_overcommitunder MSan.The test is probabilistic: it relies on memory pressure to kill queries with low
memory_overcommit_ratio_denominatorwhile sparing those with high ratio. Under MSan (~3x memory overhead), all queries could get killed in a single attempt.Changes:
numbers(2500000)tonumbers(1000000)to lower per-query memory usageCI report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100404&sha=4e239671fdc3523725369f5ad6028b4a52c43b45&name_0=PR&name_1=Integration%20tests%20%28amd_msan%2C%204%2F6%29
Related issue: #45173
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
...
Documentation entry for user-facing changes