From 94e213dd5ff5262f4cce4e928ad2a2abbc8f3e21 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 9 Jul 2020 15:30:50 -0500 Subject: [PATCH] Scripting: Per context stats in `script` in _nodes/stats (#59266) Updated `_nodes/stats`: * Update `script` in `_node/stats` to include stats per context: ``` "script": { "compilations": 1, "cache_evictions": 0, "compilation_limit_triggered": 0, "contexts":[ { "context": "aggregation_selector", "compilations": 0, "cache_evictions": 0, "compilation_limit_triggered": 0 }, ``` Refs: #50152 Backport: #59625 --- .../admin/cluster/node/stats/NodeStats.java | 12 ++- .../org/elasticsearch/script/ScriptCache.java | 4 + .../script/ScriptCacheStats.java | 17 +++- .../script/ScriptContextStats.java | 97 +++++++++++++++++++ .../elasticsearch/script/ScriptMetrics.java | 17 +++- .../elasticsearch/script/ScriptService.java | 7 +- .../org/elasticsearch/script/ScriptStats.java | 71 ++++++++++---- .../cluster/node/stats/NodeStatsTests.java | 13 +-- .../script/ScriptServiceTests.java | 2 - 9 files changed, 196 insertions(+), 44 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/script/ScriptContextStats.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java index 60075710d5486..56a177e496658 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java @@ -117,10 +117,13 @@ public NodeStats(StreamInput in) throws IOException { } else { adaptiveSelectionStats = null; } + scriptCacheStats = null; if (in.getVersion().onOrAfter(Version.V_7_8_0)) { - scriptCacheStats = in.readOptionalWriteable(ScriptCacheStats::new); - } else { - scriptCacheStats = null; + if (in.getVersion().before(Version.V_7_9_0)) { + scriptCacheStats = in.readOptionalWriteable(ScriptCacheStats::new); + } else if (scriptStats != null) { + scriptCacheStats = scriptStats.toScriptCacheStats(); + } } } @@ -271,8 +274,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalWriteable(ingestStats); if (out.getVersion().onOrAfter(Version.V_6_1_0)) { out.writeOptionalWriteable(adaptiveSelectionStats); - } - if (out.getVersion().onOrAfter(Version.V_7_8_0)) { + } if (out.getVersion().onOrAfter(Version.V_7_8_0) && out.getVersion().before(Version.V_7_9_0)) { out.writeOptionalWriteable(scriptCacheStats); } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index aa2c16ec9b7b8..3a4d93b290c45 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCache.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCache.java @@ -136,6 +136,10 @@ public ScriptStats stats() { return scriptMetrics.stats(); } + public ScriptContextStats stats(String context) { + return scriptMetrics.stats(context); + } + /** * Check whether there have been too many compilations within the last minute, throwing a circuit breaking exception if so. * This is a variant of the token bucket algorithm: https://en.wikipedia.org/wiki/Token_bucket diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java index a5d6fd1cf46b8..b41420be895e4 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java @@ -32,8 +32,9 @@ import java.util.Objects; import java.util.stream.Collectors; +// This class is deprecated in favor of ScriptStats and ScriptContextStats. It is removed in 8. public class ScriptCacheStats implements Writeable, ToXContentFragment { - private final Map context; + private final Map context; private final ScriptStats general; public ScriptCacheStats(Map context) { @@ -135,7 +136,19 @@ public ScriptStats sum() { if (general != null) { return general; } - return ScriptStats.sum(context.values()); + long compilations = 0; + long cacheEvictions = 0; + long compilationLimitTriggered = 0; + for (ScriptStats stat: context.values()) { + compilations += stat.getCompilations(); + cacheEvictions += stat.getCacheEvictions(); + compilationLimitTriggered += stat.getCompilationLimitTriggered(); + } + return new ScriptStats( + compilations, + cacheEvictions, + compilationLimitTriggered + ); } static final class Fields { diff --git a/server/src/main/java/org/elasticsearch/script/ScriptContextStats.java b/server/src/main/java/org/elasticsearch/script/ScriptContextStats.java new file mode 100644 index 0000000000000..7f376d9e33fb9 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/script/ScriptContextStats.java @@ -0,0 +1,97 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.script; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ToXContentFragment; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Objects; + +public class ScriptContextStats implements Writeable, ToXContentFragment, Comparable { + private final String context; + private final long compilations; + private final long cacheEvictions; + private final long compilationLimitTriggered; + + public ScriptContextStats(String context, long compilations, long cacheEvictions, long compilationLimitTriggered) { + this.context = Objects.requireNonNull(context); + this.compilations = compilations; + this.cacheEvictions = cacheEvictions; + this.compilationLimitTriggered = compilationLimitTriggered; + } + + public ScriptContextStats(StreamInput in) throws IOException { + context = in.readString(); + compilations = in.readVLong(); + cacheEvictions = in.readVLong(); + compilationLimitTriggered = in.readVLong(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(context); + out.writeVLong(compilations); + out.writeVLong(cacheEvictions); + out.writeVLong(compilationLimitTriggered); + } + + public String getContext() { + return context; + } + + public long getCompilations() { + return compilations; + } + + public long getCacheEvictions() { + return cacheEvictions; + } + + public long getCompilationLimitTriggered() { + return compilationLimitTriggered; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(Fields.CONTEXT, getContext()); + builder.field(Fields.COMPILATIONS, getCompilations()); + builder.field(Fields.CACHE_EVICTIONS, getCacheEvictions()); + builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, getCompilationLimitTriggered()); + builder.endObject(); + return builder; + } + + @Override + public int compareTo(ScriptContextStats o) { + return this.context.compareTo(o.context); + } + + static final class Fields { + static final String CONTEXT = "context"; + static final String COMPILATIONS = "compilations"; + static final String CACHE_EVICTIONS = "cache_evictions"; + static final String COMPILATION_LIMIT_TRIGGERED = "compilation_limit_triggered"; + } +} diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java b/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java index 18c1027074b16..3a91d18ec92f3 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java @@ -26,10 +26,6 @@ public class ScriptMetrics { final CounterMetric cacheEvictionsMetric = new CounterMetric(); final CounterMetric compilationLimitTriggered = new CounterMetric(); - public ScriptStats stats() { - return new ScriptStats(compilationsMetric.count(), cacheEvictionsMetric.count(), compilationLimitTriggered.count()); - } - public void onCompilation() { compilationsMetric.inc(); } @@ -41,4 +37,17 @@ public void onCacheEviction() { public void onCompilationLimit() { compilationLimitTriggered.inc(); } + + public ScriptStats stats() { + return new ScriptStats(compilationsMetric.count(), cacheEvictionsMetric.count(), compilationLimitTriggered.count()); + } + + public ScriptContextStats stats(String context) { + return new ScriptContextStats( + context, + compilationsMetric.count(), + cacheEvictionsMetric.count(), + compilationLimitTriggered.count() + ); + } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index 1f752942d7496..a7508fa1e2dea 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -658,7 +658,12 @@ ScriptStats stats() { if (general != null) { return general.stats(); } - return ScriptStats.sum(contextCache.values().stream().map(AtomicReference::get).map(ScriptCache::stats)::iterator); + List contextStats = new ArrayList<>(contextCache.size()); + for (Map.Entry> entry : contextCache.entrySet()) { + ScriptCache cache = entry.getValue().get(); + contextStats.add(cache.stats(entry.getKey())); + } + return new ScriptStats(contextStats); } ScriptCacheStats cacheStats() { diff --git a/server/src/main/java/org/elasticsearch/script/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index 87a5a1d4916c1..155068c9343f1 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -27,22 +27,52 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; public class ScriptStats implements Writeable, ToXContentFragment { + private final List contextStats; private final long compilations; private final long cacheEvictions; private final long compilationLimitTriggered; + public ScriptStats(List contextStats) { + ArrayList ctxStats = new ArrayList<>(contextStats.size()); + ctxStats.addAll(contextStats); + ctxStats.sort(ScriptContextStats::compareTo); + this.contextStats = Collections.unmodifiableList(ctxStats); + long compilations = 0; + long cacheEvictions = 0; + long compilationLimitTriggered = 0; + for (ScriptContextStats stats: contextStats) { + compilations += stats.getCompilations(); + cacheEvictions += stats.getCacheEvictions(); + compilationLimitTriggered += stats.getCompilationLimitTriggered(); + } + this.compilations = compilations; + this.cacheEvictions = cacheEvictions; + this.compilationLimitTriggered = compilationLimitTriggered; + } + public ScriptStats(long compilations, long cacheEvictions, long compilationLimitTriggered) { + this.contextStats = Collections.emptyList(); this.compilations = compilations; this.cacheEvictions = cacheEvictions; this.compilationLimitTriggered = compilationLimitTriggered; } + public ScriptStats(ScriptContextStats context) { + this(context.getCompilations(), context.getCacheEvictions(), context.getCompilationLimitTriggered()); + } + public ScriptStats(StreamInput in) throws IOException { compilations = in.readVLong(); cacheEvictions = in.readVLong(); compilationLimitTriggered = in.getVersion().onOrAfter(Version.V_7_0_0) ? in.readVLong() : 0; + contextStats = in.getVersion().onOrAfter(Version.V_7_9_0) ? in.readList(ScriptContextStats::new) : Collections.emptyList(); } @Override @@ -52,6 +82,13 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_7_0_0)) { out.writeVLong(compilationLimitTriggered); } + if (out.getVersion().onOrAfter(Version.V_7_9_0)) { + out.writeList(contextStats); + } + } + + public List getContextStats() { + return contextStats; } public long getCompilations() { @@ -66,36 +103,32 @@ public long getCompilationLimitTriggered() { return compilationLimitTriggered; } + public ScriptCacheStats toScriptCacheStats() { + if (contextStats.isEmpty()) { + return new ScriptCacheStats(this); + } + Map contexts = new HashMap<>(contextStats.size()); + for (ScriptContextStats contextStats : contextStats) { + contexts.put(contextStats.getContext(), new ScriptStats(contextStats)); + } + return new ScriptCacheStats(contexts); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.SCRIPT_STATS); - builder.field(Fields.COMPILATIONS, getCompilations()); - builder.field(Fields.CACHE_EVICTIONS, getCacheEvictions()); - builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, getCompilationLimitTriggered()); + builder.field(Fields.COMPILATIONS, compilations); + builder.field(Fields.CACHE_EVICTIONS, cacheEvictions); + builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, compilationLimitTriggered); builder.endObject(); return builder; } static final class Fields { static final String SCRIPT_STATS = "script"; + static final String CONTEXTS = "contexts"; static final String COMPILATIONS = "compilations"; static final String CACHE_EVICTIONS = "cache_evictions"; static final String COMPILATION_LIMIT_TRIGGERED = "compilation_limit_triggered"; } - - public static ScriptStats sum(Iterable stats) { - long compilations = 0; - long cacheEvictions = 0; - long compilationLimitTriggered = 0; - for (ScriptStats stat: stats) { - compilations += stat.compilations; - cacheEvictions += stat.cacheEvictions; - compilationLimitTriggered += stat.compilationLimitTriggered; - } - return new ScriptStats( - compilations, - cacheEvictions, - compilationLimitTriggered - ); - } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index f218a9671d195..4c0dc158c8ec4 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -316,7 +316,7 @@ public void testSerialization() throws IOException { ScriptCacheStats deserializedScriptCacheStats = deserializedNodeStats.getScriptCacheStats(); if (scriptCacheStats == null) { assertNull(deserializedScriptCacheStats); - } else { + } else if (deserializedScriptCacheStats.getContextStats() != null) { Map deserialized = deserializedScriptCacheStats.getContextStats(); long evictions = 0; long limited = 0; @@ -514,16 +514,7 @@ public static NodeStats createNodeStats() { } adaptiveSelectionStats = new AdaptiveSelectionStats(nodeConnections, nodeStats); } - ScriptCacheStats scriptCacheStats = null; - if (frequently()) { - int numContents = randomIntBetween(0, 20); - Map stats = new HashMap<>(numContents); - for (int i = 0; i < numContents; i++) { - String context = randomValueOtherThanMany(stats::containsKey, () -> randomAlphaOfLength(12)); - stats.put(context, new ScriptStats(randomLongBetween(0, 1024), randomLongBetween(0, 1024), randomLongBetween(0, 1024))); - } - scriptCacheStats = new ScriptCacheStats(stats); - } + ScriptCacheStats scriptCacheStats = scriptStats != null ? scriptStats.toScriptCacheStats() : null; //TODO NodeIndicesStats are not tested here, way too complicated to create, also they need to be migrated to Writeable yet return new NodeStats(node, randomNonNegativeLong(), null, osStats, processStats, jvmStats, threadPoolStats, fsInfo, transportStats, httpStats, allCircuitBreakerStats, scriptStats, discoveryStats, diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 5f137bee99647..4364ebd4c021b 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -37,7 +37,6 @@ import org.junit.Before; import java.io.IOException; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -585,7 +584,6 @@ public void testCacheHolderChangeSettings() throws IOException { ); assertEquals(contexts.keySet(), scriptService.cacheHolder.get().contextCache.keySet()); - String d = randomValueOtherThanMany(Arrays.asList(a, b, c)::contains, () -> randomFrom(contextNames)); assertEquals(new ScriptCache.CompilationRate(aRate), scriptService.cacheHolder.get().contextCache.get(a).get().rate); assertEquals(new ScriptCache.CompilationRate(bRate),