From 462068ee4108908ea590ba5678282f1c3c2be8ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 27 Mar 2018 15:21:11 +0200 Subject: [PATCH] Make SearchStats implement Writeable (#29258) Moves another class over from Streamable to Writeable. By this, also some constructors can be removed or made private. --- .../admin/indices/stats/CommonStats.java | 4 +- .../index/search/stats/SearchStats.java | 95 ++++++++----------- .../search/stats/SearchStatsTests.java} | 9 +- 3 files changed, 44 insertions(+), 64 deletions(-) rename server/src/test/java/org/elasticsearch/{search/stats/SearchStatsUnitTests.java => index/search/stats/SearchStatsTests.java} (93%) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/CommonStats.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/CommonStats.java index 71360c359d311..6379f8da21aa2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/CommonStats.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/CommonStats.java @@ -233,7 +233,7 @@ public CommonStats(StreamInput in) throws IOException { store = in.readOptionalStreamable(StoreStats::new); indexing = in.readOptionalStreamable(IndexingStats::new); get = in.readOptionalStreamable(GetStats::new); - search = in.readOptionalStreamable(SearchStats::new); + search = in.readOptionalWriteable(SearchStats::new); merge = in.readOptionalStreamable(MergeStats::new); refresh = in.readOptionalStreamable(RefreshStats::new); flush = in.readOptionalStreamable(FlushStats::new); @@ -253,7 +253,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalStreamable(store); out.writeOptionalStreamable(indexing); out.writeOptionalStreamable(get); - out.writeOptionalStreamable(search); + out.writeOptionalWriteable(search); out.writeOptionalStreamable(merge); out.writeOptionalStreamable(refresh); out.writeOptionalStreamable(flush); diff --git a/server/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java b/server/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java index 519cd9ff9ae71..5f514b89b64a2 100644 --- a/server/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java +++ b/server/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java @@ -23,19 +23,20 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Streamable; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; 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; -public class SearchStats implements Streamable, ToXContentFragment { +public class SearchStats implements Writeable, ToXContentFragment { - public static class Stats implements Streamable, ToXContentFragment { + public static class Stats implements Writeable, ToXContentFragment { private long queryCount; private long queryTimeInMillis; @@ -53,8 +54,8 @@ public static class Stats implements Streamable, ToXContentFragment { private long suggestTimeInMillis; private long suggestCurrent; - Stats() { - + private Stats() { + // for internal use, initializes all counts to 0 } public Stats( @@ -78,16 +79,24 @@ public Stats( this.suggestCount = suggestCount; this.suggestTimeInMillis = suggestTimeInMillis; this.suggestCurrent = suggestCurrent; - } - public Stats(Stats stats) { - this( - stats.queryCount, stats.queryTimeInMillis, stats.queryCurrent, - stats.fetchCount, stats.fetchTimeInMillis, stats.fetchCurrent, - stats.scrollCount, stats.scrollTimeInMillis, stats.scrollCurrent, - stats.suggestCount, stats.suggestTimeInMillis, stats.suggestCurrent - ); + private Stats(StreamInput in) throws IOException { + queryCount = in.readVLong(); + queryTimeInMillis = in.readVLong(); + queryCurrent = in.readVLong(); + + fetchCount = in.readVLong(); + fetchTimeInMillis = in.readVLong(); + fetchCurrent = in.readVLong(); + + scrollCount = in.readVLong(); + scrollTimeInMillis = in.readVLong(); + scrollCurrent = in.readVLong(); + + suggestCount = in.readVLong(); + suggestTimeInMillis = in.readVLong(); + suggestCurrent = in.readVLong(); } public void add(Stats stats) { @@ -173,28 +182,7 @@ public long getSuggestCurrent() { } public static Stats readStats(StreamInput in) throws IOException { - Stats stats = new Stats(); - stats.readFrom(in); - return stats; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - queryCount = in.readVLong(); - queryTimeInMillis = in.readVLong(); - queryCurrent = in.readVLong(); - - fetchCount = in.readVLong(); - fetchTimeInMillis = in.readVLong(); - fetchCurrent = in.readVLong(); - - scrollCount = in.readVLong(); - scrollTimeInMillis = in.readVLong(); - scrollCurrent = in.readVLong(); - - suggestCount = in.readVLong(); - suggestTimeInMillis = in.readVLong(); - suggestCurrent = in.readVLong(); + return new Stats(in); } @Override @@ -238,11 +226,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } } - Stats totalStats; - long openContexts; + private final Stats totalStats; + private long openContexts; @Nullable - Map groupStats; + private Map groupStats; public SearchStats() { totalStats = new Stats(); @@ -254,27 +242,27 @@ public SearchStats(Stats totalStats, long openContexts, @Nullable Map(searchStats.groupStats.size()); } for (Map.Entry entry : searchStats.groupStats.entrySet()) { - Stats stats = groupStats.get(entry.getKey()); - if (stats == null) { - groupStats.put(entry.getKey(), new Stats(entry.getValue())); - } else { - stats.add(entry.getValue()); - } + groupStats.putIfAbsent(entry.getKey(), new Stats()); + groupStats.get(entry.getKey()).add(entry.getValue()); } } } @@ -296,7 +284,7 @@ public long getOpenContexts() { @Nullable public Map getGroupStats() { - return this.groupStats; + return this.groupStats != null ? Collections.unmodifiableMap(this.groupStats) : null; } @Override @@ -344,15 +332,6 @@ static final class Fields { static final String SUGGEST_CURRENT = "suggest_current"; } - @Override - public void readFrom(StreamInput in) throws IOException { - totalStats = Stats.readStats(in); - openContexts = in.readVLong(); - if (in.readBoolean()) { - groupStats = in.readMap(StreamInput::readString, Stats::readStats); - } - } - @Override public void writeTo(StreamOutput out) throws IOException { totalStats.writeTo(out); diff --git a/server/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java b/server/src/test/java/org/elasticsearch/index/search/stats/SearchStatsTests.java similarity index 93% rename from server/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java rename to server/src/test/java/org/elasticsearch/index/search/stats/SearchStatsTests.java index 15fa7e64e3f67..5ec7aeaa0b2be 100644 --- a/server/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java +++ b/server/src/test/java/org/elasticsearch/index/search/stats/SearchStatsTests.java @@ -17,16 +17,16 @@ * under the License. */ -package org.elasticsearch.search.stats; +package org.elasticsearch.index.search.stats; -import org.elasticsearch.index.search.stats.SearchStats; import org.elasticsearch.index.search.stats.SearchStats.Stats; import org.elasticsearch.test.ESTestCase; import java.util.HashMap; import java.util.Map; -public class SearchStatsUnitTests extends ESTestCase { +public class SearchStatsTests extends ESTestCase { + // https://github.com/elastic/elasticsearch/issues/7644 public void testShardLevelSearchGroupStats() throws Exception { // let's create two dummy search stats with groups @@ -52,7 +52,7 @@ public void testShardLevelSearchGroupStats() throws Exception { assertStats(groupStats1.get("group1"), 3); } - private void assertStats(Stats stats, long equalTo) { + private static void assertStats(Stats stats, long equalTo) { assertEquals(equalTo, stats.getQueryCount()); assertEquals(equalTo, stats.getQueryTimeInMillis()); assertEquals(equalTo, stats.getQueryCurrent()); @@ -66,4 +66,5 @@ private void assertStats(Stats stats, long equalTo) { assertEquals(equalTo, stats.getSuggestTimeInMillis()); assertEquals(equalTo, stats.getSuggestCurrent()); } + }