From 8d8bd3df7c9f56995a13d8c27b515b4a45decaba Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 9 Jul 2020 14:30:10 -0500 Subject: [PATCH] Scripting: Move script_cache into _nodes/stats (#59265) Updated `_nodes/stats`: * Remove `script_cache` * 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 --- build.gradle | 4 +- .../admin/cluster/node/stats/NodeStats.java | 25 +-- .../org/elasticsearch/node/NodeService.java | 3 +- .../org/elasticsearch/script/ScriptCache.java | 4 +- .../script/ScriptCacheStats.java | 147 ------------------ .../script/ScriptContextStats.java | 97 ++++++++++++ .../elasticsearch/script/ScriptMetrics.java | 14 +- .../elasticsearch/script/ScriptService.java | 16 +- .../org/elasticsearch/script/ScriptStats.java | 49 +++--- .../cluster/node/stats/NodeStatsTests.java | 91 +++++------ .../elasticsearch/cluster/DiskUsageTests.java | 18 +-- .../script/ScriptServiceTests.java | 26 ++-- .../script/ScriptStatsTests.java | 68 ++++++++ .../MockInternalClusterInfoService.java | 2 +- ...chineLearningInfoTransportActionTests.java | 2 +- ...sportGetTrainedModelsStatsActionTests.java | 2 +- .../node/NodeStatsMonitoringDocTests.java | 2 +- 17 files changed, 286 insertions(+), 284 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java create mode 100644 server/src/main/java/org/elasticsearch/script/ScriptContextStats.java create mode 100644 server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java diff --git a/build.gradle b/build.gradle index 14419937ea9ca..6fd7909f0b3a9 100644 --- a/build.gradle +++ b/build.gradle @@ -174,8 +174,8 @@ tasks.register("verifyVersions") { * after the backport of the backcompat code is complete. */ -boolean bwc_tests_enabled = true -final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */ +boolean bwc_tests_enabled = false +final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/59265" /* place a PR link here when committing bwc changes */ if (bwc_tests_enabled == false) { if (bwc_tests_disabled_issue.isEmpty()) { throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false") 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 1106d6aca46b4..60f1fc4da1063 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 @@ -19,7 +19,6 @@ package org.elasticsearch.action.admin.cluster.node.stats; -import org.elasticsearch.Version; import org.elasticsearch.action.support.nodes.BaseNodeResponse; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; @@ -38,7 +37,6 @@ import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.monitor.process.ProcessStats; import org.elasticsearch.node.AdaptiveSelectionStats; -import org.elasticsearch.script.ScriptCacheStats; import org.elasticsearch.script.ScriptStats; import org.elasticsearch.threadpool.ThreadPoolStats; import org.elasticsearch.transport.TransportStats; @@ -83,9 +81,6 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { @Nullable private ScriptStats scriptStats; - @Nullable - private ScriptCacheStats scriptCacheStats; - @Nullable private DiscoveryStats discoveryStats; @@ -113,11 +108,6 @@ public NodeStats(StreamInput in) throws IOException { discoveryStats = in.readOptionalWriteable(DiscoveryStats::new); ingestStats = in.readOptionalWriteable(IngestStats::new); adaptiveSelectionStats = in.readOptionalWriteable(AdaptiveSelectionStats::new); - if (in.getVersion().onOrAfter(Version.V_7_8_0)) { - scriptCacheStats = in.readOptionalWriteable(ScriptCacheStats::new); - } else { - scriptCacheStats = null; - } } public NodeStats(DiscoveryNode node, long timestamp, @Nullable NodeIndicesStats indices, @@ -127,8 +117,7 @@ public NodeStats(DiscoveryNode node, long timestamp, @Nullable NodeIndicesStats @Nullable ScriptStats scriptStats, @Nullable DiscoveryStats discoveryStats, @Nullable IngestStats ingestStats, - @Nullable AdaptiveSelectionStats adaptiveSelectionStats, - @Nullable ScriptCacheStats scriptCacheStats) { + @Nullable AdaptiveSelectionStats adaptiveSelectionStats) { super(node); this.timestamp = timestamp; this.indices = indices; @@ -144,7 +133,6 @@ public NodeStats(DiscoveryNode node, long timestamp, @Nullable NodeIndicesStats this.discoveryStats = discoveryStats; this.ingestStats = ingestStats; this.adaptiveSelectionStats = adaptiveSelectionStats; - this.scriptCacheStats = scriptCacheStats; } public long getTimestamp() { @@ -239,11 +227,6 @@ public AdaptiveSelectionStats getAdaptiveSelectionStats() { return adaptiveSelectionStats; } - @Nullable - public ScriptCacheStats getScriptCacheStats() { - return scriptCacheStats; - } - @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); @@ -266,9 +249,6 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalWriteable(discoveryStats); out.writeOptionalWriteable(ingestStats); out.writeOptionalWriteable(adaptiveSelectionStats); - if (out.getVersion().onOrAfter(Version.V_7_8_0)) { - out.writeOptionalWriteable(scriptCacheStats); - } } @Override @@ -332,9 +312,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (getAdaptiveSelectionStats() != null) { getAdaptiveSelectionStats().toXContent(builder, params); } - if (getScriptCacheStats() != null) { - getScriptCacheStats().toXContent(builder, params); - } return builder; } } diff --git a/server/src/main/java/org/elasticsearch/node/NodeService.java b/server/src/main/java/org/elasticsearch/node/NodeService.java index 98e1f18dff289..2d42dfda9ea09 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeService.java +++ b/server/src/main/java/org/elasticsearch/node/NodeService.java @@ -119,8 +119,7 @@ public NodeStats stats(CommonStatsFlags indices, boolean os, boolean process, bo script ? scriptService.stats() : null, discoveryStats ? discovery.stats() : null, ingest ? ingestService.stats() : null, - adaptiveSelection ? responseCollectorService.getAdaptiveStats(searchTransportService.getPendingSearchRequests()) : null, - scriptCache ? scriptService.cacheStats() : null + adaptiveSelection ? responseCollectorService.getAdaptiveStats(searchTransportService.getPendingSearchRequests()) : null ); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index 5d59a91b207ce..58fb9ed16cee3 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCache.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCache.java @@ -132,8 +132,8 @@ static void rethrow(Throwable t) throws T { throw (T) t; } - public ScriptStats stats() { - return scriptMetrics.stats(); + public ScriptContextStats stats(String context) { + return scriptMetrics.stats(context); } /** diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java deleted file mode 100644 index a5d6fd1cf46b8..0000000000000 --- a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java +++ /dev/null @@ -1,147 +0,0 @@ -/* - * 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.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; - -public class ScriptCacheStats implements Writeable, ToXContentFragment { - private final Map context; - private final ScriptStats general; - - public ScriptCacheStats(Map context) { - this.context = Collections.unmodifiableMap(context); - this.general = null; - } - - public ScriptCacheStats(ScriptStats general) { - this.general = Objects.requireNonNull(general); - this.context = null; - } - - public ScriptCacheStats(StreamInput in) throws IOException { - boolean isContext = in.readBoolean(); - if (isContext == false) { - general = new ScriptStats(in); - context = null; - return; - } - - general = null; - int size = in.readInt(); - Map context = new HashMap<>(size); - for (int i=0; i < size; i++) { - String name = in.readString(); - context.put(name, new ScriptStats(in)); - } - this.context = Collections.unmodifiableMap(context); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - if (general != null) { - out.writeBoolean(false); - general.writeTo(out); - return; - } - - out.writeBoolean(true); - out.writeInt(context.size()); - for (String name: context.keySet().stream().sorted().collect(Collectors.toList())) { - out.writeString(name); - context.get(name).writeTo(out); - } - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(Fields.SCRIPT_CACHE_STATS); - builder.startObject(Fields.SUM); - if (general != null) { - builder.field(ScriptStats.Fields.COMPILATIONS, general.getCompilations()); - builder.field(ScriptStats.Fields.CACHE_EVICTIONS, general.getCacheEvictions()); - builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, general.getCompilationLimitTriggered()); - builder.endObject().endObject(); - return builder; - } - - ScriptStats sum = sum(); - builder.field(ScriptStats.Fields.COMPILATIONS, sum.getCompilations()); - builder.field(ScriptStats.Fields.CACHE_EVICTIONS, sum.getCacheEvictions()); - builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, sum.getCompilationLimitTriggered()); - builder.endObject(); - - builder.startArray(Fields.CONTEXTS); - for (String name: context.keySet().stream().sorted().collect(Collectors.toList())) { - ScriptStats stats = context.get(name); - builder.startObject(); - builder.field(Fields.CONTEXT, name); - builder.field(ScriptStats.Fields.COMPILATIONS, stats.getCompilations()); - builder.field(ScriptStats.Fields.CACHE_EVICTIONS, stats.getCacheEvictions()); - builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, stats.getCompilationLimitTriggered()); - builder.endObject(); - } - builder.endArray(); - builder.endObject(); - - return builder; - } - - /** - * Get the context specific stats, null if using general cache - */ - public Map getContextStats() { - return context; - } - - /** - * Get the general stats, null if using context cache - */ - public ScriptStats getGeneralStats() { - return general; - } - - /** - * The sum of all script stats, either the general stats or the sum of all stats of the context stats. - */ - public ScriptStats sum() { - if (general != null) { - return general; - } - return ScriptStats.sum(context.values()); - } - - static final class Fields { - static final String SCRIPT_CACHE_STATS = "script_cache"; - static final String CONTEXT = "context"; - static final String SUM = "sum"; - static final String CONTEXTS = "contexts"; - } -} 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..1a1719ad440d5 --- /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..093beeb6ac126 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,12 @@ public void onCacheEviction() { public void onCompilationLimit() { compilationLimitTriggered.inc(); } -} + + 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 5e8c2726825ce..c62b2ad899cde 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -541,10 +541,6 @@ public ScriptStats stats() { return cacheHolder.get().stats(); } - public ScriptCacheStats cacheStats() { - return cacheHolder.get().cacheStats(); - } - @Override public void applyClusterState(ClusterChangedEvent event) { clusterState = event.state(); @@ -604,15 +600,11 @@ ScriptCache get(String context) { } ScriptStats stats() { - return ScriptStats.sum(contextCache.values().stream().map(AtomicReference::get).map(ScriptCache::stats)::iterator); - } - - ScriptCacheStats cacheStats() { - Map context = new HashMap<>(contextCache.size()); - for (String name: contextCache.keySet()) { - context.put(name, contextCache.get(name).get().stats()); + List stats = new ArrayList<>(contextCache.size()); + for (Map.Entry> entry : contextCache.entrySet()) { + stats.add(entry.getValue().get().stats(entry.getKey())); } - return new ScriptCacheStats(context); + return new ScriptStats(stats); } /** diff --git a/server/src/main/java/org/elasticsearch/script/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index 953b3a18ae92c..af4724228b0d2 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -26,13 +26,26 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; public class ScriptStats implements Writeable, ToXContentFragment { + private final List contextStats; private final long compilations; private final long cacheEvictions; private final long compilationLimitTriggered; - public ScriptStats(long compilations, long cacheEvictions, long compilationLimitTriggered) { + + public ScriptStats(List contextStats) { + this.contextStats = contextStats.stream().sorted().collect(Collectors.toUnmodifiableList()); + 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; @@ -42,6 +55,7 @@ public ScriptStats(StreamInput in) throws IOException { compilations = in.readVLong(); cacheEvictions = in.readVLong(); compilationLimitTriggered = in.readVLong(); + contextStats = in.readList(ScriptContextStats::new); } @Override @@ -49,6 +63,11 @@ public void writeTo(StreamOutput out) throws IOException { out.writeVLong(compilations); out.writeVLong(cacheEvictions); out.writeVLong(compilationLimitTriggered); + out.writeList(contextStats); + } + + public List getContextStats() { + return contextStats; } public long getCompilations() { @@ -66,33 +85,23 @@ public long getCompilationLimitTriggered() { @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.startArray(Fields.CONTEXTS); + for (ScriptContextStats contextStats: contextStats) { + contextStats.toXContent(builder, params); + } + builder.endArray(); 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 05f9e764502f7..8ca7cf3b3b702 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 @@ -35,7 +35,7 @@ import org.elasticsearch.monitor.process.ProcessStats; import org.elasticsearch.node.AdaptiveSelectionStats; import org.elasticsearch.node.ResponseCollectorService; -import org.elasticsearch.script.ScriptCacheStats; +import org.elasticsearch.script.ScriptContextStats; import org.elasticsearch.script.ScriptStats; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.VersionUtils; @@ -46,9 +46,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; @@ -241,11 +243,35 @@ public void testSerialization() throws IOException { } } ScriptStats scriptStats = nodeStats.getScriptStats(); + ScriptStats deserializedScriptStats = deserializedNodeStats.getScriptStats(); if (scriptStats == null) { - assertNull(deserializedNodeStats.getScriptStats()); + assertNull(deserializedScriptStats); } else { - assertEquals(scriptStats.getCacheEvictions(), deserializedNodeStats.getScriptStats().getCacheEvictions()); - assertEquals(scriptStats.getCompilations(), deserializedNodeStats.getScriptStats().getCompilations()); + List deserialized = deserializedScriptStats.getContextStats(); + long evictions = 0; + long limited = 0; + long compilations = 0; + List stats = scriptStats.getContextStats(); + for (ScriptContextStats generatedStats: stats) { + List maybeDeserStats = deserialized.stream().filter( + s -> s.getContext().equals(generatedStats.getContext()) + ).collect(Collectors.toList()); + + assertEquals(1, maybeDeserStats.size()); + ScriptContextStats deserStats = maybeDeserStats.get(0); + + evictions += generatedStats.getCacheEvictions(); + assertEquals(generatedStats.getCacheEvictions(), deserStats.getCacheEvictions()); + + limited += generatedStats.getCompilationLimitTriggered(); + assertEquals(generatedStats.getCompilationLimitTriggered(), deserStats.getCompilationLimitTriggered()); + + compilations += generatedStats.getCompilations(); + assertEquals(generatedStats.getCompilations(), deserStats.getCompilations()); + } + assertEquals(evictions, scriptStats.getCacheEvictions()); + assertEquals(limited, scriptStats.getCompilationLimitTriggered()); + assertEquals(compilations, scriptStats.getCompilations()); } DiscoveryStats discoveryStats = nodeStats.getDiscoveryStats(); DiscoveryStats deserializedDiscoveryStats = deserializedNodeStats.getDiscoveryStats(); @@ -312,34 +338,6 @@ public void testSerialization() throws IOException { assertEquals(aStats.responseTime, bStats.responseTime, 0.01); }); } - ScriptCacheStats scriptCacheStats = nodeStats.getScriptCacheStats(); - ScriptCacheStats deserializedScriptCacheStats = deserializedNodeStats.getScriptCacheStats(); - if (scriptCacheStats == null) { - assertNull(deserializedScriptCacheStats); - } else { - Map deserialized = deserializedScriptCacheStats.getContextStats(); - long evictions = 0; - long limited = 0; - long compilations = 0; - Map stats = scriptCacheStats.getContextStats(); - for (String context: stats.keySet()) { - ScriptStats deserStats = deserialized.get(context); - ScriptStats generatedStats = stats.get(context); - - evictions += generatedStats.getCacheEvictions(); - assertEquals(generatedStats.getCacheEvictions(), deserStats.getCacheEvictions()); - - limited += generatedStats.getCompilationLimitTriggered(); - assertEquals(generatedStats.getCompilationLimitTriggered(), deserStats.getCompilationLimitTriggered()); - - compilations += generatedStats.getCompilations(); - assertEquals(generatedStats.getCompilations(), deserStats.getCompilations()); - } - ScriptStats sum = deserializedScriptCacheStats.sum(); - assertEquals(evictions, sum.getCacheEvictions()); - assertEquals(limited, sum.getCompilationLimitTriggered()); - assertEquals(compilations, sum.getCompilations()); - } } } } @@ -454,8 +452,21 @@ public static NodeStats createNodeStats() { } allCircuitBreakerStats = new AllCircuitBreakerStats(circuitBreakerStatsArray); } - ScriptStats scriptStats = frequently() ? - new ScriptStats(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong()) : null; + ScriptStats scriptStats = null; + if (frequently()) { + int numContents = randomIntBetween(0, 20); + List stats = new ArrayList<>(numContents); + HashSet contexts = new HashSet<>(); + for (int i = 0; i < numContents; i++) { + stats.add(new ScriptContextStats( + randomValueOtherThanMany(contexts::contains, () -> randomAlphaOfLength(12)), + randomLongBetween(0, 1024), + randomLongBetween(0, 1024), + randomLongBetween(0, 1024)) + ); + } + scriptStats = new ScriptStats(stats); + } DiscoveryStats discoveryStats = frequently() ? new DiscoveryStats( randomBoolean() @@ -514,20 +525,10 @@ 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); - } //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, - ingestStats, adaptiveSelectionStats, scriptCacheStats); + ingestStats, adaptiveSelectionStats); } private IngestStats.Stats getPipelineStats(List pipelineStats, String id) { diff --git a/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java b/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java index a5674994b61b0..d89cc710e6f9e 100644 --- a/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java @@ -152,14 +152,11 @@ public void testFillDiskUsage() { }; List nodeStats = Arrays.asList( new NodeStats(new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, - null), + null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null), new NodeStats(new DiscoveryNode("node_2", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, - null), + null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null), new NodeStats(new DiscoveryNode("node_3", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, - null) + null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null) ); InternalClusterInfoService.fillDiskUsagePerNode(logger, nodeStats, newLeastAvaiableUsages, newMostAvaiableUsages); DiskUsage leastNode_1 = newLeastAvaiableUsages.get("node_1"); @@ -196,14 +193,11 @@ public void testFillDiskUsageSomeInvalidValues() { }; List nodeStats = Arrays.asList( new NodeStats(new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, - null), + null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null), new NodeStats(new DiscoveryNode("node_2", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, - null), + null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null), new NodeStats(new DiscoveryNode("node_3", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, - null) + null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null) ); InternalClusterInfoService.fillDiskUsagePerNode(logger, nodeStats, newLeastAvailableUsages, newMostAvailableUsages); DiskUsage leastNode_1 = newLeastAvailableUsages.get("node_1"); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 509dfad83e56e..3248df1bbaa05 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -39,10 +39,12 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.stream.Collectors; import static org.elasticsearch.script.ScriptService.MAX_COMPILATION_RATE_FUNCTION; import static org.elasticsearch.script.ScriptService.SCRIPT_CACHE_EXPIRE_SETTING; @@ -240,7 +242,7 @@ public void testIndexedScriptCountedInCompilationStats() throws IOException { ScriptContext ctx = randomFrom(contexts.values()); scriptService.compile(new Script(ScriptType.STORED, null, "script", Collections.emptyMap()), ctx); assertEquals(1L, scriptService.stats().getCompilations()); - assertEquals(1L, scriptService.cacheStats().getContextStats().get(ctx.name).getCompilations()); + assertEquals(1L, getByContext(scriptService.stats(), ctx.name).getCompilations()); } public void testCacheEvictionCountedInCacheEvictionsStats() throws IOException { @@ -293,15 +295,14 @@ public void testContextCacheStats() throws IOException { assertEquals(CircuitBreakingException.class, gse.getRootCause().getClass()); // Context specific - ScriptCacheStats stats = scriptService.cacheStats(); - assertEquals(2L, stats.getContextStats().get(contextA.name).getCompilations()); - assertEquals(1L, stats.getContextStats().get(contextA.name).getCacheEvictions()); - assertEquals(1L, stats.getContextStats().get(contextA.name).getCompilationLimitTriggered()); + ScriptStats stats = scriptService.stats(); + assertEquals(2L, getByContext(stats, contextA.name).getCompilations()); + assertEquals(1L, getByContext(stats, contextA.name).getCacheEvictions()); + assertEquals(1L, getByContext(stats, contextA.name).getCompilationLimitTriggered()); - assertEquals(3L, stats.getContextStats().get(contextB.name).getCompilations()); - assertEquals(1L, stats.getContextStats().get(contextB.name).getCacheEvictions()); - assertEquals(2L, stats.getContextStats().get(contextB.name).getCompilationLimitTriggered()); - assertNull(scriptService.cacheStats().getGeneralStats()); + assertEquals(3L, getByContext(stats, contextB.name).getCompilations()); + assertEquals(1L, getByContext(stats, contextB.name).getCacheEvictions()); + assertEquals(2L, getByContext(stats, contextB.name).getCompilationLimitTriggered()); // Summed up assertEquals(5L, scriptService.stats().getCompilations()); @@ -309,6 +310,13 @@ public void testContextCacheStats() throws IOException { assertEquals(3L, scriptService.stats().getCompilationLimitTriggered()); } + private ScriptContextStats getByContext(ScriptStats stats, String context) { + List maybeContextStats = stats.getContextStats().stream().filter(c -> c.getContext().equals(context)) + .collect(Collectors.toList()); + assertEquals(1, maybeContextStats.size()); + return maybeContextStats.get(0); + } + public void testStoreScript() throws Exception { BytesReference script = BytesReference.bytes(XContentFactory.jsonBuilder() .startObject() diff --git a/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java new file mode 100644 index 0000000000000..9ce212d92c56b --- /dev/null +++ b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java @@ -0,0 +1,68 @@ +/* + * 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.Strings; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; + +public class ScriptStatsTests extends ESTestCase { + public void testXContent() throws IOException { + List contextStats = List.of( + new ScriptContextStats("contextB", 100, 201, 302), + new ScriptContextStats("contextA", 1000, 2010, 3020) + ); + ScriptStats stats = new ScriptStats(contextStats); + final XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); + builder.startObject(); + stats.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + + String expected = "{\n" + + " \"script\" : {\n" + + " \"compilations\" : 1100,\n" + + " \"cache_evictions\" : 2211,\n" + + " \"compilation_limit_triggered\" : 3322,\n" + + " \"contexts\" : [\n" + + " {\n" + + " \"context\" : \"contextA\",\n" + + " \"compilations\" : 1000,\n" + + " \"cache_evictions\" : 2010,\n" + + " \"compilation_limit_triggered\" : 3020\n" + + " },\n" + + " {\n" + + " \"context\" : \"contextB\",\n" + + " \"compilations\" : 100,\n" + + " \"cache_evictions\" : 201,\n" + + " \"compilation_limit_triggered\" : 302\n" + + " }\n" + + " ]\n" + + " }\n" + + "}"; + assertThat(Strings.toString(builder), equalTo(expected)); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java b/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java index 9b0181f355005..9a4ae5022319f 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java @@ -84,7 +84,7 @@ List adjustNodesStats(List nodesStats) { .map(fsInfoPath -> diskUsageFunction.apply(discoveryNode, fsInfoPath)) .toArray(FsInfo.Path[]::new)), nodeStats.getTransport(), nodeStats.getHttp(), nodeStats.getBreaker(), nodeStats.getScriptStats(), nodeStats.getDiscoveryStats(), - nodeStats.getIngestStats(), nodeStats.getAdaptiveSelectionStats(), nodeStats.getScriptCacheStats()); + nodeStats.getIngestStats(), nodeStats.getAdaptiveSelectionStats()); }).collect(Collectors.toList()); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java index b7641e4906933..e890520580e2f 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java @@ -610,7 +610,7 @@ private static NodeStats buildNodeStats(List pipelineNames, List