Skip to content

Randomize keeper fault injection settings in stress tests#43187

Merged
devcrafter merged 3 commits intomasterfrom
igor/keeper_fault_injection_in_stress_test
Nov 13, 2022
Merged

Randomize keeper fault injection settings in stress tests#43187
devcrafter merged 3 commits intomasterfrom
igor/keeper_fault_injection_in_stress_test

Conversation

@devcrafter
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Follow up #42607 #43122

@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 11, 2022
@davenger davenger self-assigned this Nov 13, 2022
@devcrafter
Copy link
Copy Markdown
Member Author

AST fuzzer - created #43202

Integration test failures:

@devcrafter devcrafter merged commit 7386703 into master Nov 13, 2022
@devcrafter devcrafter deleted the igor/keeper_fault_injection_in_stress_test branch November 13, 2022 22:15
@tavplubix
Copy link
Copy Markdown
Member

There are two issues with this PR:

  • SettingsRandomizer in clickhouse-test is used for functional tests, so this PR enables fault injections in functional tests (seems like it was not expected). Yes, it randomizes settings in stress tests too, but as a side effect (because stress tests just run functional tests in many threads).
  • Random settings are used in BC check as well. So we have the same issue as we had in Keeper retries during insert (clean) #42607 (comment). All queries in BC check fail with Code: 115. DB::Exception: Setting insert_keeper_max_retries is neither a builtin setting nor started with the prefix 'custom_' registered for user-defined settings. (UNKNOWN_SETTING) (you can check clickhouse-server.backward.stress.log), so it tests nothing. Unfortunately, our scripts for stress tests cannot detect this automatically, so checks were green.

@davenger
Copy link
Copy Markdown
Member

@tavplubix has very valid points, we have to revert this change :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants