This is an automated email from the ASF dual-hosted git repository.
diqiu50 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 5dfa38803e [#12145] fix(test): prevent overlapping embedded server
lifecycle (#12146)
5dfa38803e is described below
commit 5dfa38803e577a3c666ff23374e6158bb5988da7
Author: Qi Yu <[email protected]>
AuthorDate: Mon Jul 27 14:41:21 2026 +0800
[#12145] fix(test): prevent overlapping embedded server lifecycle (#12146)
### What changes were proposed in this pull request?
Wait for the embedded Gravitino server task to terminate completely in
`MiniGravitino.stop()` instead of treating an unavailable HTTP port as
completion.
Always close the REST client and delete the temporary configuration
directory,
even when server termination times out or is interrupted. Preserve the
original
shutdown failure and attach REST client cleanup failures as suppressed
exceptions.
### Why are the changes needed?
Embedded integration tests repeatedly start and stop Gravitino servers
in the
same JVM. The HTTP port can close before `GravitinoEnv.shutdown()`
finishes,
allowing the next server to initialize the singleton environment while
the old
server is still shutting down.
The old shutdown can then close the new server catalog manager, leaving
its
`ClassLoaderPool` permanently closed and causing catalog creation to
fail with:
`IllegalStateException: ClassLoaderPool is already closed`
Fix: #12145
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `./gradlew :integration-test-common:check
:integration-test-common:spotlessCheck -PskipITs`
- `./gradlew :core:test --tests
org.apache.gravitino.catalog.TestClassLoaderPoolIntegration
:integration-test-common:compileTestJava -PskipITs`
- `./gradlew :clients:client-java:test --tests
org.apache.gravitino.client.integration.test.MetalakeIT
-PskipDockerTests=true`
- `git diff --check`
---------
Signed-off-by: yuqi <[email protected]>
---
integration-test-common/build.gradle.kts | 1 +
.../gravitino/integration/test/MiniGravitino.java | 80 +++++++++++++-------
.../integration/test/TestMiniGravitino.java | 85 ++++++++++++++++++++++
3 files changed, 140 insertions(+), 26 deletions(-)
diff --git a/integration-test-common/build.gradle.kts
b/integration-test-common/build.gradle.kts
index 5f7063496d..a73ec0a627 100644
--- a/integration-test-common/build.gradle.kts
+++ b/integration-test-common/build.gradle.kts
@@ -46,6 +46,7 @@ dependencies {
testImplementation(libs.commons.io)
testImplementation(libs.guava)
testImplementation(libs.httpclient5)
+ testImplementation(libs.mockito.core)
testImplementation(libs.testcontainers)
testImplementation(libs.testcontainers.mysql)
testImplementation(libs.testcontainers.postgresql)
diff --git
a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitino.java
b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitino.java
index eb26fb37e4..65fb60a9b5 100644
---
a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitino.java
+++
b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/MiniGravitino.java
@@ -24,6 +24,7 @@ import static
org.apache.gravitino.lance.common.config.LanceConfig.LANCE_CONFIG_
import static
org.apache.gravitino.lance.common.config.LanceConfig.NAMESPACE_BACKEND;
import static
org.apache.gravitino.lance.common.config.LanceConfig.NAMESPACE_BACKEND_URI;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import java.io.File;
@@ -40,6 +41,7 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import javax.annotation.Nullable;
import org.apache.commons.io.FileUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
@@ -68,10 +70,10 @@ public class MiniGravitino {
private static final Logger LOG =
LoggerFactory.getLogger(MiniGravitino.class);
private static final Splitter COMMA =
Splitter.on(",").omitEmptyStrings().trimResults();
private MiniGravitinoContext context;
- private RESTClient restClient;
+ @Nullable private RESTClient restClient;
private final File mockConfDir;
private final ServerConfig serverConfig = new ServerConfig();
- private final ExecutorService executor = Executors.newSingleThreadExecutor();
+ private final ExecutorService executor;
private Properties properties;
private String host;
@@ -79,8 +81,23 @@ public class MiniGravitino {
private int port;
public MiniGravitino(MiniGravitinoContext context) throws IOException {
+ this(
+ context,
+ Files.createTempDirectory("MiniGravitino").toFile(),
+ Executors.newSingleThreadExecutor(),
+ null);
+ }
+
+ @VisibleForTesting
+ MiniGravitino(
+ MiniGravitinoContext context,
+ File mockConfDir,
+ ExecutorService executor,
+ @Nullable RESTClient restClient) {
this.context = context;
- this.mockConfDir = Files.createTempDirectory("MiniGravitino").toFile();
+ this.mockConfDir = mockConfDir;
+ this.executor = executor;
+ this.restClient = restClient;
mockConfDir.mkdirs();
}
@@ -216,31 +233,23 @@ public class MiniGravitino {
public void stop() throws IOException, InterruptedException {
LOG.debug("MiniGravitino shutDown...");
- executor.shutdown();
- sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
- executor.shutdownNow();
-
- long beginTime = System.currentTimeMillis();
- boolean started = true;
-
- String url = String.format("http://%s:%d/metrics";, host, port);
- while (System.currentTimeMillis() - beginTime < 1000 * 60 * 3) {
- sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
- started = HttpUtils.isHttpServerUp(url);
- if (!started) {
- break;
- }
- }
-
- restClient.close();
+ Throwable failure = null;
try {
- FileUtils.deleteDirectory(mockConfDir);
- } catch (Exception e) {
- // Ignore
- }
+ executor.shutdown();
+ sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
+ executor.shutdownNow();
- if (started) {
- throw new RuntimeException("Can not stop Gravitino server");
+ // The HTTP port may be closed before GravitinoServer.main() finishes
shutting down the
+ // singleton GravitinoEnv. Wait for the server task to terminate so the
next embedded server
+ // cannot initialize the same environment while this shutdown is still
in progress.
+ if (!executor.awaitTermination(3, TimeUnit.MINUTES)) {
+ throw new RuntimeException("Can not terminate MiniGravitino server
task");
+ }
+ } catch (InterruptedException | RuntimeException | Error e) {
+ failure = e;
+ throw e;
+ } finally {
+ cleanupResources(failure);
}
LOG.debug("MiniGravitino terminated.");
@@ -300,6 +309,25 @@ public class MiniGravitino {
return ImmutableMap.copyOf(customConfigs);
}
+ private void cleanupResources(@Nullable Throwable failure) throws
IOException {
+ try {
+ if (restClient != null) {
+ restClient.close();
+ }
+ } catch (IOException e) {
+ if (failure == null) {
+ throw e;
+ }
+ failure.addSuppressed(e);
+ } finally {
+ try {
+ FileUtils.deleteDirectory(mockConfDir);
+ } catch (Exception e) {
+ LOG.warn("Failed to delete MiniGravitino configuration directory {}",
mockConfDir, e);
+ }
+ }
+ }
+
// Customize the config file
private void customizeConfigFile(String configTempFileName, String
configFileName)
throws IOException {
diff --git
a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/TestMiniGravitino.java
b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/TestMiniGravitino.java
new file mode 100644
index 0000000000..e0f75347eb
--- /dev/null
+++
b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/TestMiniGravitino.java
@@ -0,0 +1,85 @@
+/*
+ * 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.integration.test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+import org.apache.gravitino.client.RESTClient;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class TestMiniGravitino {
+
+ @TempDir private Path mockConfDir;
+
+ @Test
+ void testStopCleansResourcesWhenServerTaskDoesNotTerminate() throws
Exception {
+ ExecutorService executor = mock(ExecutorService.class);
+ RESTClient restClient = mock(RESTClient.class);
+ MiniGravitino miniGravitino = createMiniGravitino(executor, restClient);
+ when(executor.awaitTermination(3, TimeUnit.MINUTES)).thenReturn(false);
+
+ RuntimeException exception = assertThrows(RuntimeException.class,
miniGravitino::stop);
+
+ assertEquals("Can not terminate MiniGravitino server task",
exception.getMessage());
+ verify(restClient).close();
+ assertFalse(Files.exists(mockConfDir));
+ }
+
+ @Test
+ void testStopPreservesInterruptionWhenResourceCleanupFails() throws
Exception {
+ ExecutorService executor = mock(ExecutorService.class);
+ RESTClient restClient = mock(RESTClient.class);
+ MiniGravitino miniGravitino = createMiniGravitino(executor, restClient);
+ InterruptedException interruption = new
InterruptedException("interrupted");
+ IOException closeFailure = new IOException("close failed");
+ when(executor.awaitTermination(3,
TimeUnit.MINUTES)).thenThrow(interruption);
+ doThrow(closeFailure).when(restClient).close();
+
+ InterruptedException exception = assertThrows(InterruptedException.class,
miniGravitino::stop);
+
+ assertSame(interruption, exception);
+ assertEquals(1, exception.getSuppressed().length);
+ assertSame(closeFailure, exception.getSuppressed()[0]);
+ assertFalse(Files.exists(mockConfDir));
+ }
+
+ private MiniGravitino createMiniGravitino(ExecutorService executor,
RESTClient restClient)
+ throws IOException {
+ Files.writeString(mockConfDir.resolve("gravitino.conf"), "test");
+ return new MiniGravitino(
+ new MiniGravitinoContext(Collections.emptyMap(), true, true),
+ mockConfDir.toFile(),
+ executor,
+ restClient);
+ }
+}
