Skip to content

Add BaseConnectorTest for Glue catalog in Iceberg#11746

Closed
ebyhr wants to merge 5 commits intomasterfrom
ebi/iceberg-glue-bct
Closed

Add BaseConnectorTest for Glue catalog in Iceberg#11746
ebyhr wants to merge 5 commits intomasterfrom
ebi/iceberg-glue-bct

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented Apr 1, 2022

Description

Add BaseConnectorTest for Glue catalog in Iceberg

Documentation

(x) No documentation is needed.

Release notes

(x) No release notes entries required.

@ebyhr ebyhr added test no-release-notes This pull request does not require release notes entry labels Apr 1, 2022
@cla-bot cla-bot bot added the cla-signed label Apr 1, 2022
@ebyhr ebyhr marked this pull request as draft April 1, 2022 03:58
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 2 times, most recently from c3cd734 to 83d5496 Compare April 1, 2022 06:16
Comment on lines +129 to +130
Logging logging = Logging.initialize();
logging.setLevel("org.apache.iceberg.BaseTableScan", Level.WARN); // To suppress huge "Scanning table ... filter" message
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

temporary?

assertQuery("SELECT * FROM test_rollback ORDER BY col0", "VALUES (123, CAST(987 AS BIGINT)), (456, CAST(654 AS BIGINT))");

assertUpdate(format("CALL system.rollback_to_snapshot('tpch', 'test_rollback', %s)", afterFirstInsertId));
assertUpdate(format("CALL system.rollback_to_snapshot('%s', 'test_rollback', %s)", getSession().getSchema().orElseThrow(), afterFirstInsertId));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

extract schema variable

* on ways to set your AWS credentials which will be needed to run this test.
*/
public class TestIcebergGlueConnectorTest
extends BaseIcebergConnectorTest
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BaseIcebergConnectorTest doesn't randomize names and doesn't drop tables in try-finally, because it has not been used on shared resources until now.

@findepi
Copy link
Copy Markdown
Member

findepi commented Apr 1, 2022

I wasn't aware that we have TestIcebergGlueCatalogConnectorSmokeTest, so BCST instead of BCT.
I guess we don't need both. BCT adds concurrency-related test coverage, but also takes much much more time.

@alexjo2144 @ebyhr @losipiuk @electrum @phd3 @hashhar wdyt?

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Apr 1, 2022

Managing only BCST looks good to me. Sadly, the Glue BCT took more than 1h with 1 thread in my laptop.

@findepi
Copy link
Copy Markdown
Member

findepi commented Apr 1, 2022

wow. How much does it take on the CI?

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Apr 1, 2022

I can't see the additional CI time because it's failing with OOM on the way (around testRepartitionDataOnCtas & testRepartitionDataOnInsert)

@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 4 times, most recently from c0336bd to 3ac5513 Compare April 8, 2022 15:46
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 2 times, most recently from 4849bec to 16ded08 Compare April 20, 2022 02:53
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 3 times, most recently from 403381a to 8885f23 Compare May 2, 2022 05:19
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch from 8885f23 to 81e5562 Compare May 6, 2022 09:30
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 4 times, most recently from 2924c3d to d66e93a Compare May 9, 2022 10:33
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 2 times, most recently from 85845e6 to 4a40138 Compare May 10, 2022 01:49
@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch 3 times, most recently from 5438d26 to 5840afd Compare May 10, 2022 07:57
@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented May 10, 2022

Glue connector test adds about 19 minutes in CI

  • Without Glue 6m 2022-05-09T13:22 ~ 2022-05-09T13:28
  • With Glue 25m 2022-05-10T03:55 ~ 2022-05-10T04:20

@ebyhr ebyhr force-pushed the ebi/iceberg-glue-bct branch from 5840afd to 2da7cfe Compare May 10, 2022 13:21
@findepi
Copy link
Copy Markdown
Member

findepi commented May 10, 2022

Glue connector test adds about 19 minutes in CI

That's quite something.
I think we discussed that and there was a conclusion to do BCST instead?

@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented May 10, 2022

I think we discussed that and there was a conclusion to do BCST instead?

@ebyhr ebyhr closed this May 10, 2022
@ebyhr ebyhr deleted the ebi/iceberg-glue-bct branch May 10, 2022 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry test

Development

Successfully merging this pull request may close these issues.

2 participants