Add Delta synchronizer for GCS #12523

findinpath · 2022-05-24T06:21:05Z

Description

The implementation instantiates explicitly Storage
Cloud Storage API client in a similar fashion as it is
done withing hadoop-connectors library.

The client is used for fine-tuning an API Insert call
in order to be able to create the blob corresponding to
the transaction log file.
the creation of the blob will succeed only
The creation of the blob will succeed if and only if
there are no live versions of the object.

This approach is a workaround for the limitations of the
hadoop-connectors GCS library which is exposing only
output streams when in need to create a blob.
In case of dealing mid-write of the blob with I/O exceptions,
the output stream can't be closed because this action would
expose the blob in a corrupted state on GCS. Effectively in
such a situation, the output stream would need to be intentionally
leaked - not closed, which can lead to multiple unforeseen
consequences.

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

Improvement

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?

Add the ability to perform DML statements of tables backed by Google Cloud Storage

Related issues, pull requests, and links

PR where the Storage instance is retrieved via reflection from hadoop-connectors library: #12309

Fixes: #12264

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

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

# Delta Lake
* Add support for DML statements on tables backed by GCS

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

findinpath · 2022-05-24T06:31:59Z

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

+    }
+
+    private static final class PassthroughGoogleCredential
+            extends GoogleCredential


GoogleCredential is obsolete (see https://cloud.google.com/java/docs/reference/google-api-client/latest/com.google.api.client.googleapis.auth.oauth2.GoogleCredential).

The current version of hadoop-connectors library depends however on Credential for instantiating the RetryHttpInitializer.
With an upgrade of hadoop-connectors to a more recent version containing GoogleCloudDataproc/hadoop-connectors#704 the implementation wouldn't need to extend from an obsolete class.

findinpath · 2022-08-29T10:06:43Z

@findepi / @ebyhr could you please run the PR with secrets?

ebyhr · 2022-08-29T10:13:35Z

Sent #13895 within the repository.

findinpath · 2022-08-29T10:30:44Z

I've discovered locally a few overridden tests failing because DML was unsupported on GS. I've removed them so that TestDeltaLakeGcsConnectorSmokeTest is hopefully green.

cc: @ebyhr

ebyhr

Just skimmed.

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

...main/java/io/trino/plugin/deltalake/transactionlog/writer/GcsTransactionLogSynchronizer.java

plugin/trino-delta-lake/pom.xml

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

findepi · 2022-09-05T13:46:16Z

...main/java/io/trino/plugin/deltalake/transactionlog/writer/GcsTransactionLogSynchronizer.java

+        this.gcsStorageFactory = requireNonNull(gcsStorageFactory, "gcsStorageFactory is null");
+    }
+
+    // This approach should be compatible with OSS Delta Lake.


Yes, it "should be". Is it?
Either remove the comment, or replace with one stating the fact.
You can also link to a specific file in Delta project that implements same logic there.

Removing the comment
This is a left-over from the initial draft which copied partly code from AzureTransactionLogSynchronizer

...main/java/io/trino/plugin/deltalake/transactionlog/writer/GcsTransactionLogSynchronizer.java

testing/trino-faulttolerant-tests/pom.xml

findinpath · 2022-09-27T09:50:06Z

I changed slightly the way the httpTransport is being created in io.trino.plugin.deltalake.GcsStorageFactory#create in order to depend (similarly to the hadoop-connectors implementation) on the Configuration content in order to allow eventually its customization.

findinpath · 2022-09-28T19:38:47Z

@ebyhr / @findepi I made the necessary modifications to run the Delta GCS tests in a separate job in order to reduce the risk of running into an eventual timeout caused by, assumingly, cross cloud Azure -> GCS operations. Could you please run update the corresponding trino PR #13895 ? Thank you in advance.

ebyhr · 2022-09-28T22:40:28Z

Could you fix below failure?

Error:  Parameter 'testing.gcp-storage-bucket' is required by @Parameters on method public io.trino.plugin.deltalake.TestDeltaLakeGcsConnectorSmokeTest(java.lang.String,java.lang.String) but has not been marked @Optional or defined

