Skip to content

Add Iceberg connector smoke tests on Google Cloud#13835

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
alexjo2144:iceberg/gcs-tests
Sep 2, 2022
Merged

Add Iceberg connector smoke tests on Google Cloud#13835
ebyhr merged 1 commit intotrinodb:masterfrom
alexjo2144:iceberg/gcs-tests

Conversation

@alexjo2144
Copy link
Copy Markdown
Member

Description

Run BaseConnectorSmokeTests on the Iceberg connector with Google Cloud Storage.

Is this change a fix, improvement, new feature, refactoring, or other?

Testing

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Iceberg connector

How would you describe this change to a non-technical end user or system administrator?

Improve testing against GCS.

Related issues, pull requests, and links

Relates to: #12800

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Aug 24, 2022
@alexjo2144 alexjo2144 added the no-release-notes This pull request does not require release notes entry label Aug 24, 2022
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 24, 2022

Sent #13836 within the repository.

@alexjo2144
Copy link
Copy Markdown
Member Author

Thanks for the review @ebyhr , update applied

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 25, 2022

Could you please take a look at CI failures?

Error:  Failures: 
Error:  io.trino.plugin.iceberg.TestIcebergGcsConnectorSmokeTest.testDeleteRowsConcurrently
[INFO]   Run 1: PASS
[INFO]   Run 2: PASS
Error:    Run 3: TestIcebergGcsConnectorSmokeTest>BaseIcebergConnectorSmokeTest.testDeleteRowsConcurrently:137 » ThreadTimeout
[INFO]   Run 4: PASS
[INFO] 
Error:    TestIcebergGcsConnectorSmokeTest>BaseConnectorSmokeTest.testRenameSchema:318->AbstractTestQueryFramework.assertQueryFails:348 
Expecting message:
  <"Hive metastore does not support renaming schemas">
to match regex:
  <"This connector does not support renaming schemas">
but did not.

@alexjo2144
Copy link
Copy Markdown
Member Author

Sorry, forgot to run it again after the changes. Fixed the test and squashed

@alexjo2144
Copy link
Copy Markdown
Member Author

@ebyhr I think we're good to go? Thanks for the help

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Aug 31, 2022

@alexjo2144 I think TestIcebergGcsConnectorSmokeTest.testDeleteRowsConcurrently needs fix. You can see the failure in #13836's empty commit.

Error:  Failures: 
Error:  io.trino.plugin.iceberg.TestIcebergGcsConnectorSmokeTest.testDeleteRowsConcurrently
Error:    Run 1: TestIcebergGcsConnectorSmokeTest>BaseIcebergConnectorSmokeTest.testDeleteRowsConcurrently:136->AbstractTestQueryFramework.query:232->AbstractTestQueryFramework.query:237 » ThreadTimeout
Error:    Run 2: TestIcebergGcsConnectorSmokeTest>BaseIcebergConnectorSmokeTest.testDeleteRowsConcurrently:135->BaseIcebergConnectorSmokeTest.lambda$testDeleteRowsConcurrently$2:135 » ThreadTimeout
Error:    Run 3: TestIcebergGcsConnectorSmokeTest>BaseIcebergConnectorSmokeTest.testDeleteRowsConcurrently:135->BaseIcebergConnectorSmokeTest.lambda$testDeleteRowsConcurrently$2:135 » ThreadTimeout
Error:    Run 4: TestIcebergGcsConnectorSmokeTest>BaseIcebergConnectorSmokeTest.testDeleteRowsConcurrently:137 » ThreadTimeout

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

@alexjo2144
Copy link
Copy Markdown
Member Author

I think TestIcebergGcsConnectorSmokeTest.testDeleteRowsConcurrently needs fix

Right, forgot about that. What do you think about just increasing the test timeout? This is the first version of that test running against cloud storage so it makes sense to me it might take a little longer

@alexjo2144
Copy link
Copy Markdown
Member Author

On my computer the test takes about 50 seconds, so I bumped the timeout to 120

Want to kick off another test build? I added a tmp commit to stress that that one

@alexjo2144
Copy link
Copy Markdown
Member Author

alexjo2144 commented Sep 1, 2022

@ebyhr the test job timed out because I upped the run count for all of the test classes, not just GCP. But it did run successfully against GCP 20 times, with no failures, if we're happy with that

@ebyhr ebyhr merged commit 30ee010 into trinodb:master Sep 2, 2022
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Sep 2, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 395 milestone Sep 2, 2022
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

Development

Successfully merging this pull request may close these issues.

2 participants