jerryshao commented on code in PR #13367:
URL: https://github.com/apache/gravitino/pull/13367#discussion_r4059385957
##########
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##########
@@ -229,24 +230,39 @@ public static GravitinoEnv getInstance() {
*/
public void initializeBaseComponents(Config config) {
LOG.info("Initializing Gravitino base environment...");
- this.config = config;
- FileFetcher.get().initialize(config.get(Configs.BLOCK_UNSAFE_REMOTE_URI));
- SecretPropertyUtils.configureSensitiveKeyKeywords(config);
+ initializeConfig(config);
this.manageFullComponents = false;
initBaseComponents();
LOG.info("Gravitino base environment is initialized.");
}
+ /**
+ * Initialize metadata access components without server-side integrations.
+ *
+ * <p>This profile provides storage, normalized metadata dispatchers,
governance, configured
+ * authorization, statistics, locking, secrets, and metrics. It excludes
hooks, event and audit
+ * dispatch, auxiliary services, and the job subsystem. It is intended for
processes that need
+ * direct metadata access without hosting the Gravitino server runtime.
+ *
+ * @param config The configuration object to initialize the environment.
+ */
+ public void initializeMetadataComponents(Config config) {
Review Comment:
[Important] This new public entry point is only safe on
`GravitinoEnv.getInstance()`, but neither the Javadoc nor the code says so.
Several components built here resolve part of their dependencies through the
singleton rather than through `this`:
- `core/src/main/java/org/apache/gravitino/lock/TreeLockUtils.java:46` --
`GravitinoEnv.getInstance().lockManager().createTreeLock(...)`, reached from
`CatalogManager`, `MetalakeManager` and the
Schema/Table/Fileset/Topic/Model/Function/View/Partition operation dispatchers.
-
`core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java:106,142`
-- `GravitinoEnv.getInstance().internalSchemaDispatcher()`.
-
`core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:108`
and `ViewOperationDispatcher.java:86` -- the 4-arg constructors capture `() ->
GravitinoEnv.getInstance().internalSchemaDispatcher()`.
-
`core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java:573` --
`GravitinoEnv.getInstance().catalogManager()`.
-
`core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java:660`
-- `GravitinoEnv.getInstance().idGenerator()`;
`stats/storage/JdbcPartitionStatisticStorage.java:110` and
`LancePartitionStatisticStorage.java:136` --
`GravitinoEnv.getInstance().entityStore()`.
Calling `initializeMetadataComponents` on a non-singleton `GravitinoEnv`
therefore yields an env whose fields all look correctly wired, but whose first
real operation resolves against the (possibly uninitialized) singleton -- an
NPE at `TreeLockUtils.java:46` in the common case. The PR's own test shows the
problem: `TestGravitinoEnvMetadataComponents.java:153-157` reflectively
installs a `LockManager` on the singleton so that a `TestGravitinoEnv` instance
is usable at all.
Since the Javadoc advertises this profile for "processes that need direct
metadata access without hosting the Gravitino server runtime", please state the
singleton requirement explicitly, and consider `Preconditions.checkState(this
== getInstance(), ...)` here so misuse fails at initialization rather than at
the first metadata operation.
Verified by: grepped `GravitinoEnv.getInstance()` across
`core/src/main/java` on this branch and read each call site above; confirmed
`TreeLockUtils` is referenced by `CatalogManager`, `MetalakeManager` and all
nine `*OperationDispatcher` classes.
##########
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##########
@@ -427,6 +443,17 @@ public PartitionDispatcher partitionDispatcher() {
return partitionDispatcher;
}
+ /**
+ * Get the internal PartitionDispatcher associated with the Gravitino
environment.
+ *
+ * <p>The internal dispatcher preserves normalization but skips event
emission.
+ *
+ * @return The internal PartitionDispatcher instance.
+ */
+ public PartitionDispatcher internalPartitionDispatcher() {
Review Comment:
[Question] `internalPartitionDispatcher()` has no consumer anywhere in the
tree -- `grep -rn "internalPartitionDispatcher" --include=*.java .` on this
branch returns only the field and accessor in this file plus the
`assertNotNull` in the new test. Is it meant to land ahead of a specific
caller, or should it wait for one?
Related, and worth confirming as deliberate: the full-server profile now
builds two `PartitionNormalizeDispatcher` instances over the single
`PartitionOperationDispatcher` -- one assigned to `internalPartitionDispatcher`
at `GravitinoEnv.java:945-946`, and one built inside
`initPublicMetadataDispatchers` at `GravitinoEnv.java:1088-1090` for
`partitionDispatcher`. The old code had a single instance. It is harmless since
the normalizer is stateless, but it differs from the shape every other metadata
type keeps (one internal normalize dispatcher, one public normalize-over-hook
dispatcher), so it reads like a by-product of the split rather than a decision.
Verified by: repo-wide grep for the symbol on this branch; read both
construction sites and compared them with the pre-PR
`initGravitinoServerComponents`.
##########
core/src/test/java/org/apache/gravitino/TestGravitinoEnvMetadataComponents.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.Collections;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.lock.LockManager;
+import org.apache.gravitino.stats.StatisticManager;
+import org.apache.gravitino.stats.storage.MemoryPartitionStatsStorageFactory;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+
+class TestGravitinoEnvMetadataComponents {
+
+ @Test
+ void
testMetadataProfileProvidesCompleteMetadataAccessWithoutServerServices() throws
Exception {
+ Config config = metadataConfig(false);
+ EntityStore entityStore = relationStore();
+ TestGravitinoEnv env = new TestGravitinoEnv();
+ Object originalLockManager = installSingletonLock(config);
+
+ try (MockedStatic<EntityStoreFactory> entityStoreFactory =
+ mockStatic(EntityStoreFactory.class)) {
+ entityStoreFactory
+ .when(() -> EntityStoreFactory.createEntityStore(config))
+ .thenReturn(entityStore);
+
+ env.initializeMetadataComponents(config);
+
+ assertSame(entityStore, env.entityStore());
+ assertNotNull(env.catalogManager());
+ assertNotNull(env.internalMetalakeDispatcher());
+ assertNotNull(env.internalCatalogDispatcher());
+ assertNotNull(env.internalFilesetDispatcher());
+ assertNotNull(env.internalSchemaDispatcher());
+ assertNotNull(env.internalTableDispatcher());
+ assertNotNull(env.internalPartitionDispatcher());
+ assertNotNull(env.internalTopicDispatcher());
+ assertNotNull(env.internalModelDispatcher());
+ assertNotNull(env.internalFunctionDispatcher());
+ assertNotNull(env.internalViewDispatcher());
+ assertNotNull(env.semanticModelDispatcher());
+ assertNotNull(env.credentialOperationDispatcher());
+ assertNotNull(env.secretPropertyOperationDispatcher());
+ assertNotNull(env.internalTagDispatcher());
+ assertNotNull(env.internalPolicyDispatcher());
+ assertInstanceOf(StatisticManager.class, env.statisticDispatcher());
+ assertNotNull(env.lockManager());
+ assertNotNull(env.metricsSystem());
+ assertNotNull(env.secretManager());
+
+ assertNull(env.auxServiceManager());
+ assertNull(env.eventListenerManager());
+ assertNull(env.metalakeDispatcher());
+ assertNull(env.catalogDispatcher());
+ assertNull(env.filesetDispatcher());
+ assertNull(env.schemaDispatcher());
+ assertNull(env.tableDispatcher());
+ assertNull(env.partitionDispatcher());
+ assertNull(env.topicDispatcher());
+ assertNull(env.modelDispatcher());
+ assertNull(env.functionDispatcher());
+ assertNull(env.viewDispatcher());
+ assertNull(env.tagDispatcher());
+ assertNull(env.policyDispatcher());
+ assertNull(env.internalAccessControlDispatcher());
+ assertNull(env.internalOwnerDispatcher());
+ assertNull(env.bulkManager());
+ assertNull(env.futureGrantManager());
+ assertThrows(IllegalArgumentException.class, env::eventBus);
+ assertThrows(IllegalArgumentException.class,
env::jobOperationDispatcher);
+ assertThrows(IllegalArgumentException.class,
env::internalJobOperationDispatcher);
+
+ assertDoesNotThrow(env::start);
Review Comment:
[Nit] Both tests stop at composition -- assert which fields are non-null,
which are null, then `start()` and `shutdown()`. Nothing drives an operation
through the profile, so the Javadoc claim that this profile provides working
metadata access is not exercised, and none of the `GravitinoEnv.getInstance()`
resolution paths flagged on `initializeMetadataComponents` are reached. A
single call such as `env.internalSchemaDispatcher().listSchemas(...)` against
the mocked `EntityStore` would cover them and would have surfaced the singleton
coupling without the reflection workaround.
Also, `testMetadataProfileProvidesInternalAuthorizationWhenEnabled` omits
the `verify(entityStore).close()` assertion that this test makes at line 108,
so the second test never checks that `shutdown()` released the store.
Verified by: read the full test file on this branch.
##########
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##########
@@ -1045,4 +1046,125 @@ private void initGravitinoServerComponents() {
new BuiltInJobTemplateEventListener(jobManager, entityStore,
idGenerator);
eventListenerManager.addEventListener("builtin-job-template",
builtInJobTemplateListener);
}
+
+ private void initPublicMetadataDispatchers(MetadataOperations
metadataOperations) {
+ // Create and initialize metalake related modules, the operation chain is:
+ // MetalakeEventDispatcher -> MetalakeNormalizeDispatcher ->
MetalakeHookDispatcher ->
+ // MetalakeManager
+ MetalakeHookDispatcher metalakeHookDispatcher = new
MetalakeHookDispatcher(metalakeManager);
+ MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
+ new MetalakeNormalizeDispatcher(metalakeHookDispatcher);
+ this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
+
+ // CatalogEventDispatcher -> CatalogNormalizeDispatcher ->
CatalogHookDispatcher ->
+ // CatalogManager
+ CatalogHookDispatcher catalogHookDispatcher = new
CatalogHookDispatcher(catalogManager);
+ CatalogNormalizeDispatcher catalogNormalizeDispatcher =
+ new CatalogNormalizeDispatcher(catalogHookDispatcher);
+ this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
+
+ FilesetHookDispatcher filesetHookDispatcher =
+ new
FilesetHookDispatcher(metadataOperations.filesetOperationDispatcher);
+ FilesetNormalizeDispatcher filesetNormalizeDispatcher =
+ new FilesetNormalizeDispatcher(filesetHookDispatcher, catalogManager);
+ this.filesetDispatcher = new FilesetEventDispatcher(eventBus,
filesetNormalizeDispatcher);
+
+ SchemaHookDispatcher schemaHookDispatcher =
+ new SchemaHookDispatcher(metadataOperations.schemaOperationDispatcher);
+ SchemaNormalizeDispatcher schemaNormalizeDispatcher =
+ new SchemaNormalizeDispatcher(schemaHookDispatcher, catalogManager);
+ this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
+
+ TableOperationDispatcher tableOperationDispatcher =
+ new TableOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
+ TableHookDispatcher tableHookDispatcher =
+ new TableHookDispatcher(tableOperationDispatcher,
this::internalOwnerDispatcher);
+ TableNormalizeDispatcher tableNormalizeDispatcher =
+ new TableNormalizeDispatcher(tableHookDispatcher, catalogManager);
+ this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
+
+ // TODO: We can install hooks when we need, we only supports ownership
post hook,
+ // partition doesn't have ownership, so we don't need it now.
+ PartitionNormalizeDispatcher partitionNormalizeDispatcher =
+ new PartitionNormalizeDispatcher(
+ metadataOperations.partitionOperationDispatcher, catalogManager);
+ this.partitionDispatcher = new PartitionEventDispatcher(eventBus,
partitionNormalizeDispatcher);
+
+ TopicHookDispatcher topicHookDispatcher =
+ new TopicHookDispatcher(metadataOperations.topicOperationDispatcher);
+ TopicNormalizeDispatcher topicNormalizeDispatcher =
+ new TopicNormalizeDispatcher(topicHookDispatcher, catalogManager);
+ this.topicDispatcher = new TopicEventDispatcher(eventBus,
topicNormalizeDispatcher);
+
+ ModelHookDispatcher modelHookDispatcher =
+ new ModelHookDispatcher(metadataOperations.modelOperationDispatcher);
+ ModelNormalizeDispatcher modelNormalizeDispatcher =
+ new ModelNormalizeDispatcher(modelHookDispatcher, catalogManager);
+ this.modelDispatcher = new ModelEventDispatcher(eventBus,
modelNormalizeDispatcher);
+
+ // Create and initialize Function related modules, the operation chain is:
+ // FunctionEventDispatcher -> FunctionNormalizeDispatcher ->
FunctionHookDispatcher ->
+ // FunctionOperationDispatcher
+ FunctionHookDispatcher functionHookDispatcher =
+ new FunctionHookDispatcher(
+ metadataOperations.functionOperationDispatcher,
this::internalOwnerDispatcher);
+ FunctionNormalizeDispatcher functionNormalizeDispatcher =
+ new FunctionNormalizeDispatcher(functionHookDispatcher,
catalogManager);
+ this.functionDispatcher = new FunctionEventDispatcher(eventBus,
functionNormalizeDispatcher);
+
+ // View operation chain: ViewEventDispatcher -> ViewNormalizeDispatcher ->
ViewHookDispatcher
+ // -> ViewOperationDispatcher.
+ ViewOperationDispatcher viewOperationDispatcher =
+ new ViewOperationDispatcher(catalogManager, entityStore, idGenerator,
secretManager);
+ ViewHookDispatcher viewHookDispatcher =
+ new ViewHookDispatcher(viewOperationDispatcher,
this::internalOwnerDispatcher);
+ ViewNormalizeDispatcher viewNormalizeDispatcher =
+ new ViewNormalizeDispatcher(viewHookDispatcher, catalogManager);
+ this.viewDispatcher = new ViewEventDispatcher(eventBus,
viewNormalizeDispatcher);
+
+ this.statisticDispatcher = new StatisticEventDispatcher(eventBus,
statisticDispatcher);
Review Comment:
[Nit] This line re-reads the field it assigns, so the result depends on
`initMetadataComponents()` having run exactly once -- a second call to
`initPublicMetadataDispatchers` would silently double-wrap the dispatcher
instead of failing.
It is also the only component that reuses the public field for the internal
instance. Every other type got a dedicated `internalXxx` field, so in the
metadata profile `statisticDispatcher()` returns a bare `StatisticManager`
while every other public accessor returns `null` -- the new test pins that
asymmetry at `TestGravitinoEnvMetadataComponents.java:74`. An
`internalStatisticDispatcher` field would keep the two layers distinct, make
this method idempotent like the rest, and give the metadata profile the same
"internal set / public null" shape as tag, policy and access control.
Verified by: read `initMetadataComponents` (`GravitinoEnv.java:898`, where
`statisticDispatcher` is assigned the raw `StatisticManager`) and this method;
`StatisticEventDispatcher(EventBus, StatisticDispatcher)` at
`core/src/main/java/org/apache/gravitino/listener/StatisticEventDispatcher.java:62`
takes the interface, so the self-wrap compiles without warning.
##########
core/src/test/java/org/apache/gravitino/TestGravitinoEnvMetadataComponents.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.Collections;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.lock.LockManager;
+import org.apache.gravitino.stats.StatisticManager;
+import org.apache.gravitino.stats.storage.MemoryPartitionStatsStorageFactory;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+
+class TestGravitinoEnvMetadataComponents {
+
+ @Test
+ void
testMetadataProfileProvidesCompleteMetadataAccessWithoutServerServices() throws
Exception {
+ Config config = metadataConfig(false);
+ EntityStore entityStore = relationStore();
+ TestGravitinoEnv env = new TestGravitinoEnv();
+ Object originalLockManager = installSingletonLock(config);
+
+ try (MockedStatic<EntityStoreFactory> entityStoreFactory =
+ mockStatic(EntityStoreFactory.class)) {
+ entityStoreFactory
+ .when(() -> EntityStoreFactory.createEntityStore(config))
+ .thenReturn(entityStore);
+
+ env.initializeMetadataComponents(config);
+
+ assertSame(entityStore, env.entityStore());
+ assertNotNull(env.catalogManager());
+ assertNotNull(env.internalMetalakeDispatcher());
+ assertNotNull(env.internalCatalogDispatcher());
+ assertNotNull(env.internalFilesetDispatcher());
+ assertNotNull(env.internalSchemaDispatcher());
+ assertNotNull(env.internalTableDispatcher());
+ assertNotNull(env.internalPartitionDispatcher());
+ assertNotNull(env.internalTopicDispatcher());
+ assertNotNull(env.internalModelDispatcher());
+ assertNotNull(env.internalFunctionDispatcher());
+ assertNotNull(env.internalViewDispatcher());
+ assertNotNull(env.semanticModelDispatcher());
+ assertNotNull(env.credentialOperationDispatcher());
+ assertNotNull(env.secretPropertyOperationDispatcher());
+ assertNotNull(env.internalTagDispatcher());
+ assertNotNull(env.internalPolicyDispatcher());
+ assertInstanceOf(StatisticManager.class, env.statisticDispatcher());
+ assertNotNull(env.lockManager());
+ assertNotNull(env.metricsSystem());
+ assertNotNull(env.secretManager());
+
+ assertNull(env.auxServiceManager());
+ assertNull(env.eventListenerManager());
+ assertNull(env.metalakeDispatcher());
+ assertNull(env.catalogDispatcher());
+ assertNull(env.filesetDispatcher());
+ assertNull(env.schemaDispatcher());
+ assertNull(env.tableDispatcher());
+ assertNull(env.partitionDispatcher());
+ assertNull(env.topicDispatcher());
+ assertNull(env.modelDispatcher());
+ assertNull(env.functionDispatcher());
+ assertNull(env.viewDispatcher());
+ assertNull(env.tagDispatcher());
+ assertNull(env.policyDispatcher());
+ assertNull(env.internalAccessControlDispatcher());
+ assertNull(env.internalOwnerDispatcher());
+ assertNull(env.bulkManager());
+ assertNull(env.futureGrantManager());
+ assertThrows(IllegalArgumentException.class, env::eventBus);
+ assertThrows(IllegalArgumentException.class,
env::jobOperationDispatcher);
+ assertThrows(IllegalArgumentException.class,
env::internalJobOperationDispatcher);
+
+ assertDoesNotThrow(env::start);
+ verify(entityStore).initialize(config);
+ } finally {
+ env.shutdown();
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager",
originalLockManager, true);
+ }
+
+ verify(entityStore).close();
+ }
+
+ @Test
+ void testMetadataProfileProvidesInternalAuthorizationWhenEnabled() throws
Exception {
+ Config config = metadataConfig(true);
+ EntityStore entityStore = relationStore();
+ TestGravitinoEnv env = new TestGravitinoEnv();
+ Object originalLockManager = installSingletonLock(config);
+
+ try (MockedStatic<EntityStoreFactory> entityStoreFactory =
+ mockStatic(EntityStoreFactory.class)) {
+ entityStoreFactory
+ .when(() -> EntityStoreFactory.createEntityStore(config))
+ .thenReturn(entityStore);
+
+ env.initializeMetadataComponents(config);
+
+ assertNotNull(env.internalAccessControlDispatcher());
+ assertNotNull(env.internalOwnerDispatcher());
+ assertNotNull(env.bulkManager());
+ assertNotNull(env.futureGrantManager());
+ assertNull(env.accessControlDispatcher());
+ assertNull(env.ownerDispatcher());
+ } finally {
+ env.shutdown();
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager",
originalLockManager, true);
+ }
+ }
+
+ private static Config metadataConfig(boolean enableAuthorization) {
+ Config config = new Config(false) {};
+ config.set(Configs.ENABLE_AUTHORIZATION, enableAuthorization);
+ config.set(Configs.SERVICE_ADMINS, Collections.singletonList("admin"));
+ config.set(
+ Configs.PARTITION_STATS_STORAGE_FACTORY_CLASS,
+ MemoryPartitionStatsStorageFactory.class.getCanonicalName());
+ return config;
+ }
+
+ private static EntityStore relationStore() {
+ return mock(
+ EntityStore.class,
withSettings().extraInterfaces(SupportsRelationOperations.class));
+ }
+
+ private static Object installSingletonLock(Config config) throws
IllegalAccessException {
+ Object originalLockManager =
+ FieldUtils.readField(GravitinoEnv.getInstance(), "lockManager", true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new
LockManager(config), true);
Review Comment:
[Nit] This reflectively replaces the `lockManager` of the real
`GravitinoEnv` singleton for the duration of the test, and both tests in this
class do it. While the patched value is in place, any other test in the same
JVM that acquires a tree lock gets this `LockManager` instead of its own, since
`core/src/main/java/org/apache/gravitino/lock/TreeLockUtils.java:46` always
resolves `GravitinoEnv.getInstance().lockManager()`. The restore in the
`finally` block covers sequential execution but not concurrent execution inside
the JVM.
If the metadata profile is in fact singleton-only (see the comment on
`initializeMetadataComponents`), running the test against
`GravitinoEnv.getInstance()` would both remove the need for this reflection and
test the supported usage.
Verified by: read `TreeLockUtils.java:46` and confirmed it is the single
entry point used by `CatalogManager`, `MetalakeManager` and the operation
dispatchers for lock acquisition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
