From daff146c3d6c9cba497b672c9c3de7944f35d296 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Fri, 6 Sep 2024 16:58:37 -0700 Subject: [PATCH] Use 2 prometheus metrics calls (#222) * Try making 2 calls and see if it's faster * Try making 2 calls and see if it's faster * Fixed unit test cases --- .../servlet/PrometheusMetricsServlet.java | 111 ++++++++++++++---- .../servlet/PrometheusMetricsServletTest.java | 5 +- 2 files changed, 94 insertions(+), 22 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/servlet/PrometheusMetricsServlet.java b/solr/core/src/java/org/apache/solr/servlet/PrometheusMetricsServlet.java index 517966f7813..3adc9961ade 100644 --- a/solr/core/src/java/org/apache/solr/servlet/PrometheusMetricsServlet.java +++ b/solr/core/src/java/org/apache/solr/servlet/PrometheusMetricsServlet.java @@ -72,13 +72,16 @@ public final class PrometheusMetricsServlet extends BaseSolrServlet { private final List callers = getCallers(); private List getCallers() { + AggregateMetricsApiCaller aggregateMetricsApiCaller = new AggregateMetricsApiCaller(); return List.of( - new GarbageCollectorMetricsApiCaller(), - new MemoryMetricsApiCaller(), - new OsMetricsApiCaller(), - new ThreadMetricsApiCaller(), - new StatusCodeMetricsApiCaller(), - new AggregateMetricsApiCaller()); + new GarbageCollectorMetricsApiCaller(), + new MemoryMetricsApiCaller(), + new OsMetricsApiCaller(), + new ThreadMetricsApiCaller(), + new StatusCodeMetricsApiCaller(), + aggregateMetricsApiCaller, + new CoresMetricsApiCaller( + Collections.unmodifiableList(aggregateMetricsApiCaller.missingCoreMetrics))); } private final Map cacheMetricTypes = @@ -827,8 +830,9 @@ static class AggregateMetricsApiCaller extends MetricsByPrefixApiCaller { }, ... */ + List missingCoreMetrics = new ArrayList<>(); AggregateMetricsApiCaller() { - super("solr.node,core", buildPrefix(), buildProperty()); + super("solr.node", buildPrefix(), buildProperty()); } private static String buildPrefix() { @@ -847,7 +851,7 @@ private static String buildProperty() { @Override protected void handle(List results, JsonNode metricsNode) throws IOException { - List missingCoreMetrics = new ArrayList<>(); + missingCoreMetrics.clear(); JsonNode nodeMetricNode = metricsNode.get("solr.node"); if (nodeMetricNode != null) { @@ -867,23 +871,90 @@ protected void handle(List results, JsonNode metricsNode) thro "Cannot find the solr.node metrics, going to fall back to getting metrics from all cores"); missingCoreMetrics.addAll(Arrays.asList(CoreMetric.values())); } + } + } - Map accumulative = new LinkedHashMap<>(); - for (Map.Entry entry : metricsNode.properties()) { - if ("solr.node".equals(entry.getKey())) { // this one is not a core - continue; + /** + * Collector that get metrics from all the cores and then sum those metrics by CoreMetric key. + * + *

This runs after AggregateMetricsApiCaller and pick up whatever is missing from it by reading + * missingCoreMetricsView. + * + *

Therefore, this has dependency on AggregateMetricsApiCaller and should not be executed + * concurrently with it. + */ + static class CoresMetricsApiCaller extends MetricsApiCaller { + private final List missingCoreMetricsView; + + CoresMetricsApiCaller(List missingCoreMetricsView) { + this.missingCoreMetricsView = missingCoreMetricsView; + } + + @Override + protected String buildQueryString() { + List prefixes = new ArrayList<>(); + List properties = new ArrayList<>(); + for (CoreMetric missingMetric : missingCoreMetricsView) { + prefixes.add(missingMetric.key); + if (missingMetric.property != null) { + properties.add(missingMetric.property); } - JsonNode coreMetricNode = entry.getValue(); - for (CoreMetric missingCoreMetric : - missingCoreMetrics) { // only iterate on those that aren't accounted for by "solr.node" + } + + String propertyClause = + String.join( + "&property=", + properties.stream() + .map(p -> URLEncoder.encode(p, StandardCharsets.UTF_8)) + .collect(Collectors.toSet())); + return String.format( + Locale.ROOT, + "wt=json&indent=false&compact=true&group=%s&prefix=%s%s", + "core", + URLEncoder.encode(String.join(",", prefixes), StandardCharsets.UTF_8), + propertyClause); + } + + + /* + "metrics":{ + "solr.core.loadtest.shard1_1.replica_n8":{ + "INDEX.merge.errors":0, + "INDEX.merge.major":{"count":0}, + "INDEX.merge.major.running":0, + "INDEX.merge.major.running.docs":0, + "INDEX.merge.major.running.segments":0, + "INDEX.merge.minor":{"count":0}, + "INDEX.merge.minor.running":0, + "INDEX.merge.minor.running.docs":0, + "INDEX.merge.minor.running.segments":0, + "QUERY./get.requestTimes":{"count":0}, + "QUERY./get[shard].requestTimes":{"count":0}, + "QUERY./select.requestTimes":{"count":2}, + "QUERY./select[shard].requestTimes":{"count":0}, + "UPDATE./update.requestTimes":{"count":0}, + "UPDATE./update[local].requestTimes":{"count":0}, + "UPDATE.updateHandler.autoCommits":0, + "UPDATE.updateHandler.commits":{"count":14877}, + "UPDATE.updateHandler.cumulativeDeletesById":{"count":0}, + "UPDATE.updateHandler.cumulativeDeletesByQuery":{"count":0}, + "UPDATE.updateHandler.softAutoCommits":0}, + ... + */ + + @Override + protected void handle(List results, JsonNode metrics) throws IOException { + Map accumulative = new LinkedHashMap<>(); + for (CoreMetric missingCoreMetric : missingCoreMetricsView) { + for (JsonNode coreMetricNode : metrics) { Number val = - missingCoreMetric.property != null - ? getNumber(coreMetricNode, missingCoreMetric.key, missingCoreMetric.property) - : getNumber(coreMetricNode, missingCoreMetric.key); + missingCoreMetric.property != null + ? getNumber(coreMetricNode, missingCoreMetric.key, missingCoreMetric.property) + : getNumber(coreMetricNode, missingCoreMetric.key); if (!val.equals(INVALID_NUMBER)) { accumulative.put( - missingCoreMetric, - accumulative.getOrDefault(missingCoreMetric, 0L) + val.longValue()); + missingCoreMetric, + accumulative.getOrDefault(missingCoreMetric, 0L) + val.longValue()); } } } diff --git a/solr/core/src/test/org/apache/solr/servlet/PrometheusMetricsServletTest.java b/solr/core/src/test/org/apache/solr/servlet/PrometheusMetricsServletTest.java index c153e919948..9bc873af9df 100644 --- a/solr/core/src/test/org/apache/solr/servlet/PrometheusMetricsServletTest.java +++ b/solr/core/src/test/org/apache/solr/servlet/PrometheusMetricsServletTest.java @@ -20,6 +20,7 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Assert; @@ -386,7 +387,7 @@ public void testCoresMetricsApiCaller() throws Exception { + "# TYPE update_errors counter\n" + "update_errors 4\n"; assertMetricsApiCaller( - new PrometheusMetricsServlet.AggregateMetricsApiCaller(), json, 14, output); + new PrometheusMetricsServlet.CoresMetricsApiCaller(Arrays.asList(PrometheusMetricsServlet.CoreMetric.values())), json, 14, output); } @Test @@ -487,6 +488,6 @@ public void testCoresMetricsApiCallerMissingIndex() throws Exception { + "# TYPE update_errors counter\n" + "update_errors 0\n"; assertMetricsApiCaller( - new PrometheusMetricsServlet.AggregateMetricsApiCaller(), json, 25, output); + new PrometheusMetricsServlet.CoresMetricsApiCaller(Arrays.asList(PrometheusMetricsServlet.CoreMetric.values())), json, 25, output); } }