This is an automated email from the ASF dual-hosted git repository.
jmclean 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 72182ff66a [#10327] improvement(server): ensure shutdown hook runs app
cleanup on SIGTERM (#10435)
72182ff66a is described below
commit 72182ff66a6a8b16d00e97134cffcdeb5676aab9
Author: Jalina2007 <[email protected]>
AuthorDate: Wed Apr 8 10:34:32 2026 +0530
[#10327] improvement(server): ensure shutdown hook runs app cleanup on
SIGTERM (#10435)
### What changes were proposed in this pull request?
This PR improves the shutdown handling of GravitinoServer.
A new gracefulStop() method is introduced to ensure server.stop() is
executed safely and only once. The shutdown hook now calls
gracefulStop() after the shutdown timeout so that application-level
cleanup runs when the JVM receives a termination signal (e.g., SIGTERM).
The normal shutdown flow in main was also updated to call
gracefulStop().
A unit test was added to verify that the shutdown hook invokes
server.stop() as given in the issue description.
### Why are the changes needed?
Currently, the shutdown hook only sleeps while the cleanup logic runs
after server.join(). When the process receives SIGTERM, the cleanup path
may be skipped.
This change ensures cleanup always runs during external termination and
normal exits.
Fix: #10327
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added the unit test suggested in the issue:
testMainShutdownHookShouldInvokeServerStop()
Also verified that gracefulStop() prevents stop() from executing
multiple times using temporary logging.
---
.../org/apache/gravitino/server/GravitinoServer.java | 14 ++++++++++++--
.../apache/gravitino/server/TestGravitinoServer.java | 20 ++++++++++++++++++++
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
index 90a537f885..1f96d7543b 100644
--- a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
+++ b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
@@ -22,6 +22,7 @@ import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.Properties;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.inject.Singleton;
import javax.servlet.Servlet;
import org.apache.gravitino.Configs;
@@ -88,6 +89,8 @@ public class GravitinoServer extends ResourceConfig {
private final LineageService lineageService;
+ private final AtomicBoolean isStopped = new AtomicBoolean(false);
+
public GravitinoServer(ServerConfig config, GravitinoEnv gravitinoEnv) {
this.serverConfig = config;
this.server = new JettyServer();
@@ -198,6 +201,13 @@ public class GravitinoServer extends ResourceConfig {
}
}
+ void gracefulStop() throws IOException {
+ if (!isStopped.compareAndSet(false, true)) {
+ return;
+ }
+ stop();
+ }
+
public static void main(String[] args) {
LOG.info("Starting Gravitino Server");
String confPath = System.getenv("GRAVITINO_TEST") == null ? "" : args[0];
@@ -219,8 +229,8 @@ public class GravitinoServer extends ResourceConfig {
new Thread(
() -> {
try {
- // Register some clean-up tasks that need to be done
before shutting down
Thread.sleep(server.serverConfig.get(ServerConfig.SERVER_SHUTDOWN_TIMEOUT));
+ server.gracefulStop();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
LOG.error("Interrupted exception:", e);
@@ -233,7 +243,7 @@ public class GravitinoServer extends ResourceConfig {
LOG.info("Shutting down Gravitino Server ... ");
try {
- server.stop();
+ server.gracefulStop();
LOG.info("Gravitino Server has shut down.");
} catch (Exception e) {
LOG.error("Error while stopping Gravitino Server", e);
diff --git
a/server/src/test/java/org/apache/gravitino/server/TestGravitinoServer.java
b/server/src/test/java/org/apache/gravitino/server/TestGravitinoServer.java
index 2155047c9e..0c5cfa48e6 100644
--- a/server/src/test/java/org/apache/gravitino/server/TestGravitinoServer.java
+++ b/server/src/test/java/org/apache/gravitino/server/TestGravitinoServer.java
@@ -20,9 +20,11 @@ package org.apache.gravitino.server;
import static org.apache.gravitino.Configs.ENTITY_RELATIONAL_JDBC_BACKEND_PATH;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.apache.commons.io.FileUtils;
@@ -116,4 +118,22 @@ public class TestGravitinoServer {
// TODO: Exception due to environment variable not set. Is this the right
exception?
assertThrows(IllegalArgumentException.class, () ->
config.loadFromFile("config"));
}
+
+ @Test
+ public void testMainShutdownHookShouldInvokeServerStop() throws IOException {
+ Path sourceFile =
Path.of("src/main/java/org/apache/gravitino/server/GravitinoServer.java");
+ String source = Files.readString(sourceFile);
+
+ int hookStart = source.indexOf("addShutdownHook");
+ int joinIndex = source.indexOf("server.join();", hookStart);
+
+ assertTrue(hookStart >= 0, "Main should register a shutdown hook");
+ assertTrue(
+ joinIndex > hookStart, "Main should call server.join() after
registering shutdown hook");
+
+ String hookBlock = source.substring(hookStart, joinIndex);
+ assertTrue(
+ hookBlock.contains("server.gracefulStop()"),
+ "Shutdown hook should invoke server.gracefulStop() so app-level
cleanup runs on SIGTERM");
+ }
}
