Test Delta Lake on GCS by alexjo2144 · Pull Request #12800 · trinodb/trino

alexjo2144 · 2022-06-10T17:30:12Z

Description

Integration and Product tests for running Delta Lake on Google Cloud Storage.

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

Test only

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

Delta Lake connector

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

Not important

Related issues, pull requests, and links

GCS was enabled in #12144

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:

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

...o-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeGcsConnectorSmokeTest.java

findinpath · 2022-06-13T07:38:27Z

...o-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeGcsConnectorSmokeTest.java

+            }
+        }
+        catch (FileNotFoundException e) {
+            return ImmutableList.of();


why not fail in case that the directory is not present?

There are tests that rely on this behavior. Directory xyz doesn't exist, and the test checks how many files are in xyz. Seems like a reasonable contract to me

alexjo2144 · 2022-06-14T17:21:12Z

@findepi @findepi pushed some fixups when you get a chance

hashhar

Did not look at SmokeTests yet and the changes in DeltaLake plugin.

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeConnectorSmokeTest.java

...rino-product-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeGcsReads.java

testing/trino-product-tests/src/main/java/io/trino/tests/product/TestGroups.java

hashhar · 2022-06-15T12:09:13Z

...sources/docker/presto-product-tests/conf/environment/multinode-gcs/tempto-configuration.yaml

+        prepare_statement:
+            - USE ${databases.hive.schema}


Question unrelated to this PR - can we use this to for example run CREATE SCHEMA against trino runner?

e.g. File HMS doesn't allow creating views in non-existent schemas so I have some PTs where the test itself runs CREATE SCHEMA IF NOT EXISTS and I'd maybe want the schema creation to happen here.

...main/resources/docker/presto-product-tests/conf/environment/multinode-gcs/iceberg.properties

.../docker/presto-product-tests/conf/environment/multinode-gcs/hive-site-overrides-template.xml

.../docker/presto-product-tests/conf/environment/multinode-gcs/core-site-overrides-template.xml

ebyhr · 2022-06-16T02:01:02Z

...o-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeGcsConnectorSmokeTest.java

+    private final Path gcpCredentialsFile;
+    private final FileSystem fileSystem;
+
+    @Parameters({"gcp-storage-bucket", "gcp-credentials-key"})


How about adding delta. prefix so that we can avoid conflicts in IntelliJ configuration template in other connectors?

What about testing.? I was hoping to use the same parameters for Hive and Iceberg as well

...o-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeGcsConnectorSmokeTest.java

...duct-tests-launcher/src/main/java/io/trino/tests/product/launcher/suite/suites/SuiteGcs.java

...rino-product-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeGcsReads.java

ebyhr · 2022-06-16T03:57:13Z

...-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeGcs.java

+
+        builder.addConnector("hive", forHostPath(dockerFiles.getDockerFilesHostPath("conf/environment/multinode-gcs/hive.properties")), CONTAINER_PRESTO_HIVE_PROPERTIES);
+        builder.addConnector("delta", forHostPath(dockerFiles.getDockerFilesHostPath("conf/environment/multinode-gcs/delta.properties")), CONTAINER_PRESTO_ETC + "/catalog/delta.properties");
+        builder.addConnector("iceberg", forHostPath(dockerFiles.getDockerFilesHostPath("conf/environment/multinode-gcs/iceberg.properties")), CONTAINER_PRESTO_ETC + "/catalog/iceberg.properties");


hive & iceberg connectors look unused. Is it for future work?

Yep, I was going to put tests for Hive and Iceberg up next. I also just thought it'd be handy to have for development on GCS as well, but I can remove them until the next PR if you'd like.

Got it. There is no need to remove them.

alexjo2144 · 2022-06-16T17:38:43Z

Applied comments above, and included a commit to reduce the test parallelism. I'm hoping that will help the OOM issues on the test branch #12663 (comment)

If anyone has better ideas though, let me know

alexjo2144 · 2022-06-16T17:41:49Z

I ran the suite with JMX logging, and looking at the heap usage it increases gradually up to about 3GB and then I guess GC runs and the heap goes back down to about 0.75GB.

findinpath · 2022-06-17T08:42:54Z

If anyone has better ideas though, let me know

@alexjo2144 please see #12663 (comment)

Increasing air.test.jvmsize could be also an option.

findepi · 2022-06-22T08:33:57Z

@alexjo2144 please see #12663 (comment)

Increasing air.test.jvmsize could be also an option.

That commit suggests 4g, which sounds like a lot for non-stress tests.
I am concerned by the fact that tests seem to consume lot more memory when run on CI vs locally.
Are you sure you're running with same configs?

ebyhr · 2022-07-04T06:01:09Z

GCS test succeeded in #12663, but it took 59 min (test timeout is 60min). We need to increase the timeout or find a way to shorten the time.

findinpath · 2022-07-04T09:04:10Z

+1 for increasing the the timeout-minutes for cloud tests

alexjo2144 · 2022-08-04T15:38:47Z

@ebyhr @findepi I rebased and added a tmp commit that runs the Delta suite a bunch of times. Can we test with secrets when you get a chance?

ebyhr · 2022-08-04T22:29:04Z

Updated #12663

alexjo2144 · 2022-08-11T13:30:59Z

@findepi @ebyhr pushed one more commit to put the GCS tests in a separate step. Can we try that on CI when you get a chance?

alexjo2144 · 2022-08-11T16:22:27Z

Sorry, need one more mirror. Didn't notice the new merge tests during the rebase

alexjo2144 · 2022-08-12T17:48:26Z

One more thing to test. I switched the bucket to a regioned one in us-east so it should be close to the AWS region we run CI in

alexjo2144 · 2022-08-18T17:27:57Z

@ebyhr can we do another test with secrets when you get a chance?

alexjo2144 · 2022-08-19T13:39:59Z

@ebyhr the cloud tests passed all five times. The main Delta integration tests failed twice because of #13348 and the Databricks failure was just a cleanup failure, all the actual tests passed.

.github/workflows/ci.yml

...o-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeGcsConnectorSmokeTest.java

...ing/trino-product-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeGcs.java

.github/workflows/ci.yml

alexjo2144 · 2022-08-22T20:19:18Z

All set, thanks @ebyhr

ebyhr · 2022-08-23T02:37:12Z

.github/workflows/ci.yml

GCS suite looks not running. Probably, this should be steps.check-gcp-secrets.outputs.have_gcp_secrets.

It's fixed now.

Includes catalogs for the hive, delta lake, and iceberg connectors.

ebyhr · 2022-08-23T04:54:58Z

Merged, thanks!

findepi · 2022-08-23T08:00:51Z

🎉

cla-bot bot added the cla-signed label Jun 10, 2022

alexjo2144 requested review from jirassimok and nineinchnick June 10, 2022 17:30

github-actions bot added the tests:hive label Jun 10, 2022

alexjo2144 requested review from findepi and findinpath June 10, 2022 17:30

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from 212fd35 to ca0ec94 Compare June 10, 2022 17:32

nineinchnick approved these changes Jun 13, 2022

View reviewed changes

findinpath reviewed Jun 13, 2022

View reviewed changes

findepi mentioned this pull request Jun 13, 2022

Test with GCS #12663

Closed

alexjo2144 self-assigned this Jun 14, 2022

alexjo2144 requested a review from findinpath June 14, 2022 17:20

findepi requested review from ebyhr, hashhar and nineinchnick June 15, 2022 10:01

nineinchnick approved these changes Jun 15, 2022

View reviewed changes

hashhar reviewed Jun 15, 2022

View reviewed changes

ebyhr reviewed Jun 16, 2022

View reviewed changes

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from f6ca780 to 58ea1ed Compare June 16, 2022 17:01

ebyhr mentioned this pull request Jul 6, 2022

Increase CI timeout to 90 minutes #13104

Closed

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch 3 times, most recently from fe408fb to a2e60bf Compare August 3, 2022 21:02

findepi mentioned this pull request Aug 11, 2022

Test with GCS v2 #13592

Closed

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from e280bc2 to 5102b80 Compare August 11, 2022 16:21

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from 11001bb to 9c37bce Compare August 18, 2022 15:39

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from 509a9a1 to 9c37bce Compare August 19, 2022 13:40

ebyhr approved these changes Aug 22, 2022

View reviewed changes

Improve error message when writing to unsupported Delta Lake filesystems

bb38a0b

alexjo2144 force-pushed the alexjo/test-delta-on-gcs branch from 9c37bce to 85de93c Compare August 22, 2022 20:18

ebyhr reviewed Aug 23, 2022

View reviewed changes

alexjo2144 added 2 commits August 23, 2022 11:40

Add GCS product test environment

335b35b

Includes catalogs for the hive, delta lake, and iceberg connectors.

Run Delta Lake smoke tests against the GCS filesystem

ef59122

ebyhr force-pushed the alexjo/test-delta-on-gcs branch from 85de93c to ef59122 Compare August 23, 2022 02:41

ebyhr merged commit 52bf668 into trinodb:master Aug 23, 2022

github-actions bot added this to the 394 milestone Aug 23, 2022

colebow mentioned this pull request Aug 24, 2022

Add Trino 394 release notes #13813

Merged

alexjo2144 mentioned this pull request Aug 24, 2022

Add Iceberg connector smoke tests on Google Cloud #13835

Merged

Conversation

alexjo2144 commented Jun 10, 2022

Description

Related issues, pull requests, and links

Documentation

Release notes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexjo2144 commented Jun 14, 2022

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexjo2144 commented Jun 16, 2022

Uh oh!

alexjo2144 commented Jun 16, 2022

Uh oh!

findinpath commented Jun 17, 2022

Uh oh!

findepi commented Jun 22, 2022

Uh oh!

ebyhr commented Jul 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

findinpath commented Jul 4, 2022

Uh oh!

alexjo2144 commented Aug 4, 2022

Uh oh!

ebyhr commented Aug 4, 2022

Uh oh!

alexjo2144 commented Aug 11, 2022

Uh oh!

alexjo2144 commented Aug 11, 2022

Uh oh!

alexjo2144 commented Aug 12, 2022

Uh oh!

alexjo2144 commented Aug 18, 2022

Uh oh!

alexjo2144 commented Aug 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexjo2144 commented Aug 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebyhr commented Aug 23, 2022

Uh oh!

findepi commented Aug 23, 2022

ebyhr commented Jul 4, 2022 •

edited

Loading