Support Nessie Catalog in Iceberg connector #11701

nastra · 2022-03-29T08:48:55Z

This PR integrates the (Nessie catalog functionality)[https://github.com/apache/iceberg/tree/master/nessie/src/main/java/org/apache/iceberg/nessie] to the Iceberg connector. It adds the following new things:

a new CatalogType called NESSIE
an IcebergNessieCatalogModule that sets up all necessary dependencies (including a client to connect to the Nessie server)
a NessieConfig that includes configuration settings required for Nessie
TrinoNessieCatalog + NessieIcebergTableOperations that implement the main behavior of the catalog
some unit and integration tests that verify that the Iceberg connector works with Nessie (note that the integration test requires a Nessie server to be started, which is being done via the nessie-apprunner-maven-pluginprior to the integration-test phase)

nastra · 2022-03-29T14:04:26Z

@findepi looks like one of the CI jobs timed out. would it be possible to restart just this one job?

ebyhr · 2022-03-30T23:01:11Z

@nastra GitHub Actions recently introduced "Re-run failed jobs". The feature might be not yet stable though. By the way, we will request to push an empty commit for triggering CI if necessary.

nastra · 2022-03-31T05:51:31Z

@ebyhr I don't have permissions unfortunately to re-run the failed job myself, that's why I closeed & reopened the PR, which triggers a full CI run again (same as pushing)

findinpath · 2022-03-31T13:51:27Z

Warning:  Invalid bytecodeVersion for org.glassfish.jersey.core:jersey-common:jar:2.30:compile : META-INF/versions/11/org/glassfish/jersey/internal/jsr166/SubmissionPublisher$1.class: expected 55, but was 56

This library seems to be compiled with Java 12, but Trino works with Java 11

nastra · 2022-03-31T14:11:25Z

@findinpath I can see the same warning in other CI runs on other branches. Here's an example from master from https://github.com/trinodb/trino/runs/5763019856?check_suite_focus=true.

Nessie itself is also compiled with Java 11 btw

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java

electrum · 2022-03-31T15:46:27Z

The tests should be normal unit tests, not ITs. You can probably use Testcontainers for Nessie.

findinpath · 2022-03-31T20:29:43Z

It would be nice to see some tests which involve dealing with running SQL statements over the query runner to actually test how the connector works with the new catalog type.

Ideal would be (at a later point) to have additionally also a product tests environment (similar to singlenode-spark-iceberg
See for reference:

testing/bin/ptl env describe  --environment EnvSinglenodeSparkIceberg  --config config-default

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/NessieConfig.java

plugin/trino-iceberg/pom.xml

...in/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/NessieIcebergUtil.java

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java

findinpath · 2022-03-31T20:31:50Z

Thank you @nastra for your contribution. I am looking forward to help out in this PR to add support for Nessie in Trino Iceberg connector.

nastra · 2022-04-01T09:48:22Z

@findinpath thanks for the review. I updated the code based on your review and pushed a new commit. Will look into doing some additional testing via SQL statements now.

findinpath · 2022-04-01T10:54:54Z

@nastra you can use the Glue related tests from the connector as a rough template on what kinds of tests you need to add.

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java

findinpath · 2022-04-05T19:58:46Z

.../trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/NessieIcebergClient.java

+    {
+        this.nessieApi = requireNonNull(nessieApi, "nessieApi is null");
+        this.config = requireNonNull(config, "nessieConfig is null");
+        this.reference = () -> loadReference(config.getDefaultReferenceName(), null);


Does the client need to know the reference configured for the connector?

yes as it needs to operate on the correct branch/tag

I meant, does the client need to know this detail while it is being created?
Otherwise said, should the catalog deliver the branch/tag name on each call to the client?

eventually this is what we might end up having (the catalog providing the branch/tag to the client), but I didn't want to over-complicate the current implementation. That being said, I would prefer to adjust this part of the code once the catalog knows which branch/tag it is being used with then executing a particular SQL

testing/trino-testing-containers/src/main/java/io/trino/testing/containers/NessieContainer.java

findinpath · 2022-04-06T10:43:37Z

Could you please add documentation on how to setup Nessie catalog in the Iceberg connector

You can use https://github.com/trinodb/trino/pull/11772/files#diff-e1aabf1cfd8bd8aa7d1b75e70089b57413b2e620a5eebeb36cc76fd3f2ac60db as a template for getting started.

plugin/trino-iceberg/pom.xml

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/NessieConfig.java

findinpath · 2022-04-06T10:49:56Z

.../trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/NessieIcebergClient.java

+    {
+        this.nessieApi = requireNonNull(nessieApi, "nessieApi is null");
+        this.config = requireNonNull(config, "nessieConfig is null");
+        this.reference = () -> loadReference(config.getDefaultReferenceName(), null);


I meant, does the client need to know this detail while it is being created?
Otherwise said, should the catalog deliver the branch/tag name on each call to the client?

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java

...ceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNessieCatalogConnectorSmokeTest.java

ajantha-bhat · 2023-05-22T18:00:01Z

@ebyhr: Thanks for the review.

Fixed all the comments. PR is ready for review.

ebyhr

Please squash commits into one and remove commit body.

plugin/trino-iceberg/pom.xml

...-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalogFactory.java

.../java/io/trino/plugin/iceberg/catalog/nessie/TestIcebergNessieCatalogConnectorSmokeTest.java

ebyhr · 2023-05-24T01:51:24Z

...roduct-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java

@@ -124,7 +125,7 @@ public void setUp()
        onTrino().executeQuery(format("CREATE SCHEMA IF NOT EXISTS %s.%s", TRINO_CATALOG, TEST_SCHEMA_NAME));
    }

-    @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS, ICEBERG_REST, ICEBERG_JDBC}, dataProvider = "storageFormatsWithSpecVersion")
+    @Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS, ICEBERG_REST, ICEBERG_JDBC, ICEBERG_NESSIE}, dataProvider = "storageFormatsWithSpecVersion")


