From e7375368d6e365343f8b21a09b6baef8ff61d304 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Sat, 13 Apr 2019 04:36:29 +1000 Subject: [PATCH] Remove nested loop in IndicesStatsResponse (#40988) (#41138) This commit removes nested loop in `getIndices`. --- .../admin/indices/stats/IndexStats.java | 20 ++++++ .../indices/stats/IndicesStatsResponse.java | 28 +++----- .../stats/IndicesStatsResponseTests.java | 71 ++++++++++++++++++- 3 files changed, 99 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndexStats.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndexStats.java index a36821a4b65..c66cbc38850 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndexStats.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndexStats.java @@ -108,4 +108,24 @@ public class IndexStats implements Iterable { primary = stats; return stats; } + + public static class IndexStatsBuilder { + private final String indexName; + private final String uuid; + private final List shards = new ArrayList<>(); + + public IndexStatsBuilder(String indexName, String uuid) { + this.indexName = indexName; + this.uuid = uuid; + } + + public IndexStatsBuilder add(ShardStats shardStats) { + shards.add(shardStats); + return this; + } + + public IndexStats build() { + return new IndexStats(indexName, uuid, shards.toArray(new ShardStats[shards.size()])); + } + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java index cc563948160..0540bc3ad5c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.stats; +import org.elasticsearch.action.admin.indices.stats.IndexStats.IndexStatsBuilder; import org.elasticsearch.action.support.DefaultShardOperationFailedException; import org.elasticsearch.action.support.broadcast.BroadcastResponse; import org.elasticsearch.cluster.routing.ShardRouting; @@ -29,12 +30,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.Index; import java.io.IOException; -import java.util.ArrayList; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; +import java.util.stream.Collectors; import static java.util.Collections.unmodifiableMap; @@ -83,26 +82,17 @@ public class IndicesStatsResponse extends BroadcastResponse { if (indicesStats != null) { return indicesStats; } - Map indicesStats = new HashMap<>(); - Set indices = new HashSet<>(); + final Map indexToIndexStatsBuilder = new HashMap<>(); for (ShardStats shard : shards) { - indices.add(shard.getShardRouting().index()); + Index index = shard.getShardRouting().index(); + IndexStatsBuilder indexStatsBuilder = indexToIndexStatsBuilder.computeIfAbsent(index.getName(), + k -> new IndexStatsBuilder(k, index.getUUID())); + indexStatsBuilder.add(shard); } - for (Index index : indices) { - List shards = new ArrayList<>(); - String indexName = index.getName(); - for (ShardStats shard : this.shards) { - if (shard.getShardRouting().getIndexName().equals(indexName)) { - shards.add(shard); - } - } - indicesStats.put( - indexName, new IndexStats(indexName, index.getUUID(), shards.toArray(new ShardStats[shards.size()])) - ); - } - this.indicesStats = indicesStats; + indicesStats = indexToIndexStatsBuilder.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().build())); return indicesStats; } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponseTests.java index a7e3ee57a08..99850699ec2 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponseTests.java @@ -19,16 +19,30 @@ package org.elasticsearch.action.admin.indices.stats; +import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.ShardRoutingState; +import org.elasticsearch.cluster.routing.TestShardRouting; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.Index; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.index.shard.ShardPath; import org.elasticsearch.test.ESTestCase; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.is; import static org.hamcrest.object.HasToString.hasToString; - public class IndicesStatsResponseTests extends ESTestCase { public void testInvalidLevel() { @@ -42,4 +56,59 @@ public class IndicesStatsResponseTests extends ESTestCase { hasToString(containsString("level parameter must be one of [cluster] or [indices] or [shards] but was [" + level + "]"))); } + public void testGetIndices() { + List shards = new ArrayList<>(); + int noOfIndexes = randomIntBetween(2, 5); + List expectedIndexes = new ArrayList<>(); + Map expectedIndexToPrimaryShardsCount = new HashMap<>(); + Map expectedIndexToTotalShardsCount = new HashMap<>(); + + for (int indCnt = 0; indCnt < noOfIndexes; indCnt++) { + Index index = createIndex(randomAlphaOfLength(9)); + expectedIndexes.add(index.getName()); + int numShards = randomIntBetween(1, 5); + for (int shardId = 0; shardId < numShards; shardId++) { + ShardId shId = new ShardId(index, shardId); + Path path = createTempDir().resolve("indices").resolve(index.getUUID()).resolve(String.valueOf(shardId)); + ShardPath shardPath = new ShardPath(false, path, path, shId); + ShardRouting routing = createShardRouting(index, shId, (shardId == 0)); + shards.add(new ShardStats(routing, shardPath, null, null, null, null)); + AtomicLong primaryShardsCounter = expectedIndexToPrimaryShardsCount.computeIfAbsent(index.getName(), + k -> new AtomicLong(0L)); + if (routing.primary()) { + primaryShardsCounter.incrementAndGet(); + } + AtomicLong shardsCounter = expectedIndexToTotalShardsCount.computeIfAbsent(index.getName(), k -> new AtomicLong(0L)); + shardsCounter.incrementAndGet(); + } + } + final IndicesStatsResponse indicesStatsResponse = new IndicesStatsResponse(shards.toArray(new ShardStats[shards.size()]), 0, 0, 0, + null); + Map indexStats = indicesStatsResponse.getIndices(); + + assertThat(indexStats.size(), is(noOfIndexes)); + assertThat(indexStats.keySet(), containsInAnyOrder(expectedIndexes.toArray(new String[0]))); + + for (String index : indexStats.keySet()) { + IndexStats stat = indexStats.get(index); + ShardStats[] shardStats = stat.getShards(); + long primaryCount = 0L; + long totalCount = shardStats.length; + for (ShardStats shardStat : shardStats) { + if (shardStat.getShardRouting().primary()) { + primaryCount++; + } + } + assertThat(primaryCount, is(expectedIndexToPrimaryShardsCount.get(index).get())); + assertThat(totalCount, is(expectedIndexToTotalShardsCount.get(index).get())); + } + } + + private ShardRouting createShardRouting(Index index, ShardId shardId, boolean isPrimary) { + return TestShardRouting.newShardRouting(shardId, randomAlphaOfLength(4), isPrimary, ShardRoutingState.STARTED); + } + + private Index createIndex(String indexName) { + return new Index(indexName, UUIDs.base64UUID()); + } }