This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 325446cabb5 Fix non-daemon threads blocking JVM shutdown in
RenewableTlsUtils (#17221)
325446cabb5 is described below
commit 325446cabb5c2cca2ecbcbd9893913eebd66720f
Author: 2dmurali <[email protected]>
AuthorDate: Wed Nov 19 15:39:00 2025 -0800
Fix non-daemon threads blocking JVM shutdown in RenewableTlsUtils (#17221)
Co-authored-by: Murali D <[email protected]>
---
.../apache/pinot/common/utils/NamedThreadFactory.java | 16 ++++++++++++++--
.../apache/pinot/common/utils/tls/RenewableTlsUtils.java | 11 ++++++++---
.../pinot/common/utils/tls/RenewableTlsUtilsTest.java | 5 ++++-
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/NamedThreadFactory.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/NamedThreadFactory.java
index 4c9fd7e7df8..c579dcfe170 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/NamedThreadFactory.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/NamedThreadFactory.java
@@ -53,6 +53,7 @@ public class NamedThreadFactory implements ThreadFactory {
private final ThreadGroup _group;
private final AtomicInteger _threadNumber = new AtomicInteger(1);
private final String _threadNamePrefix;
+ private final boolean _daemon;
/**
* Creates a new {@link NamedThreadFactory} instance
@@ -60,10 +61,21 @@ public class NamedThreadFactory implements ThreadFactory {
* @param threadNamePrefix the name prefix assigned to each thread created.
*/
public NamedThreadFactory(String threadNamePrefix) {
+ this(threadNamePrefix, false);
+ }
+
+ /**
+ * Creates a new {@link NamedThreadFactory} instance
+ *
+ * @param threadNamePrefix the name prefix assigned to each thread created.
+ * @param daemon if true, creates daemon threads
+ */
+ public NamedThreadFactory(String threadNamePrefix, boolean daemon) {
final SecurityManager s = System.getSecurityManager();
_group = (s != null) ? s.getThreadGroup() :
Thread.currentThread().getThreadGroup();
_threadNamePrefix =
- String.format(NAME_PATTERN, checkPrefix(threadNamePrefix),
THREAD_POOL_NUMBER.getAndIncrement());
+ String.format(NAME_PATTERN, checkPrefix(threadNamePrefix),
THREAD_POOL_NUMBER.getAndIncrement());
+ _daemon = daemon;
}
private static String checkPrefix(String prefix) {
@@ -78,7 +90,7 @@ public class NamedThreadFactory implements ThreadFactory {
public Thread newThread(Runnable r) {
final Thread t =
new Thread(_group, r, String.format("%s-%d", _threadNamePrefix,
_threadNumber.getAndIncrement()), 0);
- t.setDaemon(false);
+ t.setDaemon(_daemon);
t.setPriority(Thread.NORM_PRIORITY);
return t;
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java
index 32903c8471a..47966aec0eb 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/tls/RenewableTlsUtils.java
@@ -43,6 +43,7 @@ import
nl.altindag.ssl.keymanager.HotSwappableX509ExtendedKeyManager;
import nl.altindag.ssl.trustmanager.HotSwappableX509ExtendedTrustManager;
import nl.altindag.ssl.util.SSLFactoryUtils;
import org.apache.pinot.common.config.TlsConfig;
+import org.apache.pinot.common.utils.NamedThreadFactory;
import org.apache.pinot.spi.utils.retry.RetryPolicies;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -55,6 +56,8 @@ public class RenewableTlsUtils {
private static final String FILE_SCHEME = "file";
private static final int CERT_RELOAD_JOB_INTERVAL_IN_MINUTES = 1440;
private static final int CERT_RELOAD_JOB_INITAL_DELAY_IN_MINUTES = 20;
+ private static final String SSL_FILE_WATCHER_THREAD_PREFIX =
"ssl-file-watcher";
+ private static final String SSL_DAILY_RELOAD_THREAD_PREFIX =
"ssl-daily-reload";
private RenewableTlsUtils() {
// left blank
@@ -209,7 +212,8 @@ public class RenewableTlsUtils {
// The reloadSslFactoryWhenFileStoreChanges is a blocking call, so we
need to create a new thread to run it.
// Creating a new thread to run the reloadSslFactoryWhenFileStoreChanges
is costly; however, unless we
// invoke the createAutoRenewedSSLFactoryFromFileStore method crazily,
this should not be a problem.
- Executors.newSingleThreadExecutor().execute(() -> {
+ // Use daemon thread to allow JVM to exit when on-demand processes
complete.
+ Executors.newSingleThreadExecutor(new
NamedThreadFactory(SSL_FILE_WATCHER_THREAD_PREFIX, true)).execute(() -> {
try {
reloadSslFactoryWhenFileStoreChanges(sslFactory,
keyStoreType, keyStorePath, keyStorePassword,
@@ -227,8 +231,9 @@ public class RenewableTlsUtils {
// it was never detected and run in others. This will result in few
components with
// stale certificates. In order to prevent this issue, we are adding a
new scheduled
// thread which will run once a day and reload the certs.
-
- Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> {
+ // Use daemon thread to allow JVM to exit when on-demand processes
complete.
+ Executors.newSingleThreadScheduledExecutor(new
NamedThreadFactory(SSL_DAILY_RELOAD_THREAD_PREFIX, true))
+ .scheduleAtFixedRate(() -> {
LOGGER.info("Creating a scheduled thread to reloadSsl once a day");
try {
reloadSslFactory(sslFactory,
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/tls/RenewableTlsUtilsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/tls/RenewableTlsUtilsTest.java
index 3225dced401..c39a206e0ab 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/tls/RenewableTlsUtilsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/tls/RenewableTlsUtilsTest.java
@@ -54,6 +54,7 @@ import
nl.altindag.ssl.trustmanager.HotSwappableX509ExtendedTrustManager;
import nl.altindag.ssl.trustmanager.UnsafeX509ExtendedTrustManager;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.common.config.TlsConfig;
+import org.apache.pinot.common.utils.NamedThreadFactory;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -84,6 +85,7 @@ public class RenewableTlsUtilsTest {
private static final String TLS_KEYSTORE_FILE_PATH = DEFAULT_TEST_TLS_DIR +
"/" + TLS_KEYSTORE_FILE;
private static final String TLS_TRUSTSTORE_FILE_PATH = DEFAULT_TEST_TLS_DIR
+ "/" + TLS_TRUSTSTORE_FILE;
+ private static final String SSL_RELOAD_THREAD_PREFIX = "ssl-reload-test";
@BeforeMethod
public void setUp()
@@ -183,7 +185,8 @@ public class RenewableTlsUtilsTest {
// Start a new thread to reload the ssl factory when the tls files change
// Avoid early finalization by not using Executors.newSingleThreadExecutor
(java <= 20, JDK-8145304)
- ExecutorService executorService = Executors.newFixedThreadPool(1);
+ ExecutorService executorService = Executors.newFixedThreadPool(1,
+ new NamedThreadFactory(SSL_RELOAD_THREAD_PREFIX, true));
executorService.execute(
() -> {
try {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