findinpath · 2022-10-05T15:06:35Z

Given that the Delta Lake tests on GCS failed with OOM

Error:  java.lang.OutOfMemoryError: Java heap space
Error:  Terminating due to java.lang.OutOfMemoryError: Java heap space
...
Error:  Command was /bin/sh -c cd /home/runner/work/trino/trino/plugin/trino-delta-lake && /opt/hostedtoolcache/Java_Zulu_jdk/17.0.4-8/x64/bin/java -Duser.timezone=America/Bahia_Banderas -Duser.language=en -Duser.region=US -Dfile.encoding=UTF-8 -Xmx3g -Xms3g -XX:+ExitOnOutOfMemoryError -XX:+HeapDumpOnOutOfMemoryError -XX:-OmitStackTraceInFastThrow -jar /home/runner/work/trino/trino/plugin/trino-delta-lake/target/surefire/surefirebooter18199502927712579772.jar /home/runner/work/trino/trino/plugin/trino-delta-lake/target/surefire 2022-10-04T13-11-47_658-jvmRun1 surefire6324101022156948381tmp surefire_08238049524145815535tmp
Error:  Error occurred in starting fork, check output in log
Error:  Process Exit Code: 3

https://github.com/trinodb/trino/actions/runs/3180848245/jobs/5187102248

let's try running for a few days stress tests (running delta-lake tests with the profile gcs-tests) on GCS infrastructure and gather the results.

The implementation instantiates explicitly `Storage` Cloud Storage API client in a similar fashion as it is done withing `hadoop-connectors` library. The client is used for fine-tuning an API `Insert` call in order to be able to create the blob corresponding to the transaction log file. The creation of the blob will succeed if and only if there are no live versions of the object. This approach is a workaround for the limitations of the `hadoop-connectors` GCS library which is exposing only output streams when in need to create a blob. In case of dealing mid-write of the blob with I/O exceptions, the output stream can't be closed because this action would expose the blob in a corrupted state on GCS. Effectively in such a situation, the output stream would need to be intentionally leaked - not closed, which can lead to multiple unforeseen consequences.

The GitHub runner is running on the Azure cloud and doing cross cloud operations against a Google Cloud Storage bucket may incur from time to time increased latency. Run the GCS Delta Lake tests in a dedicated job in order to have enough buffer to run the GCS integration tests and avoid an eventual job timeout.

Use `trino-ci-test` multi region GCS bucket in testing GCS functionality of the `trino-delta-lake` connector.

findepi · 2022-10-17T10:58:21Z

