This is an automated email from the ASF dual-hosted git repository.
sigram pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 7391b7d53d4 SOLR-18564: CoreContainer.shutdown: run the remaining
closes when the client close pool times out (#5192)
7391b7d53d4 is described below
commit 7391b7d53d416dd06f5ca15a1dd712b32743fc93
Author: Nick <[email protected]>
AuthorDate: Mon Nov 2 06:28:08 2026 -0700
SOLR-18564: CoreContainer.shutdown: run the remaining closes when the
client close pool times out (#5192)
---
changelog/unreleased/SOLR-18564.yml | 7 +
.../java/org/apache/solr/core/CoreContainer.java | 13 +-
.../solr/core/CoreContainerShutdownTest.java | 163 +++++++++++++++++++++
.../org/apache/solr/common/util/ExecutorUtil.java | 17 ++-
.../apache/solr/common/util/ExecutorUtilTest.java | 21 ++-
5 files changed, 212 insertions(+), 9 deletions(-)
diff --git a/changelog/unreleased/SOLR-18564.yml
b/changelog/unreleased/SOLR-18564.yml
new file mode 100644
index 00000000000..545febcbec1
--- /dev/null
+++ b/changelog/unreleased/SOLR-18564.yml
@@ -0,0 +1,7 @@
+title: "CoreContainer.shutdown: a client close pool timeout skips the
remaining closes and leaks metrics contexts"
+type: fixed
+authors:
+ - name: Nick Shanin
+links:
+ - name: SOLR-18564
+ url: https://issues.apache.org/jira/browse/SOLR-18564
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 5b73171eacc..f5d62750d2a 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -93,6 +93,7 @@ import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.CollectionUtil;
import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.ExecutorUtil;
+import
org.apache.solr.common.util.ExecutorUtil.ExecutorTerminationTimeoutException;
import org.apache.solr.common.util.IOUtils;
import org.apache.solr.common.util.ObjectCache;
import org.apache.solr.common.util.SolrNamedThreadFactory;
@@ -1312,7 +1313,17 @@ public class CoreContainer {
// we want to close zk stuff last
zkSys.close();
} finally {
- ExecutorUtil.shutdownAndAwaitTermination(customThreadPool);
+ try {
+ ExecutorUtil.shutdownAndAwaitTermination(customThreadPool);
+ } catch (ExecutorTerminationTimeoutException e) {
+ // A close task that outlives the pool's termination budget must
not abort the
+ // rest of shutdown. The closes after this block (the plugin
closes, the
+ // container handlers and the root metrics context) are the only
release path
+ // for the container's metrics contexts, so skipping them leaks
the whole
+ // metrics tree of this container.
+ log.warn(
+ "The closeThreadPool did not terminate; continuing
CoreContainer shutdown.", e);
+ }
}
}
}
diff --git
a/solr/core/src/test/org/apache/solr/core/CoreContainerShutdownTest.java
b/solr/core/src/test/org/apache/solr/core/CoreContainerShutdownTest.java
new file mode 100644
index 00000000000..ea14a6b2ad9
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/core/CoreContainerShutdownTest.java
@@ -0,0 +1,163 @@
+/*
+ * 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.solr.core;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.ObjectReleaseTracker;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.handler.component.HttpShardHandlerFactory;
+import org.apache.solr.handler.component.ShardHandlerFactory;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.security.AuthorizationContext;
+import org.apache.solr.security.PermissionNameProvider;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Shutdown must run its trailing closes even when a close task outlives the
closeThreadPool's
+ * termination budget and the pool wait throws. Those closes are the only
release path for the
+ * container's metrics contexts; skipping them leaks the whole metrics tree,
and in a test JVM the
+ * leaked contexts are then charged to an unrelated suite by the object
release tracker.
+ */
+public class CoreContainerShutdownTest extends SolrTestCase {
+
+ private static final String SOLR_XML =
+ "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
+ + "<solr>\n"
+ + "<str name=\"configSetBaseDir\">${configsets:configsets}</str>\n"
+ + "</solr>";
+
+ @BeforeClass
+ public static void beforeClass() {
+ System.setProperty(
+ "configsets",
SolrTestCaseJ4.getFile("solr/configsets").toAbsolutePath().toString());
+ }
+
+ @Test
+ public void testShutdownRunsTrailingClosesWhenClosePoolTimesOut() throws
Exception {
+ Path solrHome = createTempDir();
+ CoreContainer cc = new CoreContainer(SolrXmlConfig.fromString(solrHome,
SOLR_XML));
+ cc.load();
+ BlockingShardHandlerFactory blockingFactory = null;
+ try {
+ assertTrue(
+ "the root metrics context is tracked after load",
+
ObjectReleaseTracker.getTrackedObjectsOfType(SolrMetricsContext.class)
+ .contains(cc.solrMetricsContext));
+
+ CloseTrackingHandler handler = new CloseTrackingHandler();
+ cc.getRequestHandlers().put("/close-tracking", handler);
+
+ // Make one close task outlive the pool's whole termination budget (both
60 second
+ // awaits), so the pool wait throws the way it did in the incident this
test pins down.
+ // This makes the test take about two minutes; the wait is the
production timeout,
+ // not a test sleep.
+ blockingFactory = new
BlockingShardHandlerFactory(cc.shardHandlerFactory);
+ cc.shardHandlerFactory = blockingFactory;
+
+ cc.shutdown();
+
+ assertTrue(
+ "container handlers must be closed even when the close pool times
out",
+ handler.closed.get());
+ assertFalse(
+ "the root metrics context must be released even when the close pool
times out",
+
ObjectReleaseTracker.getTrackedObjectsOfType(SolrMetricsContext.class)
+ .contains(cc.solrMetricsContext));
+ } finally {
+ if (blockingFactory != null) {
+ blockingFactory.releaseClose();
+ // Wait for the blocked close task to finish so the delegate factory
is closed
+ // before the framework's post-class object release check runs.
+ assertTrue(blockingFactory.awaitCloseFinished(30, TimeUnit.SECONDS));
+ }
+ }
+ }
+
+ /** A container handler whose close is observable. */
+ private static final class CloseTrackingHandler extends RequestHandlerBase {
+ final AtomicBoolean closed = new AtomicBoolean();
+
+ @Override
+ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
{}
+
+ @Override
+ public String getDescription() {
+ return "test handler that records its close";
+ }
+
+ @Override
+ public PermissionNameProvider.Name getPermissionName(AuthorizationContext
request) {
+ return PermissionNameProvider.Name.ALL;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closed.set(true);
+ super.close();
+ }
+ }
+
+ /**
+ * Wraps the real shard handler factory with a close that blocks until
released, surviving the
+ * pool's force-stop interrupt the way a stuck Jetty client close does, and
only then delegates.
+ */
+ private static final class BlockingShardHandlerFactory extends
HttpShardHandlerFactory {
+ private final ShardHandlerFactory delegate;
+ private final CountDownLatch release = new CountDownLatch(1);
+ private final CountDownLatch closeFinished = new CountDownLatch(1);
+
+ BlockingShardHandlerFactory(ShardHandlerFactory delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void close() {
+ try {
+ boolean released = false;
+ while (!released) {
+ try {
+ release.await();
+ released = true;
+ } catch (InterruptedException e) {
+ // Keep blocking: the pool's force-stop interrupts its workers,
and a close stuck
+ // inside the client shutdown survives that interrupt without
finishing.
+ }
+ }
+ delegate.close();
+ } finally {
+ closeFinished.countDown();
+ }
+ }
+
+ void releaseClose() {
+ release.countDown();
+ }
+
+ boolean awaitCloseFinished(long timeout, TimeUnit unit) throws
InterruptedException {
+ return closeFinished.await(timeout, unit);
+ }
+ }
+}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
index e4814f7b094..adf685cfd91 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
@@ -112,6 +112,7 @@ public class ExecutorUtil {
* detail on the waiting can be found in {@link
#awaitTermination(ExecutorService)}.
*
* @param pool The ExecutorService to shut down and wait on
+ * @throws ExecutorTerminationTimeoutException if the pool does not
terminate after both waits
*/
public static void shutdownAndAwaitTermination(ExecutorService pool) {
if (pool == null) return;
@@ -119,6 +120,11 @@ public class ExecutorUtil {
awaitTermination(pool);
}
+ /**
+ * Interrupt the pool's tasks and wait for termination.
+ *
+ * @throws ExecutorTerminationTimeoutException if the pool does not
terminate after both waits
+ */
public static void shutdownNowAndAwaitTermination(ExecutorService pool) {
if (pool == null) return;
pool.shutdownNow(); // Disable new tasks from being submitted; interrupt
existing tasks
@@ -130,6 +136,7 @@ public class ExecutorUtil {
* shutdown the remaining threads and wait another 60 seconds.
*
* @param pool the ExecutorService to wait on
+ * @throws ExecutorTerminationTimeoutException if the pool does not
terminate after both waits
*/
public static void awaitTermination(ExecutorService pool) {
awaitTermination(pool, 60, TimeUnit.SECONDS);
@@ -146,7 +153,8 @@ public class ExecutorUtil {
if (!pool.awaitTermination(timeout, unit)) {
String executorDetails = describeExecutorForLogging(pool);
log.error("Threads from pool did not forcefully stop. {}",
executorDetails);
- throw new RuntimeException("Timeout waiting for pool to shutdown. "
+ executorDetails);
+ throw new ExecutorTerminationTimeoutException(
+ "Timeout waiting for pool to shutdown. " + executorDetails);
}
}
} catch (InterruptedException ie) {
@@ -157,6 +165,13 @@ public class ExecutorUtil {
}
}
+ /** The executor did not terminate within either the graceful or forced
shutdown wait. */
+ public static final class ExecutorTerminationTimeoutException extends
RuntimeException {
+ private ExecutorTerminationTimeoutException(String message) {
+ super(message);
+ }
+ }
+
/** Executor logging details which include pool name when executor fails to
terminate. */
public static String describeExecutorForLogging(ExecutorService pool) {
if (pool == null) return "";
diff --git
a/solr/solrj/src/test/org/apache/solr/common/util/ExecutorUtilTest.java
b/solr/solrj/src/test/org/apache/solr/common/util/ExecutorUtilTest.java
index ed739632e84..6fcac5df494 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/ExecutorUtilTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/util/ExecutorUtilTest.java
@@ -30,6 +30,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.lucene.util.NamedThreadFactory;
import org.apache.solr.SolrTestCase;
+import
org.apache.solr.common.util.ExecutorUtil.ExecutorTerminationTimeoutException;
import org.apache.solr.logging.MDCLoggingContext;
import org.apache.solr.util.TimeOut;
import org.junit.Test;
@@ -52,9 +53,9 @@ public class ExecutorUtilTest extends SolrTestCase {
public void testExecutorUtilAwaitsTerminationWhenTaskIgnoresInterupt()
throws Exception {
final Worker w = new Worker(false);
+ final String poolName = this.getClass().getSimpleName() +
"non-interruptable";
final ExecutorService executorService =
- ExecutorUtil.newMDCAwareSingleThreadExecutor(
- new SolrNamedThreadFactory(this.getClass().getSimpleName() +
"non-interruptable"));
+ ExecutorUtil.newMDCAwareSingleThreadExecutor(new
SolrNamedThreadFactory(poolName));
try {
final Future<Boolean> f = executorService.submit(w);
@@ -63,11 +64,17 @@ public class ExecutorUtilTest extends SolrTestCase {
assertTrue(
"Worker not interupted by shutdown in a sane amount of time",
w.awaitWorkerInteruptedAtLeastOnce());
- assertThrows(
- RuntimeException.class,
- () ->
- ExecutorUtil.awaitTermination(
- executorService, MAX_AWAIT_TERMINATION_ARG_MS,
TimeUnit.MILLISECONDS));
+ RuntimeException timeout =
+ assertThrows(
+ ExecutorTerminationTimeoutException.class,
+ () ->
+ ExecutorUtil.awaitTermination(
+ executorService, MAX_AWAIT_TERMINATION_ARG_MS,
TimeUnit.MILLISECONDS));
+ assertTrue(
+ timeout.getMessage(),
+ timeout.getMessage().startsWith("Timeout waiting for pool to
shutdown. "));
+ assertTrue(
+ timeout.getMessage(), timeout.getMessage().contains("[poolName=" +
poolName + "]"));
assertFalse("Worker should not finish due to shutdown or
awaitTermination", f.isDone());
assertTrue("test sanity check: WTF? how did we get here?",
w.getNumberOfInterupts() > 0);
