This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new f76b7a703c [#12985] improvement(client): make GravitinoClient
construction free of remote calls (#13104)
f76b7a703c is described below
commit f76b7a703c724978b9485d5d848d515a2f65506c
Author: li jie <[email protected]>
AuthorDate: Wed Sep 16 19:03:40 2026 +0800
[#12985] improvement(client): make GravitinoClient construction free of
remote calls (#13104)
### What changes were proposed in this pull request?
- Move `loadMetalake` from the `GravitinoClient` constructor to lazy
initialization in `getMetalake()` using double-checked locking (volatile
+ synchronized).
- Store only the metalake name in the constructor; the metalake handle
is loaded on first use.
- Remove `@throws NoSuchMetalakeException` from the constructor and
`build()` Javadoc; update `Command.buildClient()` Javadoc to reflect the
deferred exception timing.
- Add `testNoSuchMetalakeOnFirstOperation` to verify that
`NoSuchMetalakeException` surfaces on the first operation, not at
`build()`.
### Why are the changes needed?
`GravitinoClient`'s constructor performs a remote `loadMetalake` call
that runs with whatever credentials the `AuthDataProvider` resolves at
construction time. For a shared client whose provider resolves the
caller per request (the standalone Lance REST service after #12984),
this causes the permission requirement to land non-deterministically on
the first caller, failed construction to retry on every request, and
metalake-load errors to surface instead of the operation the caller
requested.
Fix: #12985
### Does this PR introduce _any_ user-facing change?
Yes. `NoSuchMetalakeException` is now thrown on the first operation
(e.g. `listCatalogs()`) instead of at `build()` time. No API or
configuration changes.
### How was this patch tested?
- `./gradlew :clients:client-java:spotlessCheck
:clients:cli:spotlessCheck :clients:client-java:compileTestJava
:clients:cli:compileJava`
- Ran `TestGravitinoMetalake` (21 tests) and
`TestGravitinoClientBuilder` (3 tests).
---------
Co-authored-by: lijie <[email protected]>
---
.../apache/gravitino/client/GravitinoClient.java | 29 ++++++++++++++++------
.../gravitino/client/TestGravitinoMetalake.java | 21 ++++++++++++++++
.../plugin/TestGravitinoDriverPlugin.java | 29 +++++++++++-----------
3 files changed, 57 insertions(+), 22 deletions(-)
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
index 9a7fb8e749..3366e63a1d 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
@@ -85,11 +85,15 @@ import org.apache.gravitino.tag.TagValueConstraint;
public class GravitinoClient extends GravitinoClientBase
implements SupportsCatalogs, TagOperations, SupportsJobs, PolicyOperations
{
- private final GravitinoMetalake metalake;
+ private final String metalakeName;
+
+ private volatile GravitinoMetalake metalake;
/**
* Constructs a new GravitinoClient with the given URI, authenticator and
AuthDataProvider.
*
+ * <p>The metalake is loaded lazily on first use, not in the constructor.
+ *
* @param uri The base URI for the Gravitino API.
* @param metalakeName The specified metalake name.
* @param authDataProvider The provider of the data which is used for
authentication.
@@ -97,7 +101,6 @@ public class GravitinoClient extends GravitinoClientBase
* support the case that the client-side version is higher than the
server-side version.
* @param headers The base header for Gravitino API.
* @param properties A map of properties (key-value pairs) used to configure
the Gravitino client.
- * @throws NoSuchMetalakeException if the metalake with specified name does
not exist.
*/
private GravitinoClient(
String uri,
@@ -107,17 +110,27 @@ public class GravitinoClient extends GravitinoClientBase
Map<String, String> headers,
Map<String, String> properties) {
super(uri, authDataProvider, checkVersion, headers, properties);
- this.metalake = loadMetalake(metalakeName);
+ this.metalakeName = metalakeName;
}
/**
- * Get the current metalake object
+ * Returns the metalake, loading it on first access.
*
* @return the {@link GravitinoMetalake} object
* @throws NoSuchMetalakeException if the metalake with specified name does
not exist.
*/
- private GravitinoMetalake getMetalake() {
- return metalake;
+ private GravitinoMetalake getMetalake() throws NoSuchMetalakeException {
+ GravitinoMetalake result = metalake;
+ if (result == null) {
+ synchronized (this) {
+ result = metalake;
+ if (result == null) {
+ result = loadMetalake(metalakeName);
+ metalake = result;
+ }
+ }
+ }
+ return result;
}
@Override
@@ -752,9 +765,11 @@ public class GravitinoClient extends GravitinoClientBase
/**
* Builds a new GravitinoClient instance.
*
+ * <p>The metalake is loaded lazily on first use; {@link
NoSuchMetalakeException} is thrown at
+ * that point, not here.
+ *
* @return A new instance of GravitinoClient with the specified base URI.
* @throws IllegalArgumentException If the base URI is null or empty.
- * @throws NoSuchMetalakeException if the metalake with specified name
does not exist.
*/
@Override
public GravitinoClient build() {
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoMetalake.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoMetalake.java
index 09ba0761b7..59e413684c 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoMetalake.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoMetalake.java
@@ -1173,6 +1173,27 @@ public class TestGravitinoMetalake extends TestBase {
Assertions.assertNotEquals(metalake1, new Object());
}
+ @Test
+ public void testNoSuchMetalakeOnFirstOperation() throws
JsonProcessingException {
+ // NoSuchMetalakeException surfaces on first operation, not at build().
+ String missingMetalake = "nonexistent-metalake";
+ ErrorResponse errorResp =
+ ErrorResponse.notFound(NoSuchMetalakeException.class.getSimpleName(),
"metalake not found");
+ buildMockResource(
+ Method.GET, "/api/metalakes/" + missingMetalake, null, errorResp,
HttpStatus.SC_NOT_FOUND);
+
+ try (GravitinoClient client =
+ GravitinoClient.builder("http://127.0.0.1:"; +
mockServer.getLocalPort())
+ .withMetalake(missingMetalake)
+ .withVersionCheckDisabled()
+ .build()) {
+ // build() succeeded — exception must surface on first operation
+ Throwable ex =
+ Assertions.assertThrows(NoSuchMetalakeException.class, () ->
client.listCatalogs());
+ Assertions.assertTrue(ex.getMessage().contains("metalake not found"));
+ }
+ }
+
static GravitinoMetalake createMetalake(GravitinoAdminClient client, String
metalakeName)
throws JsonProcessingException {
return createMetalake(client, metalakeName, false);
diff --git
a/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/plugin/TestGravitinoDriverPlugin.java
b/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/plugin/TestGravitinoDriverPlugin.java
index 4ace94b2df..bc17c21a48 100644
---
a/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/plugin/TestGravitinoDriverPlugin.java
+++
b/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/plugin/TestGravitinoDriverPlugin.java
@@ -31,6 +31,7 @@ import java.nio.file.Path;
import java.util.Arrays;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.auth.AuthProperties;
+import org.apache.gravitino.client.GravitinoClient;
import org.apache.gravitino.spark.connector.GravitinoSparkConfig;
import org.apache.gravitino.spark.connector.catalog.SparkCatalogKind;
import
org.apache.gravitino.spark.connector.iceberg.extensions.GravitinoIcebergSparkSessionExtensions;
@@ -314,16 +315,12 @@ public class TestGravitinoDriverPlugin {
SparkConf sparkConf = tokenAuthConf();
sparkConf.set(GravitinoSparkConfig.GRAVITINO_TOKEN_VALUE, "a-token");
- // The client cannot reach a server here, but it must get past auth
configuration first: an
- // unsupported auth type or a missing token would fail before any
connection is attempted.
- Exception e =
- Assertions.assertThrows(
- Exception.class,
- () ->
- GravitinoDriverPlugin.createGravitinoClient(
- "http://127.0.0.1:1";, "metalake", sparkConf, "user",
ImmutableMap.of()));
- Assertions.assertFalse(e instanceof UnsupportedOperationException,
e.toString());
- Assertions.assertFalse(e instanceof IllegalArgumentException,
e.toString());
+ // build() no longer contacts the server; the token is resolved on first
use.
+ GravitinoClient client =
+ GravitinoDriverPlugin.createGravitinoClient(
+ "http://127.0.0.1:1";, "metalake", sparkConf, "user",
ImmutableMap.of());
+ Assertions.assertNotNull(client);
+ client.close();
}
@Test
@@ -373,14 +370,16 @@ public class TestGravitinoDriverPlugin {
void testTokenAuthTypeWithoutTokenFails() {
SparkConf sparkConf = tokenAuthConf();
+ // build() succeeds without a token; the failure surfaces on first use
when the
+ // token provider tries to resolve the token.
+ GravitinoClient client =
+ GravitinoDriverPlugin.createGravitinoClient(
+ "http://127.0.0.1:1";, "metalake", sparkConf, "user",
ImmutableMap.of());
IllegalArgumentException e =
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () ->
- GravitinoDriverPlugin.createGravitinoClient(
- "http://127.0.0.1:1";, "metalake", sparkConf, "user",
ImmutableMap.of()));
+ Assertions.assertThrows(IllegalArgumentException.class,
client::listCatalogs);
Assertions.assertTrue(e.getMessage().contains(GravitinoSparkConfig.GRAVITINO_TOKEN_VALUE));
Assertions.assertTrue(e.getMessage().contains(GravitinoSparkConfig.GRAVITINO_TOKEN_FILE));
+ client.close();
}
private static SparkConf tokenAuthConf() {
