Skip to content

Commit ec7fcea

Browse files
author
wangzhigang
committed
refactor: Update Shutdown Watchdog module for improved artifact naming and documentation
- Changed artifact ID in `pom.xml` to remove Scala version suffix for consistency. - Updated documentation to clarify building and usage instructions, including timeout configuration. - Adjusted test scripts to reflect new artifact naming conventions. - Enhanced code readability and maintainability by simplifying the watchdog thread initialization and configuration handling. - Added test-only helper method to check if the watchdog is running.
1 parent f235edc commit ec7fcea

File tree

7 files changed

+29
-52
lines changed

7 files changed

+29
-52
lines changed

docs/extensions/engines/spark/shutdown-watchdog.md

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,12 @@ build/mvn clean package -DskipTests \
3030
-pl extensions/spark/kyuubi-spark-shutdown-watchdog -am
3131
```
3232

33-
The module still publishes artifacts with the standard Scala suffix
34-
(`kyuubi-spark-shutdown-watchdog_2.12`, `..._2.13`, etc.) so that it aligns with
35-
the rest of the project. Maven expands `${scala.binary.version}` automatically,
36-
so you can run the command above without worrying about Scala version specifics.
3733
Because the implementation is pure Java, there are no Scala runtime
38-
dependencies—building by module path is enough.
34+
dependencies. You can build it with Spark Scala 2.12 libraries and use the
35+
resulting jar with Spark applications running on Scala 2.13 (or vice versa).
3936

4037
After the build succeeds the jar is located at:
41-
`./extensions/spark/kyuubi-spark-shutdown-watchdog/target/kyuubi-spark-shutdown-watchdog_${scala.binary.version}-${project.version}.jar`
38+
`./extensions/spark/kyuubi-spark-shutdown-watchdog/target/kyuubi-spark-shutdown-watchdog-${project.version}.jar`
4239

4340
## Installing
4441

@@ -55,11 +52,10 @@ Add the plugin class to `spark.plugins` when launching the Spark SQL engine:
5552
spark.plugins=org.apache.spark.kyuubi.shutdown.watchdog.SparkShutdownWatchdogPlugin
5653
```
5754

58-
Configure the timeout directly through Spark (see also the general configuration
59-
table in `docs/configuration/settings.md`):
55+
Configure the timeout directly through Spark:
6056

6157
```properties
62-
spark.kyuubi.shutdown.watchdog.timeout=60000
58+
spark.kyuubi.shutdown.watchdog.timeout=1m
6359
```
6460

6561
Tune this value according to how long you expect the engine to take during a
@@ -70,7 +66,7 @@ has already stalled.
7066

7167
## Configuration
7268

73-
| Configuration Key | Default | Description |
69+
| Configuration Key | Default | Description |
7470
|----------------------------------------|---------|------------------------------------------------------------------------------------------------------------|
7571
| spark.kyuubi.shutdown.watchdog.enabled | true | Enables/disables the plugin globally. |
7672
| spark.kyuubi.shutdown.watchdog.timeout | 0 | Maximum wait (milliseconds) for graceful shutdown before forcing termination. `0` or negative disables it. |

extensions/spark/kyuubi-spark-shutdown-watchdog/TESTING.md

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ mvn clean package
3737

3838
The JAR file will be generated at:
3939
```
40-
target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar
40+
target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar
4141
```
4242

