This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 50d83f1c66 [#12145] fix(test): prevent overlapping embedded server
lifecycle on branch-1.3 (#13008)
50d83f1c66 is described below
commit 50d83f1c66f8d308420eeb5ba89f0aa542f42760
Author: Qi Yu <[email protected]>
AuthorDate: Wed Sep 9 09:06:53 2026 +0800
[#12145] fix(test): prevent overlapping embedded server lifecycle on
branch-1.3 (#13008)
### What changes were proposed in this pull request?
Backport #12146 to `branch-1.3`. Wait for the MiniGravitino server task
to terminate before returning from `stop()`, and clean up resources even
if shutdown fails. Add a regression test that delays server cleanup and
verifies that `stop()` waits for it.
### Why are the changes needed?
The HTTP port can close before the shared `GravitinoEnv` finishes
shutting down. Starting the next embedded server during that window lets
the previous shutdown close its catalog manager. This caused
`CatalogPaimonJdbcCredentialIT` initialization to fail with
`CatalogManager is already closed` on `branch-1.3`.
CI failure:
https://github.com/apache/gravitino/actions/runs/34222422837/job/102048711221
Fix: #12145
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `./gradlew :integration-test-common:test --tests
org.apache.gravitino.integration.test.TestMiniGravitino -PskipITs` — 3
tests passed.
- `./gradlew spotlessApply :integration-test-common:check -PskipITs` — 9
tests passed.
- `git diff HEAD --check`
---
integration-test-common/build.gradle.kts | 1 +
.../gravitino/integration/test/MiniGravitino.java | 80 ++++++++----
.../integration/test/TestMiniGravitino.java | 140 +++++++++++++++++++++
3 files changed, 195 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..b117b98590
--- /dev/null
+++
b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/TestMiniGravitino.java
@@ -0,0 +1,140 @@
+/*
+ * 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.junit.jupiter.api.Assertions.assertTrue;
+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.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+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 testStopWaitsForServerCleanup() throws Exception {
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ ExecutorService stopper = Executors.newSingleThreadExecutor();
+ RESTClient restClient = mock(RESTClient.class);
+ MiniGravitino miniGravitino = createMiniGravitino(executor, restClient);
+ CountDownLatch started = new CountDownLatch(1);
+ CountDownLatch cleanupStarted = new CountDownLatch(1);
+ CountDownLatch finishCleanup = new CountDownLatch(1);
+ try {
+ Future<?> serverTask =
+ executor.submit(
+ () -> {
+ started.countDown();
+ try {
+ new CountDownLatch(1).await();
+ } catch (InterruptedException expected) {
+ // Simulate environment cleanup after the server is
interrupted.
+ cleanupStarted.countDown();
+ assertTrue(finishCleanup.await(10, TimeUnit.SECONDS));
+ }
+ return null;
+ });
+ assertTrue(started.await(10, TimeUnit.SECONDS));
+ Future<?> stopTask =
+ stopper.submit(
+ () -> {
+ miniGravitino.stop();
+ return null;
+ });
+
+ assertTrue(cleanupStarted.await(10, TimeUnit.SECONDS));
+ assertThrows(TimeoutException.class, () -> stopTask.get(1,
TimeUnit.SECONDS));
+ assertTrue(Files.exists(mockConfDir));
+
+ finishCleanup.countDown();
+ stopTask.get(10, TimeUnit.SECONDS);
+ serverTask.get(10, TimeUnit.SECONDS);
+ assertTrue(executor.isTerminated());
+ verify(restClient).close();
+ assertFalse(Files.exists(mockConfDir));
+ } finally {
+ finishCleanup.countDown();
+ executor.shutdownNow();
+ stopper.shutdownNow();
+ assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
+ assertTrue(stopper.awaitTermination(10, TimeUnit.SECONDS));
+ }
+ }
+
+ @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);
+ }
+}
