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

Debug TestClickHouseConnectorSmokeTest.testRenameSchema failure #10675

Closed
wants to merge 3 commits into from

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Jan 19, 2022

Relates to #10653

@ebyhr ebyhr added the test label Jan 19, 2022
@cla-bot cla-bot bot added the cla-signed label Jan 19, 2022
@ebyhr ebyhr closed this Jan 19, 2022
@ebyhr ebyhr deleted the ebi/clickhouse-rename-schema branch January 19, 2022 06:52
@ebyhr ebyhr restored the ebi/clickhouse-rename-schema branch January 19, 2022 07:04
@ebyhr ebyhr reopened this Jan 19, 2022
@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from af29941 to dd630c8 Compare January 19, 2022 08:40
@ebyhr
Copy link
Member Author

ebyhr commented Jan 19, 2022

2022-01-19T09:03:30.0660256Z 2022-01-19T03:03:29.984-0600 INFO [TEST START] io.trino.plugin.clickhouse.TestClickHouseConnectorSmokeTest.testRenameSchema
2022-01-19T09:03:30.0665559Z 2022-01-19T03:03:29.986-0600 INFO TIMELINE: Query 20220119_090329_00030_ipn3q :: Transaction:[271ad79c-c790-4669-b069-09e1e5988aab] :: elapsed 90ms :: planning 24ms :: waiting 13ms :: scheduling 35ms :: running 17ms :: finishing 14ms :: begin 2022-01-19T03:03:29.889-06:00 :: end 2022-01-19T03:03:29.979-06:00
2022-01-19T09:03:30.0668634Z 2022-01-19T03:03:30.019-0600 INFO TIMELINE: Query 20220119_090329_00031_ysgkv :: Transaction:[29168c5b-9df3-4eea-968d-68092d385662] :: elapsed 19ms :: planning 0ms :: waiting 0ms :: scheduling 19ms :: running 0ms :: finishing 19ms :: begin 2022-01-19T03:03:29.992-06:00 :: end 2022-01-19T03:03:30.011-06:00
...
2022-01-19T09:03:30.0876659Z 2022-01-19T03:03:30.023-0600 INFO name: test_rename_schema_gmlk6e5eao, engine: Atomic, data_path: /var/lib/clickhouse/store/, metadata_path: /var/lib/clickhouse/store/a98/a98f6c70-94a3-4781-a98f-6c7094a31781/, uuid: a98f6c70-94a3-4781-a98f-6c7094a31781
...
2022-01-19T09:03:30.0879891Z 2022-01-19T03:03:30.062-0600 INFO TIMELINE: Query 20220119_090329_00030_ysgkv :: Transaction:[0f204999-a35b-4ccd-952e-d2672d5bc92c] :: elapsed 144ms :: planning 70ms :: waiting 17ms :: scheduling 39ms :: running 23ms :: finishing 12ms :: begin 2022-01-19T03:03:29.914-06:00 :: end 2022-01-19T03:03:30.058-06:00
2022-01-19T09:03:30.0880948Z 2022-01-19T03:03:30.064-0600 INFO [TEST SUCCESS] io.trino.plugin.clickhouse.TestClickHouseConnectorSmokeTest.testPredicate; (took: 0.4 seconds)

https://github.com/trinodb/trino/runs/4864994772
https://github.com/trinodb/trino/runs/4864994860

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from dd630c8 to 9cb540e Compare January 20, 2022 07:09
@ebyhr
Copy link
Member Author

ebyhr commented Jan 20, 2022

2022-01-20T07:37:08.4884981Z 2022-01-20T01:37:08.474-0600 SEVERE [TEST FAILURE] io.trino.plugin.clickhouse.TestAltinityConnectorSmokeTest.testRenameSchema; (took: 0.1 seconds)
2022-01-20T07:37:08.4885463Z java.lang.AssertionError: 
2022-01-20T07:37:08.4885714Z Expecting message:
2022-01-20T07:37:08.4886178Z   <"ClickHouse exception, code: 48, host: localhost, port: 49161; Code: 48, e.displayText() = DB::Exception: Ordinary: RENAME DATABASE is not supported (version 20.8.19.4 (official build))
2022-01-20T07:37:08.4886550Z ">
2022-01-20T07:37:08.4886721Z to match regex:
2022-01-20T07:37:08.4886997Z   <"ClickHouse exception, code: 62,.* Syntax error.*\n">
2022-01-20T07:37:08.4887241Z but did not.

https://github.com/trinodb/trino/runs/4878762262

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from 9cb540e to aff2284 Compare January 20, 2022 08:37
@ebyhr
Copy link
Member Author

ebyhr commented Jan 20, 2022

