Skip to content

Commit b148c40

Browse files
authored
Refactor: remove coreContainerWorkExecutor (#3405)
It's only used for one silly thing: It's needless to create an executor just to asynchronously wait for another executor (coreLoadExecutor) to close. We can instead call shutdownAndAwaitTermination on coreLoadExecutor when the container shuts down.
1 parent df686dd commit b148c40

2 files changed

Lines changed: 8 additions & 24 deletions

File tree

solr/core/src/java/org/apache/solr/core/CoreContainer.java

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,7 @@ public JerseyAppHandlerCache getJerseyAppHandlerCache() {
245245

246246
private volatile HttpSolrClientProvider solrClientProvider;
247247

248-
private volatile ExecutorService coreContainerWorkExecutor =
249-
ExecutorUtil.newMDCAwareCachedThreadPool(
250-
new SolrNamedThreadFactory("coreContainerWorkExecutor"));
248+
private volatile ExecutorService coreLoadExecutor;
251249

252250
private final OrderedExecutor<BytesRef> replayUpdatesExecutor;
253251

@@ -839,16 +837,6 @@ private void loadInternal() {
839837

840838
tracer = TracerConfigurator.loadTracer(loader, cfg.getTracerConfiguratorPluginInfo());
841839

842-
coreContainerWorkExecutor =
843-
MetricUtils.instrumentedExecutorService(
844-
coreContainerWorkExecutor,
845-
null,
846-
metricManager.registry(SolrMetricManager.getRegistryName(SolrInfoBean.Group.node)),
847-
SolrMetricManager.mkName(
848-
"coreContainerWorkExecutor",
849-
SolrInfoBean.Category.CONTAINER.toString(),
850-
"threadPool"));
851-
852840
shardHandlerFactory =
853841
ShardHandlerFactory.newInstance(cfg.getShardHandlerFactoryPluginInfo(), loader);
854842
if (shardHandlerFactory instanceof SolrMetricProducer metricProducer) {
@@ -1098,7 +1086,7 @@ private void loadInternal() {
10981086
}
10991087

11001088
// setup executor to load cores in parallel
1101-
ExecutorService coreLoadExecutor =
1089+
coreLoadExecutor =
11021090
MetricUtils.instrumentedExecutorService(
11031091
ExecutorUtil.newMDCAwareFixedThreadPool(
11041092
cfg.getCoreLoadThreadCount(isZooKeeperAware()),
@@ -1168,11 +1156,9 @@ private void loadInternal() {
11681156
backgroundCloser.start();
11691157

11701158
} finally {
1171-
if (asyncSolrCoreLoad) {
1172-
coreContainerWorkExecutor.execute(
1173-
() -> ExecutorUtil.shutdownAndAwaitTerminationForever(coreLoadExecutor));
1174-
} else {
1175-
ExecutorUtil.shutdownAndAwaitTerminationForever(coreLoadExecutor);
1159+
coreLoadExecutor.shutdown(); // doesn't block
1160+
if (!asyncSolrCoreLoad) {
1161+
ExecutorUtil.awaitTerminationForever(coreLoadExecutor);
11761162
}
11771163
}
11781164

@@ -1362,7 +1348,7 @@ public void shutdown() {
13621348
zkSys.zkController.tryCancelAllElections();
13631349
}
13641350

1365-
ExecutorUtil.shutdownAndAwaitTermination(coreContainerWorkExecutor);
1351+
ExecutorUtil.shutdownAndAwaitTermination(coreLoadExecutor); // actually already shutdown
13661352

13671353
// First wake up the closer thread, it'll terminate almost immediately since it checks
13681354
// isShutDown.

solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,8 @@ public void test() throws Exception {
276276
assertEquals(1, values.size());
277277
assertNotNull(values.get("solr.node"));
278278
values = (NamedList<?>) values.get("solr.node");
279-
assertEquals(27, values.size());
279+
assertEquals(15, values.size());
280280
assertNotNull(values.get("CONTAINER.cores.lazy")); // this is a gauge node
281-
assertNotNull(values.get("CONTAINER.threadPool.coreContainerWorkExecutor.completed"));
282281
assertNotNull(values.get("CONTAINER.threadPool.coreLoadExecutor.completed"));
283282

284283
resp = new SolrQueryResponse();
@@ -300,8 +299,7 @@ public void test() throws Exception {
300299
values = (NamedList<?>) values.get("metrics");
301300
assertNotNull(values.get("solr.node"));
302301
values = (NamedList<?>) values.get("solr.node");
303-
assertEquals(7, values.size());
304-
assertNotNull(values.get("CONTAINER.threadPool.coreContainerWorkExecutor.completed"));
302+
assertEquals(5, values.size());
305303
assertNotNull(values.get("CONTAINER.threadPool.coreLoadExecutor.completed"));
306304

307305
resp = new SolrQueryResponse();

0 commit comments

Comments
 (0)