Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not continue perf tests in case of exception in create_query/fill_query #46362

Merged

Conversation

azat
Copy link
Collaborator

@azat azat commented Feb 13, 2023

Previously due to using of threading.Thread, exception had been ignored, and perf test simply continues, yes it will show some errors, but it also may show that the test became faster, while it is because the underlying table was empty.

Replace threading.Thread with a SafeThread (inline implementation) that simply rethrow exception in join.

Here is an example of such report 1, look at the ip_trie test.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

…query

Previously due to using of threading.Thread, exception had been
ignored, and perf test simply continues, yes it will show some errors,
but it also may show that the test became faster, while it is because
the underlying table was empty.

Replace threading.Thread with a SafeThread (inline implementation) that
simply rethrow exception in join.

Here is an example of such report [1], look at the ip_trie test.

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/45654/2101b66570cbb9eb9a492afa8ab82d562c34336b/performance_comparison_[3/4]/report.html

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Feb 13, 2023
@vdimir vdimir self-assigned this Feb 14, 2023
@robot-ch-test-poll1 robot-ch-test-poll1 merged commit eb58042 into ClickHouse:master Feb 14, 2023
@azat azat deleted the tests/perf-create-errors-fix branch February 14, 2023 13:22
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.

None yet

4 participants