Skip to content

Fix failed stress test (OpenTelemetry)#41050

Merged
rschu1ze merged 3 commits intoClickHouse:masterfrom
FrankChen021:exception_safe
Sep 8, 2022
Merged

Fix failed stress test (OpenTelemetry)#41050
rschu1ze merged 3 commits intoClickHouse:masterfrom
FrankChen021:exception_safe

Conversation

@FrankChen021
Copy link
Copy Markdown
Contributor

@FrankChen021 FrankChen021 commented Sep 6, 2022

Fixes the stress test problem mentioned at: #39010 (comment)

The solution is to move addAttribute call from ctor to dtor so that both ctor and dtor are exception safe.

cc @rschu1ze @tavplubix @qoega

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Signed-off-by: Frank Chen <[email protected]>
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 6, 2022
@rschu1ze rschu1ze self-assigned this Sep 6, 2022
Signed-off-by: Frank Chen <[email protected]>
Signed-off-by: Frank Chen <[email protected]>
@rschu1ze rschu1ze changed the title Fix failed stress test Fix failed stress test (OpenTelemetry) Sep 7, 2022
@rschu1ze rschu1ze removed the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 7, 2022
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 7, 2022
@antonio2368 antonio2368 added the can be tested Allows running workflows for external contributors label Sep 7, 2022
@novikd novikd mentioned this pull request Sep 7, 2022
@FrankChen021
Copy link
Copy Markdown
Contributor Author

There's a test in Stateless tests (ubsan) that still fails due to clickhouse-server does not exist.

I didn't find any error messages related to the opentelemetry in the clickhouse-server.err.log.
Not sure what causes the failure.

@rschu1ze
Copy link
Copy Markdown
Member

rschu1ze commented Sep 8, 2022

I didn't find any error messages related to the opentelemetry in the clickhouse-server.err.log. Not sure what causes the failure.

I agree --> merging.

@rschu1ze rschu1ze merged commit 6880885 into ClickHouse:master Sep 8, 2022
@azat
Copy link
Copy Markdown
Member

azat commented Sep 8, 2022

There's a test in Stateless tests (ubsan) that still fails due to clickhouse-server does not exist.

Actually it fails because of, likely, server was too slow to respond to the ping queries from clickhouse-test, I've seen this before.

@FrankChen021 FrankChen021 deleted the exception_safe branch September 8, 2022 09:22
@tavplubix
Copy link
Copy Markdown
Member

Stateless tests (ubsan) - actually it's #40410

@FrankChen021
Copy link
Copy Markdown
Contributor Author

Good to hear that, thank you for your information @tavplubix

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

Labels

can be tested Allows running workflows for external contributors 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.

7 participants