This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 53a2a36df2f SOLR-17118: Simplify/fix CoreContainerProvider
initialization (#2474)
53a2a36df2f is described below
commit 53a2a36df2fb843df640ce51ead4a6e0f65d8e74
Author: David Smiley <[email protected]>
AuthorDate: Sat Jun 1 04:04:21 2024 +0100
SOLR-17118: Simplify/fix CoreContainerProvider initialization (#2474)
Thereby fixing a rare occurrence of Solr hanging on startup.
CoreContainerProvider: Don't need any CountDownLatch (x2), the
synchronized WeakHashMap of "services", the ServiceHolder, the
ContextInitializationKey. No looping to wait for initialization.
JettySolrRunner: incorporate the CoreContainerProvider and various servlet
filters in a normal way -- add all this before starting, not after. Thus Jetty
will shut them down properly so we don't have to. Removed some needless
synchronized wait/notify and other needless stuff.
HealthCheckHandlerTest was shutting down CoreContainer improperly; this
subset of the test was removed.
(cherry picked from commit 1177796e32631c62d8f00e7df4341c92b75e1617)
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/core/CoreContainer.java | 1 +
.../apache/solr/servlet/CoreContainerProvider.java | 173 +++++----------------
.../apache/solr/servlet/SolrDispatchFilter.java | 27 +---
.../test/org/apache/solr/cloud/ZkFailoverTest.java | 2 +-
.../solr/handler/admin/HealthCheckHandlerTest.java | 30 ----
.../org/apache/solr/embedded/JettySolrRunner.java | 129 +++++----------
7 files changed, 85 insertions(+), 279 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index ac68379d1be..a6489e78012 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -85,6 +85,8 @@ Bug Fixes
* SOLR-17255: Fix bugs in SolrParams.toLocalParamsString() (hossman)
+* SOLR-17118: Simplify CoreContainerProvider initialization and
JettySolrRunner. Avoid deadlock/hang. (David Smiley)
+
Dependency Upgrades
---------------------
(No changes)
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 b9a13cb19d9..0285b56e5d9 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1259,6 +1259,7 @@ public class CoreContainer {
return isShutDown;
}
+ /** Close / shut down. Only called by {@link
org.apache.solr.servlet.CoreContainerProvider}. */
public void shutdown() {
ZkController zkController = getZkController();
diff --git
a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
index 6cd261bc0f2..8b8bc3c927d 100644
--- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
+++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
@@ -36,16 +36,10 @@ import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Instant;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Locale;
-import java.util.Map;
-import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
-import java.util.WeakHashMap;
-import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
-import java.util.function.Supplier;
import java.util.stream.Stream;
import javax.naming.Context;
import javax.naming.InitialContext;
@@ -93,72 +87,63 @@ public class CoreContainerProvider implements
ServletContextListener {
private HttpClient httpClient;
private SolrMetricManager metricManager;
private RateLimitManager rateLimitManager;
- private final CountDownLatch init = new CountDownLatch(1);
private String registryName;
- // AFAIK the only reason we need this is to support JettySolrRunner for
tests. In tests we might
- // have multiple CoreContainers in the same JVM, but I *think* that doesn't
happen in a real
- // server.
- private static final Map<ContextInitializationKey, ServiceHolder> services =
- Collections.synchronizedMap(new WeakHashMap<>());
-
- // todo: dependency injection instead, but for now this method and the
associated map will have
- // to suffice.
- // Note that this relies on ServletContext.equals() not implementing
anything significantly
- // different than Object.equals for its .equals method (I've found no
implementation that even
- // implements it).
- public static ServiceHolder serviceForContext(ServletContext ctx) throws
InterruptedException {
- ContextInitializationKey key = new ContextInitializationKey(ctx);
- return services.computeIfAbsent(key, ServiceHolder::new);
+
+ /**
+ * Acquires an instance from the context. Never null.
+ *
+ * @throws IllegalStateException if not present.
+ */
+ public static CoreContainerProvider serviceForContext(ServletContext ctx) {
+ var provider = (CoreContainerProvider)
ctx.getAttribute(CoreContainerProvider.class.getName());
+ if (provider == null) {
+ throw new IllegalStateException("CoreContainer failed to initialize");
+ }
+ return provider;
}
@Override
- public void contextInitialized(ServletContextEvent sce) {
- init(sce.getServletContext());
+ public void contextInitialized(ServletContextEvent event) {
+ final var ctx = event.getServletContext();
+ init(ctx);
+ ctx.setAttribute(CoreContainerProvider.class.getName(), this);
}
@Override
public void contextDestroyed(ServletContextEvent sce) {
close();
+ // could remove ourselves from ctx but why bother
}
+ /**
+ * @see SolrDispatchFilter#getCores()
+ */
CoreContainer getCoreContainer() throws UnavailableException {
- waitForCoreContainer(() -> cores, init);
+ checkReady();
return cores;
}
+ /**
+ * @see SolrDispatchFilter#getHttpClient()
+ */
HttpClient getHttpClient() throws UnavailableException {
- waitForCoreContainer(() -> cores, init);
+ checkReady();
return httpClient;
}
- private static void waitForCoreContainer(Supplier<CoreContainer> provider,
CountDownLatch latch)
- throws UnavailableException {
- CoreContainer cores = provider.get();
- if (cores == null || cores.isShutDown()) {
- long startWait = System.nanoTime();
- try {
- while (!latch.await(10, TimeUnit.SECONDS)) {
- long now = System.nanoTime();
- if (log.isInfoEnabled()) {
- log.info(
- "Still waiting for CoreContainerStartup ({} seconds elapsed)",
- (now - startWait) / 1_000_000_000);
- }
- }
- } catch (InterruptedException e) { // well, no wait then
- Thread.currentThread().interrupt();
- }
- cores = provider.get();
- if (cores == null || cores.isShutDown()) {
- final String msg =
- "Error processing the request. CoreContainer is either not
initialized or shutting down.";
- log.error(msg);
- throw new UnavailableException(msg);
- }
+ private void checkReady() throws UnavailableException {
+ // TODO throw AlreadyClosedException instead?
+ if (cores == null) {
+ // cores could be null if it didn't start properly or if it's completely
shut down.
+ // It appears impossible that it'd be null if it didn't even try to
start yet.
+ final String msg = "Error processing the request. CoreContainer has shut
down.";
+ log.error(msg);
+ throw new UnavailableException(msg);
}
+ assert !cores.isShutDown(); // shutdown sequence initiates *here*, thus
will be nulled first
}
- public void close() {
+ private void close() {
CoreContainer cc = cores;
// Mark Miller suggested that we should be publishing that we are down
before anything else
@@ -197,9 +182,9 @@ public class CoreContainerProvider implements
ServletContextListener {
}
}
- public void init(ServletContext servletContext) {
+ private void init(ServletContext servletContext) {
if (log.isTraceEnabled()) {
- log.trace("CoreService.init(): {}", this.getClass().getClassLoader());
+ log.trace("init(): {}", this.getClass().getClassLoader());
}
CoreContainer coresInit = null;
try {
@@ -272,12 +257,8 @@ public class CoreContainerProvider implements
ServletContextListener {
throw (Error) t;
}
} finally {
- log.trace("SolrDispatchFilter.init() done");
+ log.trace("init() done");
this.cores = coresInit; // crucially final assignment
- services
- .computeIfAbsent(new ContextInitializationKey(servletContext),
ServiceHolder::new)
- .setService(this);
- init.countDown();
}
}
@@ -496,82 +477,4 @@ public class CoreContainerProvider implements
ServletContextListener {
void setRateLimitManager(RateLimitManager rateLimitManager) {
this.rateLimitManager = rateLimitManager;
}
-
- private static class ContextInitializationKey {
- private final ServletContext ctx;
- private final CountDownLatch initializing = new CountDownLatch(1);
-
- private ContextInitializationKey(ServletContext ctx) {
- if (ctx == null) {
- throw new IllegalArgumentException("Context must not be null");
- }
- // if one of these is reachable both must be to avoid collection from
weak hashmap, so
- // set an attribute holding this object to ensure we never get collected
until the
- // ServletContext is eligible for collection too.
- ctx.setAttribute(this.getClass().getName(), this);
- this.ctx = ctx;
- }
-
- public synchronized ServletContext getCtx() {
- return ctx;
- }
-
- synchronized void makeReady() {
- this.initializing.countDown();
- }
-
- // NOT synchronized :)
- public void waitForReadyService() throws InterruptedException {
- initializing.await();
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o) return true;
- if (!(o instanceof ContextInitializationKey)) return false;
- ContextInitializationKey that = (ContextInitializationKey) o;
- return ctx.equals(that.ctx);
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(ctx);
- }
- }
-
- static class ServiceHolder {
- private volatile CoreContainerProvider service;
- private volatile ContextInitializationKey key;
-
- private ServiceHolder(ContextInitializationKey key) {
- if (key == null) {
- throw new IllegalArgumentException(
- "Key for accessing this service holder must be supplied");
- }
- this.key = key;
- }
-
- public void setService(CoreContainerProvider service) {
- this.service = service;
- key.makeReady();
- key = null; // be sure not to hold a reference to the context via the key
- }
-
- public CoreContainerProvider getService() {
- try {
- if (key != null) {
- try {
- key.waitForReadyService();
- } catch (NullPointerException e) {
- // ignore, means we raced with set service and lost, but that's
fine since null implies
- // we are ready.
- }
- }
- } catch (InterruptedException e) {
- throw new SolrException(
- ErrorCode.SERVER_ERROR, "Interrupted while obtaining reference to
CoreService");
- }
- return service;
- }
- }
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index f1b1513ce51..bd408f886cf 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -57,7 +57,6 @@ import org.apache.solr.security.AuditEvent;
import org.apache.solr.security.AuthenticationPlugin;
import org.apache.solr.security.PKIAuthenticationPlugin;
import org.apache.solr.security.PublicKeyHandler;
-import org.apache.solr.servlet.CoreContainerProvider.ServiceHolder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -76,10 +75,7 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
public static final String ATTR_TRACING_SPAN = Span.class.getName();
public static final String ATTR_TRACING_TRACER = Tracer.class.getName();
- // TODO: see if we can get rid of the holder here (Servlet spec actually
guarantees
- // ContextListeners run before filter init, but JettySolrRunner that we use
for tests is
- // complicated)
- private ServiceHolder coreService;
+ private CoreContainerProvider containerProvider;
protected final CountDownLatch init = new CountDownLatch(1);
@@ -98,7 +94,7 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
public HttpClient getHttpClient() {
try {
- return coreService.getService().getHttpClient();
+ return containerProvider.getHttpClient();
} catch (UnavailableException e) {
throw new SolrException(
ErrorCode.SERVER_ERROR, "Internal Http Client Unavailable, startup
may have failed");
@@ -140,14 +136,9 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
@Override
public void init(FilterConfig config) throws ServletException {
try {
- coreService =
CoreContainerProvider.serviceForContext(config.getServletContext());
+ containerProvider =
CoreContainerProvider.serviceForContext(config.getServletContext());
boolean isCoordinator =
- NodeRoles.MODE_ON.equals(
- coreService
- .getService()
- .getCoreContainer()
- .nodeRoles
- .getRoleMode(NodeRoles.Role.COORDINATOR));
+
NodeRoles.MODE_ON.equals(getCores().nodeRoles.getRoleMode(NodeRoles.Role.COORDINATOR));
solrCallFactory =
isCoordinator ? new CoordinatorHttpSolrCall.Factory() : new
HttpSolrCallFactory() {};
if (log.isTraceEnabled()) {
@@ -155,9 +146,6 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
}
configExcludes(this, config.getInitParameter("excludePatterns"));
- } catch (InterruptedException e) {
- throw new ServletException("Interrupted while fetching core service");
-
} catch (Throwable t) {
// catch this so our filter still works
log.error("Could not start Dispatch Filter.", t);
@@ -170,8 +158,9 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
}
}
+ /** The CoreContainer. It's ready for use, albeit could shut down whenever.
Never null. */
public CoreContainer getCores() throws UnavailableException {
- return coreService.getService().getCoreContainer();
+ return containerProvider.getCoreContainer();
}
@Override
@@ -208,7 +197,7 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
}
Tracer t = getCores() == null ? GlobalTracer.get() :
getCores().getTracer();
request.setAttribute(ATTR_TRACING_TRACER, t);
- RateLimitManager rateLimitManager =
coreService.getService().getRateLimitManager();
+ RateLimitManager rateLimitManager =
containerProvider.getRateLimitManager();
try {
ServletUtils.rateLimitRequest(
rateLimitManager,
@@ -412,7 +401,7 @@ public class SolrDispatchFilter extends BaseSolrFilter
implements PathExcluder {
@VisibleForTesting
void replaceRateLimitManager(RateLimitManager rateLimitManager) {
- coreService.getService().setRateLimitManager(rateLimitManager);
+ containerProvider.setRateLimitManager(rateLimitManager);
}
/** internal API */
diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkFailoverTest.java
b/solr/core/src/test/org/apache/solr/cloud/ZkFailoverTest.java
index f4251ad9b6f..d9dceb79614 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ZkFailoverTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ZkFailoverTest.java
@@ -65,7 +65,7 @@ public class ZkFailoverTest extends SolrCloudTestCase {
// This attempt will succeed since there will be enough time to connect
System.setProperty("waitForZk", "20");
restartSolrAndZk();
- waitForLiveNodes(2);
+ waitForLiveNodes(cluster.getJettySolrRunners().size());
waitForState("Timeout waiting for " + coll, coll, clusterShape(2, 2));
QueryResponse rsp =
new QueryRequest(new
SolrQuery("*:*")).process(cluster.getSolrClient(), coll);
diff --git
a/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
b/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
index 74019a02b35..2751cfb9950 100644
---
a/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
+++
b/solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java
@@ -115,36 +115,6 @@ public class HealthCheckHandlerTest extends
SolrCloudTestCase {
newJetty.stop();
}
- // add a new node for the purpose of negative testing
- // negative check that if core container is not available at the node
- newJetty = cluster.startJettySolrRunner();
- try (SolrClient solrClient =
getHttpSolrClient(newJetty.getBaseUrl().toString())) {
-
- // positive check that our (new) "healthy" node works with direct http
client
- assertEquals(CommonParams.OK,
runHealthcheckWithClient(solrClient).getNodeStatus());
-
- // shutdown the core container of new node
- newJetty.getCoreContainer().shutdown();
-
- // api shouldn't unreachable
- SolrException thrown =
- expectThrows(
- SolrException.class,
- () -> {
- runHealthcheckWithClient(solrClient).getNodeStatus();
- fail("API shouldn't be available, and fail at above request");
- });
- assertEquals("Exception code should be 404", 404, thrown.code());
- assertTrue(
- "Should have seen an exception containing the an error",
- thrown
- .getMessage()
- .contains(
- "Error processing the request. CoreContainer is either not
initialized or shutting down."));
- } finally {
- newJetty.stop();
- }
-
// (redundant) positive check that our (previously) exiting "healthy" node
(still) works
// after getting negative results from our broken node and failed core
container
try (SolrClient solrClient =
diff --git
a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
index 55b7a45e285..12aa7134f15 100644
--- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
+++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
@@ -39,7 +39,6 @@ import java.util.Map;
import java.util.Properties;
import java.util.Random;
import java.util.Set;
-import java.util.concurrent.ExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
@@ -47,6 +46,7 @@ import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
+import javax.servlet.ServletContextEvent;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
@@ -59,10 +59,8 @@ import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.cloud.SocketProxy;
import org.apache.solr.client.solrj.embedded.SSLConfig;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
-import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.common.util.TimeSource;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
@@ -93,7 +91,6 @@ import org.eclipse.jetty.servlet.FilterHolder;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.servlet.Source;
-import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.util.thread.ReservedThreadExecutor;
@@ -146,8 +143,6 @@ public class JettySolrRunner {
private volatile boolean started = false;
- private CoreContainerProvider coreContainerProvider;
-
public static class DebugFilter implements Filter {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -376,24 +371,12 @@ public class JettySolrRunner {
new ServletContextHandler(server, config.context,
ServletContextHandler.SESSIONS);
root.setResourceBase(".");
- server.addEventListener(
- new LifeCycle.Listener() {
-
- @Override
- public void lifeCycleStopping(LifeCycle arg0) {}
-
- @Override
- public synchronized void lifeCycleStopped(LifeCycle arg0) {
- if (coreContainerProvider != null) {
- coreContainerProvider.close();
- }
- }
-
+ root.addEventListener(
+ // Install CCP first. Subclass CCP to do some pre-initialization
+ new CoreContainerProvider() {
@Override
- public void lifeCycleStarting(LifeCycle arg0) {}
-
- @Override
- public synchronized void lifeCycleStarted(LifeCycle arg0) {
+ public void contextInitialized(ServletContextEvent event) {
+ // awkwardly, parts of Solr want to know the port but we don't
know that until now
jettyPort = getFirstConnectorPort();
int port = jettyPort;
if (proxyPort != -1) port = proxyPort;
@@ -404,41 +387,31 @@ public class JettySolrRunner {
.setAttribute(SolrDispatchFilter.PROPERTIES_ATTRIBUTE,
nodeProperties);
root.getServletContext()
.setAttribute(SolrDispatchFilter.SOLRHOME_ATTRIBUTE,
solrHome);
+
SSLConfigurationsFactory.current().init(); // normally happens
in jetty-ssl.xml
- coreContainerProvider = new CoreContainerProvider();
- coreContainerProvider.init(root.getServletContext());
+
log.info("Jetty properties: {}", nodeProperties);
- debugFilter =
- root.addFilter(DebugFilter.class, "/*",
EnumSet.of(DispatcherType.REQUEST));
- extraFilters = new ArrayList<>();
- for (Map.Entry<Class<? extends Filter>, String> entry :
- config.extraFilters.entrySet()) {
- extraFilters.add(
- root.addFilter(
- entry.getKey(), entry.getValue(),
EnumSet.of(DispatcherType.REQUEST)));
- }
-
- for (Map.Entry<ServletHolder, String> entry :
config.extraServlets.entrySet()) {
- root.addServlet(entry.getKey(), entry.getValue());
- }
- dispatchFilter =
root.getServletHandler().newFilterHolder(Source.EMBEDDED);
- dispatchFilter.setHeldClass(SolrDispatchFilter.class);
- dispatchFilter.setInitParameter("excludePatterns",
excludePatterns);
- // Map dispatchFilter in same path as in web.xml
- root.addFilter(dispatchFilter, "/*",
EnumSet.of(DispatcherType.REQUEST));
-
- synchronized (JettySolrRunner.this) {
- waitOnSolr = true;
- JettySolrRunner.this.notify();
- }
- }
-
- @Override
- public void lifeCycleFailure(LifeCycle arg0, Throwable arg1) {
- System.clearProperty("hostPort");
+ super.contextInitialized(event);
}
});
+
+ debugFilter = root.addFilter(DebugFilter.class, "/*",
EnumSet.of(DispatcherType.REQUEST));
+ extraFilters = new ArrayList<>();
+ for (Map.Entry<Class<? extends Filter>, String> entry :
config.extraFilters.entrySet()) {
+ extraFilters.add(
+ root.addFilter(entry.getKey(), entry.getValue(),
EnumSet.of(DispatcherType.REQUEST)));
+ }
+
+ for (Map.Entry<ServletHolder, String> entry :
config.extraServlets.entrySet()) {
+ root.addServlet(entry.getKey(), entry.getValue());
+ }
+ dispatchFilter =
root.getServletHandler().newFilterHolder(Source.EMBEDDED);
+ dispatchFilter.setHeldClass(SolrDispatchFilter.class);
+ dispatchFilter.setInitParameter("excludePatterns", excludePatterns);
+ // Map dispatchFilter in same path as in web.xml
+ root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST));
+
// Default servlet as a fall-through
root.addServlet(Servlet404.class, "/");
chain = root;
@@ -455,6 +428,7 @@ public class JettySolrRunner {
rwh.addRule(new RewritePatternRule("/api/*", "/solr/____v2"));
chain = rwh;
}
+
GzipHandler gzipHandler = new GzipHandler();
gzipHandler.setHandler(chain);
@@ -483,15 +457,14 @@ public class JettySolrRunner {
* @return the {@link CoreContainer} for this node
*/
public CoreContainer getCoreContainer() {
+ final var solrDispatchFilter = getSolrDispatchFilter();
+ if (solrDispatchFilter == null) {
+ return null;
+ }
try {
- if (getSolrDispatchFilter() == null ||
getSolrDispatchFilter().getCores() == null) {
- return null;
- }
- return getSolrDispatchFilter().getCores();
+ return solrDispatchFilter.getCores();
} catch (UnavailableException e) {
- // Since this is only used in tests, this is just a straight-up failure
- // If this is converted for other use something else might be better here
- throw new RuntimeException(e);
+ return null;
}
}
@@ -558,15 +531,7 @@ public class JettySolrRunner {
server.start();
}
}
- synchronized (JettySolrRunner.this) {
- int cnt = 0;
- while (!waitOnSolr || !dispatchFilter.isRunning()) {
- this.wait(100);
- if (cnt++ == 15) {
- throw new RuntimeException("Jetty/Solr unresponsive");
- }
- }
- }
+ assert dispatchFilter.isRunning();
if (config.waitForLoadingCoresToFinishMs != null
&& config.waitForLoadingCoresToFinishMs > 0L) {
@@ -663,21 +628,11 @@ public class JettySolrRunner {
Map<String, String> prevContext = MDC.getCopyOfContextMap();
MDC.clear();
try {
- Filter filter = dispatchFilter.getFilter();
QueuedThreadPool qtp = (QueuedThreadPool) server.getThreadPool();
ReservedThreadExecutor rte = qtp.getBean(ReservedThreadExecutor.class);
server.stop();
- if (server.getState().equals(Server.FAILED)) {
- filter.destroy();
- if (extraFilters != null) {
- for (FilterHolder f : extraFilters) {
- f.getFilter().destroy();
- }
- }
- }
-
// stop timeout is 0, so we will interrupt right away
while (!qtp.isStopped()) {
qtp.stop();
@@ -701,12 +656,6 @@ public class JettySolrRunner {
timeout.waitFor("Timeout waiting for reserved executor to stop.",
rte::isStopped);
}
- // we want to shutdown outside of jetty cutting us off
- SolrDispatchFilter sdf = getSolrDispatchFilter();
- if (sdf != null) {
- ExecutorUtil.shutdownAndAwaitTermination(getJettyShutDownThreadPool());
- }
-
do {
try {
server.join();
@@ -728,10 +677,6 @@ public class JettySolrRunner {
}
}
- private ExecutorService getJettyShutDownThreadPool() {
- return ExecutorUtil.newMDCAwareCachedThreadPool(new
SolrNamedThreadFactory("jettyShutDown"));
- }
-
public void outputMetrics(File outputDirectory, String fileName) throws
IOException {
if (getCoreContainer() != null) {
@@ -920,11 +865,7 @@ public class JettySolrRunner {
} catch (UnavailableException e) {
throw new IllegalStateException("The CoreContainer is unavailable!");
}
- if (cores != null) {
- cores.waitForLoadingCoresToFinish(timeoutMs);
- } else {
- throw new IllegalStateException("The CoreContainer is not set!");
- }
+ cores.waitForLoadingCoresToFinish(timeoutMs);
} else {
throw new IllegalStateException("The dispatchFilter is not set!");
}