Some tests are missing ICEBERG_NESSIE group.

the idea was to just run a few tests with nessie to keep CI times low and to show that trino+spark+nessie work

testing/trino-testing-containers/src/main/java/io/trino/testing/containers/NessieContainer.java

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogConfig.java

testing/trino-faulttolerant-tests/pom.xml

Co-authored-by: Ajantha Bhat <ajanthabhat@gmail.com>

ajantha-bhat · 2023-05-26T05:25:40Z

The failed test TestHiveStorageFormats > testSelectWithNullFormat(0: StorageFormat{name=SEQUENCEFILE, properties={}, sessionProperties={}}) [groups: storage_formats_detailed] is independent of PR changes and could be flaky.

Re-triggering the build

ebyhr · 2023-05-26T05:30:23Z

@ajantha-bhat Please note that you don't need to retrigger CI when the failed test is unrelated to the change. Also, we recommend pushing an empty commit instead of reopening PR when retriggering CI.

ajantha-bhat · 2023-05-26T05:35:03Z

@ajantha-bhat Please note that you don't need to retrigger CI when the failed test is unrelated to the change. Also, we recommend pushing an empty commit instead of reopening PR when retriggering CI.

Thanks. I am new to Trino contributions. Good to know.

ajantha-bhat · 2023-05-26T09:05:52Z

PR is ready. Thanks.

…ctor To reduce the review effort, only the basic Nessie configurations were supported in trinodb#11701. Nessie server can be deployed with Auth mode like keycloak. So, need to expose the Nessie client configurations to handle the Auth. Along with that, some common Nessie server configurations like read-timeout-ms, connect-timeout-ms and compression-enabled properties are exposed to have finer control over the Nessie commits.

cla-bot bot added the cla-signed label Mar 29, 2022

nastra requested a review from findepi March 29, 2022 08:49

nastra force-pushed the add-nessie-support branch from 66fed2e to 191715e Compare March 29, 2022 11:39

nastra closed this Mar 30, 2022

nastra reopened this Mar 30, 2022

findinpath self-requested a review March 31, 2022 13:54

findinpath reviewed Mar 31, 2022

View reviewed changes

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java Outdated Show resolved Hide resolved

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java Outdated Show resolved Hide resolved

nastra force-pushed the add-nessie-support branch from 191715e to 6beef07 Compare March 31, 2022 14:43

findinpath reviewed Mar 31, 2022

View reviewed changes

nastra force-pushed the add-nessie-support branch from 585fa60 to be8ec7c Compare April 1, 2022 09:43

nastra force-pushed the add-nessie-support branch 2 times, most recently from 247f4ee to 9466225 Compare April 1, 2022 14:33

findinpath reviewed Apr 5, 2022

View reviewed changes

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java Outdated Show resolved Hide resolved

findinpath reviewed Apr 5, 2022

View reviewed changes

...n/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java Outdated Show resolved Hide resolved

findinpath reviewed Apr 5, 2022

View reviewed changes

testing/trino-testing-containers/src/main/java/io/trino/testing/containers/NessieContainer.java Outdated Show resolved Hide resolved

nastra force-pushed the add-nessie-support branch from 9466225 to 86ce1dc Compare April 6, 2022 10:31

findinpath reviewed Apr 6, 2022

View reviewed changes

...ceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergNessieCatalogConnectorSmokeTest.java Outdated Show resolved Hide resolved

