From dda33155d6beda8a7f04406d293084c5af68c0d4 Mon Sep 17 00:00:00 2001 From: Alex Ksikes Date: Mon, 15 Dec 2014 11:02:41 +0100 Subject: [PATCH] Indices API: Fix wrong search stats groups This provides a fix to issue #7644. A new Stats object must be created, and not a reference to the retrieved stats, before we can add stats to it. Otherwise, we would keep on adding to the same object on subsequent calls to IndicesStatsResponse#getPrimaries() or IndicesStatsResponse#getTotal(). Closes #7644 and #8950 --- .../index/search/stats/SearchStats.java | 6 +- .../search/stats/SearchStatsUnitTests.java | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java diff --git a/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java b/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java index 5021e57ce7a..44d93230401 100644 --- a/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java +++ b/src/main/java/org/elasticsearch/index/search/stats/SearchStats.java @@ -60,6 +60,10 @@ public class SearchStats implements Streamable, ToXContent { this.fetchCurrent = fetchCurrent; } + public Stats(Stats stats) { + this(stats.queryCount, stats.queryTimeInMillis, stats.queryCurrent, stats.fetchCount, stats.fetchTimeInMillis, stats.fetchCurrent); + } + public void add(Stats stats) { queryCount += stats.queryCount; queryTimeInMillis += stats.queryTimeInMillis; @@ -178,7 +182,7 @@ public class SearchStats implements Streamable, ToXContent { for (Map.Entry entry : searchStats.groupStats.entrySet()) { Stats stats = groupStats.get(entry.getKey()); if (stats == null) { - groupStats.put(entry.getKey(), entry.getValue()); + groupStats.put(entry.getKey(), new Stats(entry.getValue())); } else { stats.add(entry.getValue()); } diff --git a/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java b/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java new file mode 100644 index 00000000000..a9f134b1be6 --- /dev/null +++ b/src/test/java/org/elasticsearch/search/stats/SearchStatsUnitTests.java @@ -0,0 +1,66 @@ +/* + * 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.search.stats; + +import org.elasticsearch.index.search.stats.SearchStats; +import org.elasticsearch.index.search.stats.SearchStats.Stats; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +public class SearchStatsUnitTests extends ElasticsearchTestCase { + + @Test + // https://github.com/elasticsearch/elasticsearch/issues/7644 + public void testShardLevelSearchGroupStats() throws Exception { + // let's create two dummy search stats with groups + Map groupStats1 = new HashMap<>(); + Map groupStats2 = new HashMap<>(); + groupStats2.put("group1", new Stats(1, 1, 1, 1, 1, 1)); + SearchStats searchStats1 = new SearchStats(new Stats(1, 1, 1, 1, 1, 1), 0, groupStats1); + SearchStats searchStats2 = new SearchStats(new Stats(1, 1, 1, 1, 1, 1), 0, groupStats2); + + // adding these two search stats and checking group stats are correct + searchStats1.add(searchStats2); + assertStats(groupStats1.get("group1"), 1); + + // another call, adding again ... + searchStats1.add(searchStats2); + assertStats(groupStats1.get("group1"), 2); + + // making sure stats2 was not affected (this would previously return 2!) + assertStats(groupStats2.get("group1"), 1); + + // adding again would then return wrong search stats (would return 4! instead of 3) + searchStats1.add(searchStats2); + assertStats(groupStats1.get("group1"), 3); + } + + private void assertStats(Stats stats, long equalTo) { + assertEquals(equalTo, stats.getQueryCount()); + assertEquals(equalTo, stats.getQueryTimeInMillis()); + assertEquals(equalTo, stats.getQueryCurrent()); + assertEquals(equalTo, stats.getFetchCount()); + assertEquals(equalTo, stats.getFetchTimeInMillis()); + assertEquals(equalTo, stats.getFetchCurrent()); + } +} \ No newline at end of file