[solr] branch main updated: SOLR-15616: Allow thread metrics to be cached

[is...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ishan 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 3eda6fbed7d SOLR-15616: Allow thread metrics to be cached
3eda6fbed7d is described below

commit 3eda6fbed7d6bae72b855691beecc5d33d93eda5
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Mon Jan 23 19:32:29 2023 +0530

    SOLR-15616: Allow thread metrics to be cached
---
 solr/CHANGES.txt                                   |  1 +
 .../java/org/apache/solr/core/MetricsConfig.java   | 26 ++++++++++++++++++++--
 .../java/org/apache/solr/core/SolrXmlConfig.java   |  6 +++++
 .../apache/solr/servlet/CoreContainerProvider.java | 19 ++++++++++------
 solr/server/solr/solr.xml                          |  8 +++++++
 .../deployment-guide/pages/metrics-reporting.adoc  | 17 ++++++++++++++
 6 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0b0b6091ea3..3415214cdc7 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -136,6 +136,7 @@ Optimizations
 
 * SOLR-15732: queries to missing collection are slow (noble)
 
+* SOLR-15616: Allow thread metrics to be cached (Ishan Chattopadhyaya, ab)
 
 Bug Fixes
 ---------------------
diff --git a/solr/core/src/java/org/apache/solr/core/MetricsConfig.java b/solr/core/src/java/org/apache/solr/core/MetricsConfig.java
index ab84d53f3a1..fd6dc5cd7c5 100644
--- a/solr/core/src/java/org/apache/solr/core/MetricsConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/MetricsConfig.java
@@ -34,6 +34,7 @@ public class MetricsConfig {
   private final Object nullString;
   private final Object nullObject;
   private final boolean enabled;
+  private final CacheConfig cacheConfig;
 
   private MetricsConfig(
       boolean enabled,
@@ -46,7 +47,8 @@ public class MetricsConfig {
       Object nullNumber,
       Object notANumber,
       Object nullString,
-      Object nullObject) {
+      Object nullObject,
+      CacheConfig cacheConfig) {
     this.enabled = enabled;
     this.metricReporters = metricReporters;
     this.hiddenSysProps = hiddenSysProps;
@@ -58,12 +60,17 @@ public class MetricsConfig {
     this.notANumber = notANumber;
     this.nullString = nullString;
     this.nullObject = nullObject;
+    this.cacheConfig = cacheConfig;
   }
 
   public boolean isEnabled() {
     return enabled;
   }
 
+  public CacheConfig getCacheConfig() {
+    return cacheConfig;
+  }
+
   private static final PluginInfo[] NO_OP_REPORTERS = new PluginInfo[0];
 
   public PluginInfo[] getMetricReporters() {
@@ -149,6 +156,7 @@ public class MetricsConfig {
     private Object nullObject = null;
     // default to metrics enabled
     private boolean enabled = true;
+    private CacheConfig cacheConfig = null;
 
     public MetricsConfigBuilder() {}
 
@@ -157,6 +165,11 @@ public class MetricsConfig {
       return this;
     }
 
+    public MetricsConfigBuilder setCacheConfig(CacheConfig cacheConfig) {
+      this.cacheConfig = cacheConfig;
+      return this;
+    }
+
     public MetricsConfigBuilder setHiddenSysProps(Set<String> hiddenSysProps) {
       if (hiddenSysProps != null && !hiddenSysProps.isEmpty()) {
         this.hiddenSysProps.clear();
@@ -223,7 +236,16 @@ public class MetricsConfig {
           nullNumber,
           notANumber,
           nullString,
-          nullObject);
+          nullObject,
+          cacheConfig);
+    }
+  }
+
+  public static class CacheConfig {
+    public Integer threadsIntervalSeconds; // intervals for which the threads metrics are cached
+
+    public CacheConfig (Integer threadsIntervalSeconds) {
+      this.threadsIntervalSeconds = threadsIntervalSeconds;
     }
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
index 77c7adb67e0..a8a2c45cca2 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
@@ -657,6 +657,12 @@ public class SolrXmlConfig {
       builder.setNullObject(decodeNullValue(missingValues.get("nullObject")));
     }
 
+    ConfigNode caching = metrics.get("solr/metrics/caching");
+    if (caching != null) {
+      Object threadsCachingIntervalSeconds = DOMUtil.childNodesToNamedList(caching).get("threadsIntervalSeconds", null);
+      builder.setCacheConfig(new MetricsConfig.CacheConfig(threadsCachingIntervalSeconds == null? null: Integer.parseInt(threadsCachingIntervalSeconds.toString())));
+    }
+
     PluginInfo[] reporterPlugins = getMetricReporterPluginInfos(metrics);
     Set<String> hiddenSysProps = getHiddenSysProps(metrics);
     return builder
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 65568e584d4..1dc0a9dce6f 100644
--- a/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
+++ b/solr/core/src/java/org/apache/solr/servlet/CoreContainerProvider.java
@@ -23,6 +23,7 @@ import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIB
 import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_LEVEL;
 import static org.apache.solr.servlet.SolrDispatchFilter.SOLR_LOG_MUTECONSOLE;
 
+import com.codahale.metrics.jvm.CachedThreadStatesGaugeSet;
 import com.codahale.metrics.jvm.ClassLoadingGaugeSet;
 import com.codahale.metrics.jvm.GarbageCollectorMetricSet;
 import com.codahale.metrics.jvm.MemoryUsageGaugeSet;
@@ -60,6 +61,7 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.MetricsConfig;
 import org.apache.solr.core.NodeConfig;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrInfoBean.Group;
@@ -227,7 +229,7 @@ public class CoreContainerProvider implements ServletContextListener {
 
       coresInit = createCoreContainer(computeSolrHome(servletContext), extraProperties);
       this.httpClient = coresInit.getUpdateShardHandler().getDefaultHttpClient();
-      setupJvmMetrics(coresInit);
+      setupJvmMetrics(coresInit, coresInit.getNodeConfig().getMetricsConfig());
 
       SolrZkClient zkClient = null;
       ZkController zkController = coresInit.getZkController();
@@ -411,7 +413,7 @@ public class CoreContainerProvider implements ServletContextListener {
     return coreContainer;
   }
 
-  private void setupJvmMetrics(CoreContainer coresInit) {
+  private void setupJvmMetrics(CoreContainer coresInit, MetricsConfig config) {
     metricManager = coresInit.getMetricManager();
     registryName = SolrMetricManager.getRegistryName(Group.jvm);
     final Set<String> hiddenSysProps = coresInit.getConfig().getMetricsConfig().getHiddenSysProps();
@@ -426,11 +428,14 @@ public class CoreContainerProvider implements ServletContextListener {
           registryName, new GarbageCollectorMetricSet(), ResolutionStrategy.IGNORE, "gc");
       metricManager.registerAll(
           registryName, new MemoryUsageGaugeSet(), ResolutionStrategy.IGNORE, "memory");
-      metricManager.registerAll(
-          registryName,
-          new ThreadStatesGaugeSet(),
-          ResolutionStrategy.IGNORE,
-          "threads"); // todo should we use CachedThreadStatesGaugeSet instead?
+
+      if (config.getCacheConfig() != null && config.getCacheConfig().threadsIntervalSeconds != null) {
+         log.info("Threads metrics will be cached for " + config.getCacheConfig().threadsIntervalSeconds + " seconds");
+         metricManager.registerAll(registryName, new CachedThreadStatesGaugeSet(config.getCacheConfig().threadsIntervalSeconds, TimeUnit.SECONDS), SolrMetricManager.ResolutionStrategy.IGNORE, "threads");
+      } else {
+         metricManager.registerAll(registryName, new ThreadStatesGaugeSet(), SolrMetricManager.ResolutionStrategy.IGNORE, "threads");
+      }
+
       MetricsMap sysprops =
           new MetricsMap(
               map ->
diff --git a/solr/server/solr/solr.xml b/solr/server/solr/solr.xml
index 136154acfd6..459dbff87f2 100644
--- a/solr/server/solr/solr.xml
+++ b/solr/server/solr/solr.xml
@@ -60,7 +60,15 @@
   </shardHandlerFactory>
 
   <metrics enabled="${metricsEnabled:true}">
+    <!--    Solr computes JVM metrics for threads. Computing these metrics, esp. computing deadlocks etc.,
+     requires potentially expensive computations, and can be avoided for every metrics call by
+     setting a high caching expiration interval (in seconds).
+      <caching>
+        <int name="threadsIntervalSeconds">5</int>
+      </caching>
+    -->
     <!--reporter name="jmx_metrics" group="core" class="org.apache.solr.metrics.reporters.SolrJmxReporter"/-->
   </metrics>
 
+
 </solr>
diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc
index b97a25280cf..ac069e247ec 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/metrics-reporting.adoc
@@ -310,6 +310,23 @@ complex objects:
 </metrics>
 ----
 
+=== Caching Threads Metrics ===
+The threads metrics in the JVM group can be expensive to compute, as it requires traversing all threads. 
+This can be avoided for every call to the metrics API (group=jvm) by setting a high caching expiration interval
+(in seconds). For example, to cache the thread metrics for 5 seconds:
+
+[source,xml]
+----
+<solr>
+ <metrics>
+  <caching>
+   <int name="threadsIntervalSeconds">5</int>
+  </caching>
+  ...
+ </metrics>
+...
+</solr>
+----
 
 == Reporters