ajantha-bhat closed this May 22, 2023

ajantha-bhat reopened this May 22, 2023

ajantha-bhat mentioned this pull request May 23, 2023

Handle dependency declarations nastra/trino#7

Merged

ebyhr approved these changes May 24, 2023

View reviewed changes

ebyhr reviewed May 24, 2023

View reviewed changes

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogConfig.java Outdated Show resolved Hide resolved

nastra force-pushed the add-nessie-support branch from 9e47932 to a3d8a20 Compare May 25, 2023 13:01

ebyhr reviewed May 25, 2023

View reviewed changes

testing/trino-faulttolerant-tests/pom.xml Outdated Show resolved Hide resolved

Support Nessie Catalog in Iceberg connector

aee6900

Co-authored-by: Ajantha Bhat <ajanthabhat@gmail.com>

nastra force-pushed the add-nessie-support branch from a3d8a20 to aee6900 Compare May 25, 2023 23:04

ajantha-bhat closed this May 26, 2023

ajantha-bhat reopened this May 26, 2023

ebyhr merged commit 9dc8375 into trinodb:master May 29, 2023

github-actions bot added this to the 419 milestone May 29, 2023

ebyhr mentioned this pull request May 29, 2023

Release notes for 419 #17546

Closed

ajantha-bhat mentioned this pull request May 29, 2023

Nessie Iceberg Table support for Presto/Trino projectnessie/nessie#778

Closed

nastra deleted the add-nessie-support branch May 30, 2023 05:30

ajantha-bhat mentioned this pull request Jun 1, 2023

Support additional configurations for Nessie catalog #17725

Merged

colebow mentioned this pull request Jun 2, 2023

Add Trino 419 release notes #17626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Nessie Catalog in Iceberg connector #11701

Support Nessie Catalog in Iceberg connector #11701

nastra commented Mar 29, 2022 •

edited

Loading

nastra commented Mar 29, 2022

ebyhr commented Mar 30, 2022 •

edited

Loading

nastra commented Mar 31, 2022

findinpath commented Mar 31, 2022

nastra commented Mar 31, 2022 •

edited

Loading

electrum commented Mar 31, 2022

findinpath commented Mar 31, 2022

findinpath commented Mar 31, 2022

nastra commented Apr 1, 2022

findinpath commented Apr 1, 2022

findinpath Apr 5, 2022

nastra Apr 6, 2022

findinpath Apr 6, 2022

nastra Apr 6, 2022

findinpath commented Apr 6, 2022

findinpath Apr 6, 2022

ajantha-bhat commented May 22, 2023 •

edited

Loading

ebyhr left a comment

ebyhr May 24, 2023

nastra May 24, 2023 •

edited

Loading

ajantha-bhat commented May 26, 2023

ebyhr commented May 26, 2023

ajantha-bhat commented May 26, 2023

ajantha-bhat commented May 26, 2023

Support Nessie Catalog in Iceberg connector #11701

Support Nessie Catalog in Iceberg connector #11701

Conversation

nastra commented Mar 29, 2022 • edited Loading

nastra commented Mar 29, 2022

ebyhr commented Mar 30, 2022 • edited Loading

nastra commented Mar 31, 2022

findinpath commented Mar 31, 2022

nastra commented Mar 31, 2022 • edited Loading

electrum commented Mar 31, 2022

findinpath commented Mar 31, 2022

findinpath commented Mar 31, 2022

nastra commented Apr 1, 2022

findinpath commented Apr 1, 2022

findinpath Apr 5, 2022

Choose a reason for hiding this comment

nastra Apr 6, 2022

Choose a reason for hiding this comment

findinpath Apr 6, 2022

Choose a reason for hiding this comment

nastra Apr 6, 2022

Choose a reason for hiding this comment

findinpath commented Apr 6, 2022

findinpath Apr 6, 2022

Choose a reason for hiding this comment

ajantha-bhat commented May 22, 2023 • edited Loading

ebyhr left a comment

Choose a reason for hiding this comment

ebyhr May 24, 2023

Choose a reason for hiding this comment

nastra May 24, 2023 • edited Loading

Choose a reason for hiding this comment

ajantha-bhat commented May 26, 2023

ebyhr commented May 26, 2023

ajantha-bhat commented May 26, 2023

ajantha-bhat commented May 26, 2023

nastra commented Mar 29, 2022 •

edited

Loading

ebyhr commented Mar 30, 2022 •

edited

Loading

nastra commented Mar 31, 2022 •

edited

Loading

ajantha-bhat commented May 22, 2023 •

edited

Loading

nastra May 24, 2023 •

edited

Loading