4343
## Unit Tests
@@ -110,15 +110,15 @@ public class TestNormalShutdown {
110110

111111
```bash
112112
# Compile
113-
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar" TestNormalShutdown.java
113+
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar" TestNormalShutdown.java
114114

115115
# Run
116116
$SPARK_HOME/bin/spark-submit \
117117
--class TestNormalShutdown \
118118
--conf "spark.kyuubi.shutdown.watchdog.enabled=true" \
119119
--conf "spark.kyuubi.shutdown.watchdog.timeout=5s" \
120120
--conf "spark.plugins=org.apache.spark.kyuubi.shutdown.watchdog.SparkShutdownWatchdogPlugin" \
121-
--jars target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar \
121+
--jars target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar \
122122
. \
123123
TestNormalShutdown
124124
```
@@ -197,15 +197,15 @@ public class TestHangingShutdown {
197197

198198
```bash
199199
# Compile
200-
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar" TestHangingShutdown.java
200+
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar" TestHangingShutdown.java
201201

202202
# Run (capture output to log file)
203203
$SPARK_HOME/bin/spark-submit \
204204
--class TestHangingShutdown \
205205
--conf "spark.kyuubi.shutdown.watchdog.enabled=true" \
206206
--conf "spark.kyuubi.shutdown.watchdog.timeout=5s" \
207207
--conf "spark.plugins=org.apache.spark.kyuubi.shutdown.watchdog.SparkShutdownWatchdogPlugin" \
208-
--jars target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar \
208+
--jars target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar \
209209
. \
210210
TestHangingShutdown 2>&1 | tee test-hanging.log
211211
```
@@ -265,7 +265,7 @@ $SPARK_HOME/bin/spark-submit \
265265
--conf "spark.kyuubi.shutdown.watchdog.enabled=false" \
266266
--conf "spark.kyuubi.shutdown.watchdog.timeout=5s" \
267267
--conf "spark.plugins=org.apache.spark.kyuubi.shutdown.watchdog.SparkShutdownWatchdogPlugin" \
268-
--jars target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar \
268+
--jars target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar \
269269
. \
270270
TestHangingShutdown 2>&1 | tee test-disabled.log &
271271

@@ -324,10 +324,10 @@ public class TestThreadDump {
324324

325325
```bash
326326
# Compile (need slf4j-api and slf4j-simple for logger)
327-
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar" TestThreadDump.java
327+
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar" TestThreadDump.java
328328

329329
# Run
330-
java -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar:." TestThreadDump
330+
java -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar:." TestThreadDump
331331
```
332332

333333
3. **Expected Results**:
@@ -486,15 +486,15 @@ mvn test
486486
# Save as TestHangingShutdown.java
487487

488488
# 5. Compile test application
489-
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar" TestHangingShutdown.java
489+
javac -cp "$SPARK_HOME/jars/*:target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar" TestHangingShutdown.java
490490

491491
# 6. Run hanging shutdown test
492492
$SPARK_HOME/bin/spark-submit \
493493
--class TestHangingShutdown \
494494
--conf "spark.kyuubi.shutdown.watchdog.enabled=true" \
495495
--conf "spark.kyuubi.shutdown.watchdog.timeout=5s" \
496496
--conf "spark.plugins=org.apache.spark.kyuubi.shutdown.watchdog.SparkShutdownWatchdogPlugin" \
497-
--jars target/kyuubi-spark-shutdown-watchdog_2.12-1.11.0-SNAPSHOT.jar \
497+
--jars target/kyuubi-spark-shutdown-watchdog-1.11.0-SNAPSHOT.jar \
498498
. \
499499
TestHangingShutdown 2>&1 | tee test-results.log
500500

@@ -511,9 +511,3 @@ grep "hanging-thread" test-results.log
511511
| 3.0+ | 2.12 | 8+ | ✅ Compatible |
512512
| 3.5+ | 2.12 | 11+ | ✅ Tested |
513513
| 4.0+ | 2.13 | 11+ | ✅ Tested |
514-
515-
## References
516-
517-
- [Spark Plugin API Documentation](https://spark.apache.org/docs/latest/plugins.html)
518-
- [Spark Configuration Guide](https://spark.apache.org/docs/latest/configuration.html)
519-
- [Kyuubi Project](https://kyuubi.apache.org/)

extensions/spark/kyuubi-spark-shutdown-watchdog/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<relativePath>../../../pom.xml</relativePath>
2626
</parent>
2727

28-
<artifactId>kyuubi-spark-shutdown-watchdog_${scala.binary.version}</artifactId>
28+
<artifactId>kyuubi-spark-shutdown-watchdog</artifactId>
2929
<packaging>jar</packaging>
3030
<name>Kyuubi Spark Shutdown Watchdog</name>
3131
<url>https://kyuubi.apache.org/</url>

extensions/spark/kyuubi-spark-shutdown-watchdog/src/main/java/org/apache/spark/kyuubi/shutdown/watchdog/ShutdownWatchdog.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ static void startIfNeeded(SparkConf sparkConf, Logger logger) {
7070
}
7171

7272
final Thread watchdogThread =
73-
new Thread(() -> runWatchdogLoop(timeoutMillis, logger), "spark-shutdown-watchdog");
73+
new Thread(() -> runWatchdogLoop(timeoutMillis, logger), "shutdown-watchdog");
7474
watchdogThread.setDaemon(true);
7575

7676
if (!WATCHDOG_THREAD_REF.compareAndSet(existing, watchdogThread)) {
@@ -127,6 +127,7 @@ private static void runWatchdogLoop(long timeoutMillis, Logger logger) {
127127
}
128128

129129
/** Test-only helper to check whether the watchdog thread is currently running. */
130+
@VisibleForTesting
130131
static boolean isRunningForTests() {
131132
Thread t = WATCHDOG_THREAD_REF.get();
132133
return t != null && t.isAlive();

extensions/spark/kyuubi-spark-shutdown-watchdog/src/main/java/org/apache/spark/kyuubi/shutdown/watchdog/SparkShutdownWatchdogConf.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ final class SparkShutdownWatchdogConf {
3030
static final boolean SHUTDOWN_WATCHDOG_ENABLED_DEFAULT = true;
3131

3232
static final String SHUTDOWN_WATCHDOG_TIMEOUT_KEY = "spark.kyuubi.shutdown.watchdog.timeout";
33-
private static final String SHUTDOWN_WATCHDOG_TIMEOUT_FALLBACK = "0ms";
33+
private static final String SHUTDOWN_WATCHDOG_TIMEOUT_DEFAULT = "0ms";
3434

3535
private SparkShutdownWatchdogConf() {}
3636

@@ -39,6 +39,6 @@ static boolean isEnabled(SparkConf conf) {
3939
}
4040

4141
static long getTimeoutMillis(SparkConf conf) {
42-
return conf.getTimeAsMs(SHUTDOWN_WATCHDOG_TIMEOUT_KEY, SHUTDOWN_WATCHDOG_TIMEOUT_FALLBACK);
42+
return conf.getTimeAsMs(SHUTDOWN_WATCHDOG_TIMEOUT_KEY, SHUTDOWN_WATCHDOG_TIMEOUT_DEFAULT);
4343
}
4444
}

extensions/spark/kyuubi-spark-shutdown-watchdog/src/main/java/org/apache/spark/kyuubi/shutdown/watchdog/SparkShutdownWatchdogPlugin.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,6 @@ public void shutdown() {
5858

5959
@Override
6060
public ExecutorPlugin executorPlugin() {
61-
return new ExecutorPlugin() {
62-
@Override
63-
public void init(PluginContext context, Map<String, String> extraConf) {
64-
// no-op
65-
}
66-
67-
@Override
68-
public void shutdown() {
69-
// no-op
70-
}
71-
};
61+
return null;
7262
}
7363
}

extensions/spark/kyuubi-spark-shutdown-watchdog/src/test/scala/org/apache/spark/kyuubi/shutdown/watchdog/ShutdownWatchdogSuite.scala

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@ package org.apache.spark.kyuubi.shutdown.watchdog
1919

2020
import java.util.concurrent.{CountDownLatch, TimeUnit}
2121
import java.util.concurrent.atomic.AtomicInteger
22-
import java.util.function.IntConsumer
2322

24-
import org.apache.spark.SparkConf
25-
import org.apache.spark.SparkFunSuite
23+
import org.apache.spark.{SparkConf, SparkFunSuite}
2624
import org.scalatest.BeforeAndAfterEach
2725
import org.scalatest.concurrent.Eventually
2826
import org.scalatest.concurrent.PatienceConfiguration.Timeout
@@ -47,7 +45,7 @@ class ShutdownWatchdogSuite
4745
.set(SparkShutdownWatchdogConf.SHUTDOWN_WATCHDOG_TIMEOUT_KEY, "1000ms")
4846

4947
ShutdownWatchdog.startIfNeeded(conf, logger)
50-
assert(!ShutdownWatchdog.isRunningForTests())
48+
assert(!ShutdownWatchdog.isRunningForTests)
5149
}
5250

5351
test("watchdog does not start when timeout is non-positive") {
@@ -56,7 +54,7 @@ class ShutdownWatchdogSuite
5654
.set(SparkShutdownWatchdogConf.SHUTDOWN_WATCHDOG_TIMEOUT_KEY, "0ms")
5755

5856
ShutdownWatchdog.startIfNeeded(conf, logger)
59-
assert(!ShutdownWatchdog.isRunningForTests())
57+
assert(!ShutdownWatchdog.isRunningForTests)
6058
}
6159

6260
test("watchdog triggers emergency exit after timeout") {
@@ -67,11 +65,9 @@ class ShutdownWatchdogSuite
6765
val exitCode = new AtomicInteger(-1)
6866
val exitLatch = new CountDownLatch(1)
6967

70-
ShutdownWatchdog.setExitFn(new IntConsumer {
71-
override def accept(value: Int): Unit = {
72-
exitCode.set(value)
73-
exitLatch.countDown()
74-
}
68+
ShutdownWatchdog.setExitFn((value: Int) => {
69+
exitCode.set(value)
70+
exitLatch.countDown()
7571
})
7672

7773
ShutdownWatchdog.startIfNeeded(conf, logger)
@@ -81,7 +77,7 @@ class ShutdownWatchdogSuite
8177

8278
// Ensure the watchdog thread cleaned itself up.
8379
eventually(Timeout(Span(2, Seconds))) {
84-
assert(!ShutdownWatchdog.isRunningForTests())
80+
assert(!ShutdownWatchdog.isRunningForTests)
8581
}
8682
}
8783
}

0 commit comments

Comments
 (0)