This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 24968cdf47 [ZEPPELIN-6693] Drive NotebookServer heartbeat scheduler
shutdown from ZeppelinServer lifecycle
24968cdf47 is described below
commit 24968cdf471dca93c8d6125e58fcd597eb1ead46
Author: JangAyeon <[email protected]>
AuthorDate: Fri Sep 25 18:34:48 2026 +0900
[ZEPPELIN-6693] Drive NotebookServer heartbeat scheduler shutdown from
ZeppelinServer lifecycle
### What is this PR for?
ZEPPELIN-6092 added a websocket heartbeat scheduler to `NotebookServer`,
torn down by a dedicated JVM shutdown hook that the class registered for
itself. That hook only covers the JVM-exit path:
- When the server is closed inside a running JVM (e.g.
`MiniZeppelinServer.shutDown()` calling `zepServer.close()`), the hook never
runs, so repeated start/stop cycles can leave a scheduler and a hook behind per
instance.
- On SIGTERM, `ZeppelinServer`'s own shutdown hook and this one run in
parallel for the same event.
This PR stops the scheduler from `ZeppelinServer#shutdown`, which already
covers both paths (the JVM shutdown hook and `close()`), and removes the
dedicated hook together with its `removeShutdownHook` / `IllegalStateException`
handling and self-reference guard. The scheduler is stopped after Jetty, so no
new connection can restart it.
### What type of PR is it?
Improvement
### Todos
* [x] - Stop the heartbeat scheduler from `ZeppelinServer#shutdown`
* [x] - Remove the dedicated shutdown hook from `NotebookServer`
* [x] - Add a test that covers the `ZeppelinServer#shutdown` path
* [x] - Add tests for repeated start/stop cycles
### What is the Jira issue?
[ZEPPELIN-6693](https://issues.apache.org/jira/browse/ZEPPELIN-6693)
### How should this be tested?
*
`NotebookServerHeartbeatTest#zeppelinServerShutdownStopsHeartbeatScheduler`:
starts a `MiniZeppelinServer`, starts the heartbeat scheduler, calls
`shutDown()`, and checks the scheduler is shut down. Fails if the
`stopHeartbeatScheduler()` call in `ZeppelinServer#shutdown` is removed.
*
`NotebookServerHeartbeatTest#stopHeartbeatSchedulerAllowsRepeatedStartStopCycles`:
starts and stops the scheduler three times and checks that each scheduler is
shut down and a new one can start.
*
`NotebookServerHeartbeatTest#stopHeartbeatSchedulerIsSafeWhenNeverStarted`:
stopping without a prior start, twice, does not throw.
* Existing `NotebookServerHeartbeatTest` cases still pass.
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5499 from JangAyeon/ZEPPELIN-6693.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../org/apache/zeppelin/server/ZeppelinServer.java | 2 +
.../org/apache/zeppelin/socket/NotebookServer.java | 18 ++-------
.../socket/NotebookServerHeartbeatTest.java | 46 ++++++++++++++++++++++
3 files changed, 51 insertions(+), 15 deletions(-)
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
index 6dece8a13d..480422b4a5 100644
---
a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
+++
b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java
@@ -359,6 +359,8 @@ public class ZeppelinServer implements AutoCloseable {
jettyWebServer.stop();
}
if (sharedServiceLocator != null) {
+ // Stop after Jetty so no new connection can restart the heartbeat
scheduler.
+
sharedServiceLocator.getService(NotebookServer.class).stopHeartbeatScheduler();
if (!zConf.isRecoveryEnabled()) {
sharedServiceLocator.getService(InterpreterSettingManager.class).close();
}
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index 41a65e3a4f..d55eb27196 100644
---
a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++
b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -150,7 +150,6 @@ public class NotebookServer implements
AngularObjectRegistryListener,
// Package-private (not private) so NotebookServerHeartbeatTest can observe
scheduler
// lifecycle without exposing it as part of the public API.
ScheduledExecutorService heartbeatScheduler;
- private Thread heartbeatShutdownHook;
private boolean heartbeatInitialized;
// TODO(jl): This will be removed by handling session directly
@@ -287,29 +286,18 @@ public class NotebookServer implements
AngularObjectRegistryListener,
});
heartbeatScheduler.scheduleAtFixedRate(
this::sendHeartbeat, intervalMs, intervalMs, TimeUnit.MILLISECONDS);
- heartbeatShutdownHook = new Thread(this::stopHeartbeatScheduler);
- Runtime.getRuntime().addShutdownHook(heartbeatShutdownHook);
LOGGER.info("Started websocket heartbeat scheduler with interval {} ms",
intervalMs);
}
/**
- * Stops the websocket heartbeat scheduler, if running, and deregisters its
shutdown hook so
- * repeated start/stop cycles do not accumulate hooks. Safe to call multiple
times and safe
- * to call when the scheduler was never started.
+ * Stops the websocket heartbeat scheduler, if running. Safe to call
multiple times and safe
+ * to call when the scheduler was never started; a later connection starts
it again.
*/
- synchronized void stopHeartbeatScheduler() {
+ public synchronized void stopHeartbeatScheduler() {
if (heartbeatScheduler != null) {
heartbeatScheduler.shutdownNow();
heartbeatScheduler = null;
}
- if (heartbeatShutdownHook != null && Thread.currentThread() !=
heartbeatShutdownHook) {
- try {
- Runtime.getRuntime().removeShutdownHook(heartbeatShutdownHook);
- } catch (IllegalStateException e) {
- // JVM is already shutting down; the hook will simply run (as a
harmless no-op).
- }
- heartbeatShutdownHook = null;
- }
heartbeatInitialized = false;
}
diff --git
a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerHeartbeatTest.java
b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerHeartbeatTest.java
index 6f53668261..f932b1fa2d 100644
---
a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerHeartbeatTest.java
+++
b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerHeartbeatTest.java
@@ -19,11 +19,14 @@ package org.apache.zeppelin.socket;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+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.util.concurrent.ScheduledExecutorService;
+import org.apache.zeppelin.MiniZeppelinServer;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.notebook.AuthorizationService;
import org.junit.jupiter.api.AfterEach;
@@ -106,4 +109,47 @@ class NotebookServerHeartbeatTest {
assertNull(server.heartbeatScheduler);
}
+
+ @Test
+ void stopHeartbeatSchedulerAllowsRepeatedStartStopCycles() {
+ NotebookServer server = buildNotebookServer(50L);
+
+ for (int i = 0; i < 3; i++) {
+ server.startHeartbeatScheduler();
+ ScheduledExecutorService scheduler = server.heartbeatScheduler;
+ assertNotNull(scheduler);
+
+ server.stopHeartbeatScheduler();
+
+ assertTrue(scheduler.isShutdown());
+ assertNull(server.heartbeatScheduler);
+ }
+ }
+
+ @Test
+ void stopHeartbeatSchedulerIsSafeWhenNeverStarted() {
+ NotebookServer server = buildNotebookServer(50L);
+
+ assertDoesNotThrow(server::stopHeartbeatScheduler);
+ assertDoesNotThrow(server::stopHeartbeatScheduler);
+ }
+
+ @Test
+ void zeppelinServerShutdownStopsHeartbeatScheduler() throws Exception {
+ MiniZeppelinServer zepServer =
+ new
MiniZeppelinServer(NotebookServerHeartbeatTest.class.getSimpleName());
+ try {
+ zepServer.start();
+ NotebookServer server = zepServer.getService(NotebookServer.class);
+ server.startHeartbeatScheduler();
+ ScheduledExecutorService scheduler = server.heartbeatScheduler;
+ assertNotNull(scheduler);
+
+ zepServer.shutDown();
+
+ assertTrue(scheduler.isShutdown());
+ } finally {
+ zepServer.destroy();
+ }
+ }
}
