Skip to content

Rewrite tests for memory overcommit#43105

Merged
novikd merged 5 commits intomasterfrom
overcommit-test
Nov 11, 2022
Merged

Rewrite tests for memory overcommit#43105
novikd merged 5 commits intomasterfrom
overcommit-test

Conversation

@novikd
Copy link
Copy Markdown
Member

@novikd novikd commented Nov 10, 2022

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Rewrote tests for OvercommitTracker, because they were flaky and didn't check much. Fixes #42629.

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Nov 10, 2022
@antonio2368 antonio2368 self-assigned this Nov 10, 2022
antonio2368
antonio2368 approved these changes Nov 10, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@antonio2368 antonio2368 left a comment

Choose a reason for hiding this comment

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

LGTM
As far as I understood the changes are mostly using memory heavier queries?

tests/queries/0_stateless/02104_overcommit_memory.sh
@@ -1,49 +0,0 @@
#!/usr/bin/env bash
Copy link
Copy Markdown
Member

@antonio2368 antonio2368 Nov 10, 2022

Choose a reason for hiding this comment

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

Why not have a stateless test?

Copy link
Copy Markdown
Member Author

@novikd novikd Nov 10, 2022

Choose a reason for hiding this comment

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

I reimplemented this test as an integrational because it had several problems:

  1. It was flaky.
  2. It didn't respect the limit of parallel queries number.
  3. It was not strict enough.

@novikd
Copy link
Copy Markdown
Member Author

novikd commented Nov 11, 2022

Stateless tests (debug) [2/3] — fail: 4, passed: 1521, skipped: 19
Stateless tests flaky check (asan) — Empty test_results.tsv

Unrelated.

Stress test (ubsan) — Killed by signal (in clickhouse-server.log)

#42740

Stress test (tsan) — Backward compatibility check: Error message in clickhouse-server.log
Stress test (msan) — OOM killer (or signal 9) in clickhouse-server.log
Stress test (asan) — Backward compatibility check: Error message in clickhouse-server.log

#37791

@novikd novikd merged commit 3179428 into master Nov 11, 2022
@novikd novikd deleted the overcommit-test branch November 11, 2022 16:34
@novikd novikd mentioned this pull request Nov 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antonio2368 antonio2368 antonio2368 approved these changes

Assignees

@antonio2368 antonio2368

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test 02104_overcommit_memory is flaky in debug

3 participants

@novikd @antonio2368 @robot-ch-test-poll