[#10717] fix(trino-connector): Enable multi-metalake support for Trino 469+#10748
[#10717] fix(trino-connector): Enable multi-metalake support for Trino 469+#10748sachinnn99 wants to merge 1 commit intoapache:mainfrom
Conversation
sachinnn99
force-pushed
the
fix/10717-enable-multi-metalake-469
branch
from
2d1f11a to
905f660
Compare
|
@diqiu50 Opened this as a clean PR based on your feedback. Added documentation for |
Code Coverage Report
Files
|
| | gravitino.trino.skip-version-validation | boolean | false | The `gravitino.trino.skip-version-validation` defines whether to skip Trino version validation. Gravitino supports Trino versions between 435 and 478. If this option is `true`, unsupported Trino versions can still be used, but compatibility is not guaranteed. | No | 1.0.0 | | ||
| | gravitino.client. | string | (none) | The configuration key prefix for the Gravitino client config. | No | 1.0.0 | | ||
| | gravitino.trino.skip-catalog-patterns | string | (none) | The `gravitino.trino.skip-catalog-patterns` defines a comma-separated list of catalog name regex patterns that should be excluded from loading. For example, `test_.*, .*_tmp` excludes all catalogs starting with `test_` or ending with `_tmp`. | No | 1.2.0 | | ||
| | gravitino.use-single-metalake | boolean | true | If `true`, only one metalake is used and catalogs are identified by `<catalog_name>`. If `false`, multi-metalake mode is enabled and catalogs are identified by `<metalake_name>.<catalog_name>`. Multi-metalake mode is supported on Trino connector versions 469-472 and 473-478. It is not supported on connector versions 446-451 or 452-468. | No | 1.2.0 | |
There was a problem hiding this comment.
Please add all supported versions in the Note section.
There was a problem hiding this comment.
On the other hand, the configuration you added earlier is also useful. The unsupported part is the drop catalog operation, and users can choose to ignore it.
There was a problem hiding this comment.
Done — moved the version support details to the Note section and simplified the table cell description. Also added a note that DROP CATALOG is not supported in multi-metalake mode.
| **Note:** Invalid configuration properties will result in exceptions. Please see [Gravitino Java client configurations](../how-to-use-gravitino-client.md#gravitino-java-client-configuration) for more support client configuration. | ||
|
|
||
| Multi-metalake mode (`gravitino.use-single-metalake=false`) is supported on Trino connector versions 469-478. It is not supported on versions 446-468. The `DROP CATALOG` operation is not supported in multi-metalake mode. | ||
|
|
There was a problem hiding this comment.
Gravitino Trino connector 435-465 is support too
There was a problem hiding this comment.
@diqiu50 Thanks for the feedback. I checked the code on main — supportCatalogNameWithMetalake() returns true for 435-451 and false for 452-468. The connector version boundaries are 435-439, 440-445, 446-451, 452-468, so "435-465" doesn't align with a boundary.
Did you mean 435-451 should be documented as supported? Or should 452-468 also be enabled?
There was a problem hiding this comment.
Yes, we should document it. Trino 452–468 does not support it, and UT fails when it is enabled.
sachinnn99
force-pushed
the
fix/10717-enable-multi-metalake-469
branch
from
94b257d to
76ca0a8
Compare
|
You need to solve two issues in this PR. There is a test named TestGravitinoConnectorWithMetalakeCatalogName in the tester. If supportCatalogNameWithMet is set to true, you need to make sure this test is enabled and verify it. |
…r Trino 469+ Remove the supportCatalogNameWithMetalake and getTrinoCatalogName overrides in the Trino 469-472 and 473-478 factories so they inherit the default true from the base factory, fixing the startup exception when gravitino.use-single-metalake=false on Trino 469+. For connector versions where multi-metalake mode is not supported (currently 452-468), log a warning instead of throwing; DROP CATALOG and some other operations may fail on those versions. Enable and adapt the MultiMetalake integration test for 469-472 and 473-478, and document which versions support multi-metalake mode.
sachinnn99
force-pushed
the
fix/10717-enable-multi-metalake-469
branch
from
76ca0a8 to
ffeaa5c
Compare
|
@diqiu50 Pushed an update addressing your feedback: 1. Version support matrix (
2. Unsupported version behavior
3. Tests
Question: For 446-451, should we keep the test |
2 participants
What changes were proposed in this pull request?
Enable multi-metalake mode (
gravitino.use-single-metalake=false) for Trino connector versions 469-472 and 473-478 by marking them as supporting catalog names with metalake prefix. Disable the flag for 446-451 which does not support this mode.Why are the changes needed?
Fix: #10717
Connector versions 469+ and 473+ reported
supportCatalogNameWithMetalake() == false, causing a hard startup error whengravitino.use-single-metalake=false. These versions do support the feature, so the flag is corrected and previously disabled multi-metalake tests are re-enabled.Does this PR introduce any user-facing change?
Yes: users on Trino 469+ with
gravitino.use-single-metalake=falsecan now start the connector successfully. Documentation for thegravitino.use-single-metalakeproperty has been added to the configuration reference.How was this patch tested?
MultiMetalaketests inTestGravitinoConnector469andTestGravitinoConnector478../gradlew :trino-connector:trino-connector:test :trino-connector:trino-connector-469-472:test :trino-connector:trino-connector-473-478:test -PskipITs— all pass.