From 3c963967242aed73a906b7bc17c26a4b8b07083c Mon Sep 17 00:00:00 2001 From: Shalin Shekhar Mangar Date: Tue, 3 Jan 2017 15:52:01 +0530 Subject: [PATCH] SOLR-9896: Instrument and collect metrics from query, update, core admin and core load thread pools --- solr/CHANGES.txt | 2 ++ .../java/org/apache/solr/core/CoreContainer.java | 15 ++++++++++++--- .../solr/handler/admin/CoreAdminHandler.java | 13 ++++++++++++- .../component/HttpShardHandlerFactory.java | 10 +++++++--- .../apache/solr/update/UpdateShardHandler.java | 9 ++++++++- .../org/apache/solr/util/stats/MetricUtils.java | 9 +++++++++ 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6326e547113..8609f9139bc 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -210,6 +210,8 @@ New Features * SOLR-9684: Add priority Streaming Expression (Joel Bernstein, David Smiley) +* SOLR-9896: Instrument and collect metrics from query, update, core admin and core load thread pools. (shalin) + Optimizations ---------------------- * SOLR-9704: Facet Module / JSON Facet API: Optimize blockChildren facets that have 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 f3747dcb9f6..de7c34d8a70 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -78,6 +78,7 @@ import org.apache.solr.security.SecurityPluginHolder; import org.apache.solr.update.SolrCoreState; import org.apache.solr.update.UpdateShardHandler; import org.apache.solr.util.DefaultSolrThreadFactory; +import org.apache.solr.util.stats.MetricUtils; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -462,6 +463,11 @@ public class CoreContainer { metricManager = new SolrMetricManager(); + coreContainerWorkExecutor = MetricUtils.instrumentedExecutorService( + coreContainerWorkExecutor, + metricManager.registry(SolrMetricManager.getRegistryName(SolrInfoMBean.Group.node)), + SolrMetricManager.mkName("coreContainerWorkExecutor", "threadPool")); + shardHandlerFactory = ShardHandlerFactory.newInstance(cfg.getShardHandlerFactoryPluginInfo(), loader); if (shardHandlerFactory instanceof SolrMetricProducer) { SolrMetricProducer metricProducer = (SolrMetricProducer) shardHandlerFactory; @@ -520,9 +526,12 @@ public class CoreContainer { unloadedCores, true, "unloaded", "cores"); // setup executor to load cores in parallel - ExecutorService coreLoadExecutor = ExecutorUtil.newMDCAwareFixedThreadPool( - cfg.getCoreLoadThreadCount(isZooKeeperAware()), - new DefaultSolrThreadFactory("coreLoadExecutor") ); + ExecutorService coreLoadExecutor = MetricUtils.instrumentedExecutorService( + ExecutorUtil.newMDCAwareFixedThreadPool( + cfg.getCoreLoadThreadCount(isZooKeeperAware()), + new DefaultSolrThreadFactory("coreLoadExecutor")), + metricManager.registry(SolrMetricManager.getRegistryName(SolrInfoMBean.Group.node)), + SolrMetricManager.mkName("coreLoadExecutor", "threadPool")); final List> futures = new ArrayList<>(); try { List cds = coresLocator.discover(this); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java index 7b0ecfb7633..458b7a5eefc 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java @@ -18,6 +18,7 @@ package org.apache.solr.handler.admin; import java.io.File; import java.lang.invoke.MethodHandles; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -42,11 +43,13 @@ import org.apache.solr.common.util.NamedList; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.handler.RequestHandlerBase; +import org.apache.solr.metrics.SolrMetricManager; 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.apache.solr.util.DefaultSolrThreadFactory; +import org.apache.solr.util.stats.MetricUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; @@ -65,7 +68,7 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa protected final CoreContainer coreContainer; protected final Map> requestStatusMap; - protected final ExecutorService parallelExecutor = ExecutorUtil.newMDCAwareFixedThreadPool(50, + protected ExecutorService parallelExecutor = ExecutorUtil.newMDCAwareFixedThreadPool(50, new DefaultSolrThreadFactory("parallelCoreAdminExecutor")); protected static int MAX_TRACKED_REQUESTS = 100; @@ -111,6 +114,14 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa "it is a special Handler configured directly by the RequestDispatcher"); } + @Override + public Collection initializeMetrics(SolrMetricManager manager, String registryName, String scope) { + Collection metrics = super.initializeMetrics(manager, registryName, scope); + parallelExecutor = MetricUtils.instrumentedExecutorService(parallelExecutor, manager.registry(registryName), + SolrMetricManager.mkName("parallelCoreAdminExecutor", getCategory().name(),scope, "threadPool")); + return metrics; + } + /** * The instance of CoreContainer this handler handles. This should be the CoreContainer instance that created this * handler. diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java index 3c01720c6bb..d190ce03f1f 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java +++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java @@ -42,6 +42,7 @@ import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.util.DefaultSolrThreadFactory; import org.apache.solr.util.stats.InstrumentedHttpRequestExecutor; import org.apache.solr.util.stats.InstrumentedPoolingHttpClientConnectionManager; +import org.apache.solr.util.stats.MetricUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,8 +59,8 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CompletionService; import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.ExecutorService; import java.util.concurrent.SynchronousQueue; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -73,7 +74,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory implements org. // // Consider CallerRuns policy and a lower max threads to throttle // requests at some point (or should we simply return failure?) - private ThreadPoolExecutor commExecutor = new ExecutorUtil.MDCAwareThreadPoolExecutor( + private ExecutorService commExecutor = new ExecutorUtil.MDCAwareThreadPoolExecutor( 0, Integer.MAX_VALUE, 5, TimeUnit.SECONDS, // terminate idle threads after 5 sec @@ -191,7 +192,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory implements org. return clientParams; } - protected ThreadPoolExecutor getThreadPoolExecutor(){ + protected ExecutorService getThreadPoolExecutor(){ return this.commExecutor; } @@ -378,6 +379,9 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory implements org. List metricNames = new ArrayList<>(4); metricNames.addAll(clientConnectionManager.initializeMetrics(manager, registry, scope)); metricNames.addAll(httpRequestExecutor.initializeMetrics(manager, registry, scope)); + commExecutor = MetricUtils.instrumentedExecutorService(commExecutor, + manager.registry(registry), + SolrMetricManager.mkName("httpShardExecutor", scope, "threadPool")); return metricNames; } diff --git a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java index c3ed8cd14e7..9d230bcd57d 100644 --- a/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java +++ b/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.ExecutorService; +import com.codahale.metrics.InstrumentedExecutorService; import org.apache.http.client.HttpClient; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; @@ -102,12 +103,18 @@ public class UpdateShardHandler implements SolrMetricProducer { List metricNames = new ArrayList<>(4); metricNames.addAll(clientConnectionManager.initializeMetrics(manager, registry, scope)); metricNames.addAll(httpRequestExecutor.initializeMetrics(manager, registry, scope)); + updateExecutor = new InstrumentedExecutorService(updateExecutor, + manager.registry(registry), + SolrMetricManager.mkName("updateExecutor", scope, "threadPool")); + recoveryExecutor = new InstrumentedExecutorService(recoveryExecutor, + manager.registry(registry), + SolrMetricManager.mkName("recoveryExecutor", scope, "threadPool")); return metricNames; } @Override public String getDescription() { - return "Metrics tracked by UpdateShardHandler for "; + return "Metrics tracked by UpdateShardHandler related to distributed updates and recovery"; } @Override diff --git a/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java b/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java index 62f57763fcf..af5a0b5ff42 100644 --- a/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java +++ b/solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java @@ -19,11 +19,13 @@ package org.apache.solr.util.stats; import java.util.List; import java.util.Map; import java.util.SortedSet; +import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; +import com.codahale.metrics.InstrumentedExecutorService; import com.codahale.metrics.Meter; import com.codahale.metrics.Metric; import com.codahale.metrics.MetricFilter; @@ -141,4 +143,11 @@ public class MetricUtils { response.add("requests", counter.getCount()); return response; } + + /** + * Returns an instrumented wrapper over the given executor service. + */ + public static ExecutorService instrumentedExecutorService(ExecutorService delegate, MetricRegistry metricRegistry, String scope) { + return new InstrumentedExecutorService(delegate, metricRegistry, scope); + } }