Green (https://github.com/trinodb/trino/actions/runs/3250216748/jobs/5333596290)

findepi · 2022-10-17T10:59:18Z

Removed the temporary change !temporary Run only delta lake related tests

findinpath · 2022-10-25T05:56:07Z

Unrelated CI hit for trino-kudu

TestKuduConnectorTest.testAggregation may relate to #14706

cla-bot bot added the cla-signed label May 24, 2022

findinpath requested a review from findepi May 24, 2022 06:21

github-actions bot added the tests:hive label May 24, 2022

findinpath requested a review from alexjo2144 May 24, 2022 06:21

findinpath commented May 24, 2022

View reviewed changes

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/GcsStorageFactory.java Outdated Show resolved Hide resolved

findinpath commented May 24, 2022

View reviewed changes

findinpath requested a review from ebyhr May 24, 2022 06:33

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 7d1c2ba to c56056e Compare May 25, 2022 10:06

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from c56056e to d4423ba Compare June 13, 2022 14:27

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch 2 times, most recently from 4c8f31a to f53717b Compare June 24, 2022 14:05

findinpath mentioned this pull request Aug 16, 2022

Add Delta synchronizer for GCS #12309

Closed

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from f53717b to 0dbd94f Compare August 16, 2022 13:35

findinpath changed the title ~~Add Delta synchronizer for GCS - Storage is initiated explicitly~~ Add Delta synchronizer for GCS Aug 16, 2022

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 0dbd94f to 0d29de7 Compare August 16, 2022 13:57

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 0d29de7 to b9eb669 Compare August 29, 2022 09:44

ebyhr mentioned this pull request Aug 29, 2022

Test: Add Delta synchronizer for GCS #13895

Closed

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from b9eb669 to 934e3a4 Compare August 29, 2022 10:28

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 934e3a4 to e09d8dd Compare August 30, 2022 14:46

ebyhr reviewed Aug 31, 2022

View reviewed changes

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch 3 times, most recently from f7f7f78 to 1ae12f6 Compare August 31, 2022 10:20

findepi reviewed Sep 5, 2022

View reviewed changes

plugin/trino-delta-lake/pom.xml Outdated Show resolved Hide resolved

findepi reviewed Sep 5, 2022

View reviewed changes

plugin/trino-delta-lake/pom.xml Outdated Show resolved Hide resolved

findepi approved these changes Sep 5, 2022

View reviewed changes

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 1ae12f6 to 86968a7 Compare September 5, 2022 15:20

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from 9d60ca1 to beeb9b4 Compare September 27, 2022 09:50

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch 8 times, most recently from 44f6b3c to 5e7eb20 Compare October 3, 2022 15:27

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch 2 times, most recently from 5eed40c to dfd6c1b Compare October 13, 2022 19:09

findinpath added 4 commits October 14, 2022 14:32

Extract guice bindings for cloud transaction log synchronizers

13bb222

Use trino-ci-test multi-region GCS bucket

dbeed72

Use `trino-ci-test` multi region GCS bucket in testing GCS functionality of the `trino-delta-lake` connector.

findinpath force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from dfd6c1b to cf3fc1c Compare October 14, 2022 12:32

findepi force-pushed the delta-gcs-log-synchronizer-explicit-storage-instantiation branch from cf3fc1c to dbeed72 Compare October 17, 2022 10:58

empty

50cb6d0

findepi merged commit 0ccc025 into trinodb:master Oct 25, 2022

findepi mentioned this pull request Oct 25, 2022

Release notes for 401 #14628

Closed

github-actions bot added this to the 401 milestone Oct 25, 2022

colebow mentioned this pull request Oct 25, 2022

Add Trino 401 release notes #14675

Merged

findinpath mentioned this pull request Oct 26, 2022

Document the DML operations support on GCS #14766

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Delta synchronizer for GCS #12523

Add Delta synchronizer for GCS #12523

findinpath commented May 24, 2022 •

edited

Loading

findinpath May 24, 2022

findinpath commented Aug 29, 2022

ebyhr commented Aug 29, 2022

findinpath commented Aug 29, 2022

ebyhr left a comment

findepi Sep 5, 2022

findinpath Sep 5, 2022

findinpath commented Sep 27, 2022

findinpath commented Sep 28, 2022

ebyhr commented Sep 28, 2022

findinpath commented Oct 5, 2022

findepi commented Oct 17, 2022

findepi commented Oct 17, 2022

findinpath commented Oct 25, 2022

Add Delta synchronizer for GCS #12523

Add Delta synchronizer for GCS #12523

Conversation

findinpath commented May 24, 2022 • edited Loading

Description

Related issues, pull requests, and links

Documentation

Release notes

findinpath May 24, 2022

Choose a reason for hiding this comment

findinpath commented Aug 29, 2022

ebyhr commented Aug 29, 2022

findinpath commented Aug 29, 2022

ebyhr left a comment

Choose a reason for hiding this comment

findepi Sep 5, 2022

Choose a reason for hiding this comment

findinpath Sep 5, 2022

Choose a reason for hiding this comment

findinpath commented Sep 27, 2022

findinpath commented Sep 28, 2022

ebyhr commented Sep 28, 2022

findinpath commented Oct 5, 2022

findepi commented Oct 17, 2022

findepi commented Oct 17, 2022

findinpath commented Oct 25, 2022

findinpath commented May 24, 2022 •

edited

Loading