@zhicwu @den-crane Do you know if ClickHouse jdbc driver looks for other ports in some situations? The above failure is weird. The server version in TestAltinityConnectorSmokeTest is 20.3.12-aes, but the message comes from other servers 20.8.19.4.

@zhicwu
Copy link
Contributor

zhicwu commented Jan 20, 2022

@zhicwu @den-crane Do you know if ClickHouse jdbc driver looks up other ports in some situations? The above failure is weird. The server version in TestAltinityConnectorSmokeTest is 20.3.12-aes, but the message comes from other servers 20.8.19.4.

Not to my knowledge. Just tried the image, server version should be 20.3.12.112. Needs to debug to understand what happened.

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch 6 times, most recently from 5a5c116 to 56f68f4 Compare January 25, 2022 03:00
@ebyhr
Copy link
Member Author

ebyhr commented Jan 25, 2022

47 successful and 6 failing checks without @Test annotation.

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from 299aba4 to 7448dee Compare January 25, 2022 11:25
@ebyhr
Copy link
Member Author

ebyhr commented Jan 26, 2022

51 successful and 1 failing checks https://github.com/trinodb/trino/runs/4945020952

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from 573bd86 to b3c7060 Compare January 26, 2022 00:42
@ebyhr
Copy link
Member Author

ebyhr commented Jan 26, 2022

52 successful and 1 failing checks https://github.com/trinodb/trino/runs/4945420015?check_suite_focus=true
TestClickHouseConnectorSmokeTest connected to 21.8.13.1.altinitystable

@ebyhr
Copy link
Member Author

ebyhr commented Jan 26, 2022

Another possible cause is testcontainers returns wrong ports.

@zhicwu
Copy link
Contributor

zhicwu commented Jan 26, 2022

Another possible cause is testcontainers returns wrong ports.

Might be. Perhaps we can try GenericContainer first? I can only check when I'm at home, maybe tonight :p

dockerContainer = new GenericContainer<>(CLICKHOUSE_IMAGE).withExposedPorts(HTTP_PORT)
                .waitingFor(Wait.forHttp("/ping").forPort(HTTP_PORT).forStatusCode(200)
                        .withStartupTimeout(Duration.of(60, SECONDS)));

@ebyhr
Copy link
Member Author

ebyhr commented Jan 26, 2022

ClickHouseContainer is just a thin wrapper of GenericContainer, so the replacement basically won't resolve in my guess.

@zhicwu
Copy link
Contributor

zhicwu commented Jan 26, 2022

Is this only reproducible on CI when running the tests in parallel for multiple times? I tried below on a VM running clickhouse/clickhouse-server:21.12 a few times but didn't find any issue.

nohup ./mvnw test -B --strict-checksums -Dair.check.skip-all --fail-at-end -pl :trino-clickhouse &
./mvnw test -B --strict-checksums -Dair.check.skip-all --fail-at-end -pl :trino-clickhouse

Besides fixing the checkstyle issue, I changed docker image from altinity/clickhouse-server:21.8.13.1.altinitystable to 21.8.12.29.altinitystable because I had trouble pulling the former one. I also changed dependency to below but it's irrelevant.

<dependency>
    <groupId>com.clickhouse</groupId>
    <artifactId>clickhouse-jdbc</artifactId>
    <!-- changing to patch3 will fail decimal test case somehow -->
    <version>0.3.2-patch1</version>
    <classifier>shaded</classifier>
    <exclusions>
        <exclusion>
            <groupId>*</groupId>
            <artifactId>*</artifactId>
        </exclusion>
    </exclusions>
</dependency>

@ebyhr
Copy link
Member Author

ebyhr commented Jan 27, 2022

Is this only reproducible on CI when running the tests in parallel for multiple times?

Yes

@cla-bot
Copy link

cla-bot bot commented Jan 28, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@cla-bot cla-bot bot removed the cla-signed label Jan 28, 2022
@cla-bot
Copy link

cla-bot bot commented Jan 28, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from 54e6d67 to 9097b52 Compare January 28, 2022 05:40
@cla-bot
Copy link

cla-bot bot commented Jan 28, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from 9097b52 to 7ffe9c4 Compare January 31, 2022 02:57
@cla-bot cla-bot bot added the cla-signed label Jan 31, 2022
@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch 2 times, most recently from 356a8f9 to d914804 Compare February 1, 2022 01:13
@ebyhr ebyhr force-pushed the ebi/clickhouse-rename-schema branch from d914804 to 6b27951 Compare February 2, 2022 03:15
@ebyhr ebyhr closed this Feb 3, 2022
@ebyhr ebyhr deleted the ebi/clickhouse-rename-schema branch February 3, 2022 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants