From 02611f83d4a46878053f4e38ee0796f3b7356be4 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 12 Nov 2025 13:34:55 -0600 Subject: [PATCH] Revert "Improving statsByShard performance when the number of shards is very large (#130857)" (#137973) This reverts commit 22c15bce94ee6435a3eed1375e306fb0cac6eccb. --- docs/changelog/130857.yaml | 6 - .../stats/TransportClusterStatsAction.java | 11 +- .../admin/indices/stats/CommonStats.java | 9 +- .../stats/TransportIndicesStatsAction.java | 9 +- .../indices/IndicesQueryCache.java | 117 +++++----------- .../elasticsearch/indices/IndicesService.java | 19 ++- .../cluster/stats/VersionStatsTests.java | 2 +- .../index/shard/IndexShardTests.java | 2 +- .../indices/IndicesQueryCacheTests.java | 129 +++++------------- .../indices/IndicesServiceCloseTests.java | 14 +- .../indices/IndicesServiceTests.java | 10 +- 11 files changed, 95 insertions(+), 233 deletions(-) delete mode 100644 docs/changelog/130857.yaml diff --git a/docs/changelog/130857.yaml b/docs/changelog/130857.yaml deleted file mode 100644 index 5e5a5a309dfeb..0000000000000 --- a/docs/changelog/130857.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 130857 -summary: Improving statsByShard performance when the number of shards is very large -area: Stats -type: bug -issues: - - 97222 diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/TransportClusterStatsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/TransportClusterStatsAction.java index 9cc73f302a7a9..94936a3c1b320 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/TransportClusterStatsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/stats/TransportClusterStatsAction.java @@ -48,8 +48,6 @@ import org.elasticsearch.index.seqno.RetentionLeaseStats; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.shard.IndexShard; -import org.elasticsearch.index.shard.ShardId; -import org.elasticsearch.indices.IndicesQueryCache; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.node.NodeService; @@ -262,12 +260,9 @@ protected ClusterStatsNodeResponse nodeOperation(ClusterStatsNodeRequest nodeReq false, false ); - Map shardIdToSharedRam = IndicesQueryCache.getSharedRamSizeForAllShards(indicesService); List shardsStats = new ArrayList<>(); for (IndexService indexService : indicesService) { for (IndexShard indexShard : indexService) { - // get the shared ram for this shard id (or zero if there's nothing in the map) - long sharedRam = shardIdToSharedRam.getOrDefault(indexShard.shardId(), 0L); cancellableTask.ensureNotCancelled(); if (indexShard.routingEntry() != null && indexShard.routingEntry().active()) { // only report on fully started shards @@ -288,7 +283,7 @@ protected ClusterStatsNodeResponse nodeOperation(ClusterStatsNodeRequest nodeReq new ShardStats( indexShard.routingEntry(), indexShard.shardPath(), - CommonStats.getShardLevelStats(indicesService.getIndicesQueryCache(), indexShard, SHARD_STATS_FLAGS, sharedRam), + CommonStats.getShardLevelStats(indicesService.getIndicesQueryCache(), indexShard, SHARD_STATS_FLAGS), commitStats, seqNoStats, retentionLeaseStats, @@ -319,7 +314,7 @@ protected ClusterStatsNodeResponse nodeOperation(ClusterStatsNodeRequest nodeReq clusterStatus, nodeInfo, nodeStats, - shardsStats.toArray(new ShardStats[0]), + shardsStats.toArray(new ShardStats[shardsStats.size()]), searchUsageStats, repositoryUsageStats, ccsTelemetry, @@ -481,7 +476,7 @@ protected void sendItemRequest(String clusterAlias, ActionListener v.acceptResponse(response)); + remoteClustersStats.computeIfPresent(clusterAlias, (k, v) -> v.acceptResponse(response)); } } 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 bb3da1850d96f..0fba9bdcd0a92 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 @@ -154,12 +154,7 @@ public CommonStats(CommonStatsFlags flags) { /** * Filters the given flags for {@link CommonStatsFlags#SHARD_LEVEL} flags and calculates the corresponding statistics. */ - public static CommonStats getShardLevelStats( - IndicesQueryCache indicesQueryCache, - IndexShard indexShard, - CommonStatsFlags flags, - long precomputedSharedRam - ) { + public static CommonStats getShardLevelStats(IndicesQueryCache indicesQueryCache, IndexShard indexShard, CommonStatsFlags flags) { // Filter shard level flags CommonStatsFlags filteredFlags = flags.clone(); for (CommonStatsFlags.Flag flag : filteredFlags.getFlags()) { @@ -179,7 +174,7 @@ public static CommonStats getShardLevelStats( case Refresh -> stats.refresh = indexShard.refreshStats(); case Flush -> stats.flush = indexShard.flushStats(); case Warmer -> stats.warmer = indexShard.warmerStats(); - case QueryCache -> stats.queryCache = indicesQueryCache.getStats(indexShard.shardId(), precomputedSharedRam); + case QueryCache -> stats.queryCache = indicesQueryCache.getStats(indexShard.shardId()); case FieldData -> stats.fieldData = indexShard.fieldDataStats(flags.fieldDataFields()); case Completion -> stats.completion = indexShard.completionStats(flags.completionDataFields()); case Segments -> stats.segments = indexShard.segmentStats( diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/TransportIndicesStatsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/TransportIndicesStatsAction.java index ba59e3e5f1c0b..210fb42ca584b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/stats/TransportIndicesStatsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/stats/TransportIndicesStatsAction.java @@ -27,7 +27,6 @@ import org.elasticsearch.index.seqno.RetentionLeaseStats; import org.elasticsearch.index.seqno.SeqNoStats; import org.elasticsearch.index.shard.IndexShard; -import org.elasticsearch.indices.IndicesQueryCache; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.CancellableTask; @@ -115,13 +114,7 @@ protected void shardOperation(IndicesStatsRequest request, ShardRouting shardRou assert task instanceof CancellableTask; IndexService indexService = indicesService.indexServiceSafe(shardRouting.shardId().getIndex()); IndexShard indexShard = indexService.getShard(shardRouting.shardId().id()); - long sharedRam = IndicesQueryCache.getSharedRamSizeForShard(indicesService, indexShard.shardId()); - CommonStats commonStats = CommonStats.getShardLevelStats( - indicesService.getIndicesQueryCache(), - indexShard, - request.flags(), - sharedRam - ); + CommonStats commonStats = CommonStats.getShardLevelStats(indicesService.getIndicesQueryCache(), indexShard, request.flags()); CommitStats commitStats; SeqNoStats seqNoStats; RetentionLeaseStats retentionLeaseStats; diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java index 11af6b2e2d5ba..6c4694b4f8235 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java @@ -26,15 +26,12 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Predicates; -import org.elasticsearch.index.IndexService; import org.elasticsearch.index.cache.query.QueryCacheStats; -import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import java.io.Closeable; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; @@ -70,38 +67,6 @@ public class IndicesQueryCache implements QueryCache, Closeable { private final Map shardStats = new ConcurrentHashMap<>(); private volatile long sharedRamBytesUsed; - /** - * Calculates a map of {@link ShardId} to {@link Long} which contains the calculated share of the {@link IndicesQueryCache} shared ram - * size for a given shard (that is, the sum of all the longs is the size of the indices query cache). Since many shards will not - * participate in the cache, shards whose calculated share is zero will not be contained in the map at all. As a consequence, the - * correct pattern for using the returned map will be via {@link Map#getOrDefault(Object, Object)} with a {@code defaultValue} of - * {@code 0L}. - */ - public static Map getSharedRamSizeForAllShards(IndicesService indicesService) { - Map shardIdToSharedRam = new HashMap<>(); - IndicesQueryCache.CacheTotals cacheTotals = IndicesQueryCache.getCacheTotalsForAllShards(indicesService); - for (IndexService indexService : indicesService) { - for (IndexShard indexShard : indexService) { - final var queryCache = indicesService.getIndicesQueryCache(); - long sharedRam = (queryCache == null) ? 0L : queryCache.getSharedRamSizeForShard(indexShard.shardId(), cacheTotals); - // as a size optimization, only store non-zero values in the map - if (sharedRam > 0L) { - shardIdToSharedRam.put(indexShard.shardId(), sharedRam); - } - } - } - return Collections.unmodifiableMap(shardIdToSharedRam); - } - - public long getCacheSizeForShard(ShardId shardId) { - Stats stats = shardStats.get(shardId); - return stats != null ? stats.cacheSize : 0L; - } - - public long getSharedRamBytesUsed() { - return sharedRamBytesUsed; - } - // This is a hack for the fact that the close listener for the // ShardCoreKeyMap will be called before onDocIdSetEviction // See onDocIdSetEviction for more info @@ -124,58 +89,40 @@ private static QueryCacheStats toQueryCacheStatsSafe(@Nullable Stats stats) { return stats == null ? new QueryCacheStats() : stats.toQueryCacheStats(); } - /** - * This computes the total cache size in bytes, and the total shard count in the cache for all shards. - * @param indicesService - * @return A CacheTotals object containing the computed total number of items in the cache and the number of shards seen in the cache - */ - private static CacheTotals getCacheTotalsForAllShards(IndicesService indicesService) { - IndicesQueryCache queryCache = indicesService.getIndicesQueryCache(); - boolean hasQueryCache = queryCache != null; + private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { + if (sharedRamBytesUsed == 0L) { + return 0L; + } + + /* + * We have some shared ram usage that we try to distribute proportionally to the number of segment-requests in the cache for each + * shard. + */ + // TODO avoid looping over all local shards here - see https://github.com/elastic/elasticsearch/issues/97222 long totalItemsInCache = 0L; int shardCount = 0; - for (final IndexService indexService : indicesService) { - for (final IndexShard indexShard : indexService) { - final var shardId = indexShard.shardId(); - long cacheSize = hasQueryCache ? queryCache.getCacheSizeForShard(shardId) : 0L; - shardCount++; - assert cacheSize >= 0 : "Unexpected cache size of " + cacheSize + " for shard " + shardId; - totalItemsInCache += cacheSize; + if (itemsInCacheForShard == 0L) { + for (final var stats : shardStats.values()) { + shardCount += 1; + if (stats.cacheSize > 0L) { + // some shard has nonzero cache footprint, so we apportion the shared size by cache footprint, and this shard has none + return 0L; + } + } + } else { + // branchless loop for the common case + for (final var stats : shardStats.values()) { + shardCount += 1; + totalItemsInCache += stats.cacheSize; } - } - return new CacheTotals(totalItemsInCache, shardCount); - } - - public static long getSharedRamSizeForShard(IndicesService indicesService, ShardId shardId) { - IndicesQueryCache.CacheTotals cacheTotals = IndicesQueryCache.getCacheTotalsForAllShards(indicesService); - final var queryCache = indicesService.getIndicesQueryCache(); - return (queryCache == null) ? 0L : queryCache.getSharedRamSizeForShard(shardId, cacheTotals); - } - - /** - * This method computes the shared RAM size in bytes for the given indexShard. - * @param shardId The shard to compute the shared RAM size for - * @param cacheTotals Shard totals computed in getCacheTotalsForAllShards() - * @return the shared RAM size in bytes allocated to the given shard, or 0 if unavailable - */ - private long getSharedRamSizeForShard(ShardId shardId, CacheTotals cacheTotals) { - long sharedRamBytesUsed = getSharedRamBytesUsed(); - if (sharedRamBytesUsed == 0L) { - return 0L; } - int shardCount = cacheTotals.shardCount(); if (shardCount == 0) { // Sometimes it's not possible to do this when there are no shard entries at all, which can happen as the shared ram usage can // extend beyond the closing of all shards. return 0L; } - /* - * We have some shared ram usage that we try to distribute proportionally to the number of segment-requests in the cache for each - * shard. - */ - long totalItemsInCache = cacheTotals.totalItemsInCache(); - long itemsInCacheForShard = getCacheSizeForShard(shardId); + final long additionalRamBytesUsed; if (totalItemsInCache == 0) { // all shards have zero cache footprint, so we apportion the size of the shared bytes equally across all shards @@ -196,12 +143,10 @@ private long getSharedRamSizeForShard(ShardId shardId, CacheTotals cacheTotals) return additionalRamBytesUsed; } - private record CacheTotals(long totalItemsInCache, int shardCount) {} - /** Get usage statistics for the given shard. */ - public QueryCacheStats getStats(ShardId shard, long precomputedSharedRamBytesUsed) { + public QueryCacheStats getStats(ShardId shard) { final QueryCacheStats queryCacheStats = toQueryCacheStatsSafe(shardStats.get(shard)); - queryCacheStats.addRamBytesUsed(precomputedSharedRamBytesUsed); + queryCacheStats.addRamBytesUsed(getShareOfAdditionalRamBytesUsed(queryCacheStats.getCacheSize())); return queryCacheStats; } @@ -298,7 +243,7 @@ QueryCacheStats toQueryCacheStats() { public String toString() { return "{shardId=" + shardId - + ", ramBytesUsed=" + + ", ramBytedUsed=" + ramBytesUsed + ", hitCount=" + hitCount @@ -395,7 +340,11 @@ protected void onDocIdSetCache(Object readerCoreKey, long ramBytesUsed) { shardStats.cacheCount += 1; shardStats.ramBytesUsed += ramBytesUsed; - StatsAndCount statsAndCount = stats2.computeIfAbsent(readerCoreKey, ignored -> new StatsAndCount(shardStats)); + StatsAndCount statsAndCount = stats2.get(readerCoreKey); + if (statsAndCount == null) { + statsAndCount = new StatsAndCount(shardStats); + stats2.put(readerCoreKey, statsAndCount); + } statsAndCount.count += 1; } @@ -408,7 +357,7 @@ protected void onDocIdSetEviction(Object readerCoreKey, int numEntries, long sum if (numEntries > 0) { // We can't use ShardCoreKeyMap here because its core closed // listener is called before the listener of the cache which - // triggers this eviction. So instead we use stats2 that + // triggers this eviction. So instead we use use stats2 that // we only evict when nothing is cached anymore on the segment // instead of relying on close listeners final StatsAndCount statsAndCount = stats2.get(readerCoreKey); diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index c3393f86886f1..9c7f1d277bf9d 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -520,36 +520,33 @@ static Map statsByIndex(final IndicesService indicesService, } static Map> statsByShard(final IndicesService indicesService, final CommonStatsFlags flags) { - Map shardIdToSharedRam = IndicesQueryCache.getSharedRamSizeForAllShards(indicesService); final Map> statsByShard = new HashMap<>(); + for (final IndexService indexService : indicesService) { for (final IndexShard indexShard : indexService) { - // get the shared ram for this shard id (or zero if there's nothing in the map) - long sharedRam = shardIdToSharedRam.getOrDefault(indexShard.shardId(), 0L); try { - final IndexShardStats indexShardStats = indicesService.indexShardStats(indicesService, indexShard, flags, sharedRam); + final IndexShardStats indexShardStats = indicesService.indexShardStats(indicesService, indexShard, flags); + if (indexShardStats == null) { continue; } + if (statsByShard.containsKey(indexService.index()) == false) { statsByShard.put(indexService.index(), arrayAsArrayList(indexShardStats)); } else { statsByShard.get(indexService.index()).add(indexShardStats); } } catch (IllegalIndexShardStateException | AlreadyClosedException e) { + // we can safely ignore illegal state on ones that are closing for example logger.trace(() -> format("%s ignoring shard stats", indexShard.shardId()), e); } } } + return statsByShard; } - IndexShardStats indexShardStats( - final IndicesService indicesService, - final IndexShard indexShard, - final CommonStatsFlags flags, - final long precomputedSharedRam - ) { + IndexShardStats indexShardStats(final IndicesService indicesService, final IndexShard indexShard, final CommonStatsFlags flags) { if (indexShard.routingEntry() == null) { return null; } @@ -574,7 +571,7 @@ IndexShardStats indexShardStats( new ShardStats( indexShard.routingEntry(), indexShard.shardPath(), - CommonStats.getShardLevelStats(indicesService.getIndicesQueryCache(), indexShard, flags, precomputedSharedRam), + CommonStats.getShardLevelStats(indicesService.getIndicesQueryCache(), indexShard, flags), commitStats, seqNoStats, retentionLeaseStats, diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/VersionStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/VersionStatsTests.java index 2ab6c89ad265d..fbd6e0916eefe 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/VersionStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/VersionStatsTests.java @@ -115,7 +115,7 @@ public void testCreation() { ShardStats shardStats = new ShardStats( shardRouting, new ShardPath(false, path, path, shardRouting.shardId()), - CommonStats.getShardLevelStats(null, indexShard, new CommonStatsFlags(CommonStatsFlags.Flag.Store), 0L), + CommonStats.getShardLevelStats(null, indexShard, new CommonStatsFlags(CommonStatsFlags.Flag.Store)), null, null, null, diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 61118c8cfb072..25ee257c3eeb5 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -1626,7 +1626,7 @@ public void testShardStats() throws IOException { ShardStats stats = new ShardStats( shard.routingEntry(), shard.shardPath(), - CommonStats.getShardLevelStats(new IndicesQueryCache(Settings.EMPTY), shard, new CommonStatsFlags(), 0L), + CommonStats.getShardLevelStats(new IndicesQueryCache(Settings.EMPTY), shard, new CommonStatsFlags()), shard.commitStats(), shard.seqNoStats(), shard.getRetentionLeaseStats(), diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index bad8723e90073..5629fe81511c6 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -30,11 +30,8 @@ import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.IOUtils; -import org.elasticsearch.core.Tuple; -import org.elasticsearch.index.IndexService; import org.elasticsearch.index.cache.query.QueryCacheStats; import org.elasticsearch.index.cache.query.TrivialQueryCachingPolicy; -import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; @@ -47,8 +44,6 @@ import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.lessThan; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class IndicesQueryCacheTests extends ESTestCase { @@ -61,6 +56,10 @@ private static class DummyQuery extends Query implements Accountable { this(Integer.toString(id), 10); } + DummyQuery(String id) { + this(id, 10); + } + DummyQuery(String id, long sizeInCache) { this.id = id; this.sizeInCache = sizeInCache; @@ -87,10 +86,10 @@ public String toString(String field) { } @Override - public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) { + public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { return new ConstantScoreWeight(this, boost) { @Override - public ScorerSupplier scorerSupplier(LeafReaderContext context) { + public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException { Scorer scorer = new ConstantScoreScorer(score(), scoreMode, DocIdSetIterator.all(context.reader().maxDoc())); return new DefaultScorerSupplier(scorer); } @@ -126,7 +125,7 @@ public void testBasics() throws IOException { IndicesQueryCache cache = new IndicesQueryCache(settings); s.setQueryCache(cache); - QueryCacheStats stats = cache.getStats(shard, 0L); + QueryCacheStats stats = cache.getStats(shard); assertEquals(0L, stats.getCacheSize()); assertEquals(0L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); @@ -134,7 +133,7 @@ public void testBasics() throws IOException { assertEquals(1, s.count(new DummyQuery(0))); - stats = cache.getStats(shard, 0L); + stats = cache.getStats(shard); assertEquals(1L, stats.getCacheSize()); assertEquals(1L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); @@ -144,7 +143,7 @@ public void testBasics() throws IOException { assertEquals(1, s.count(new DummyQuery(i))); } - stats = cache.getStats(shard, 0L); + stats = cache.getStats(shard); assertEquals(10L, stats.getCacheSize()); assertEquals(20L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); @@ -152,7 +151,7 @@ public void testBasics() throws IOException { s.count(new DummyQuery(10)); - stats = cache.getStats(shard, 0L); + stats = cache.getStats(shard); assertEquals(10L, stats.getCacheSize()); assertEquals(20L, stats.getCacheCount()); assertEquals(1L, stats.getHitCount()); @@ -161,7 +160,7 @@ public void testBasics() throws IOException { IOUtils.close(r, dir); // got emptied, but no changes to other metrics - stats = cache.getStats(shard, 0L); + stats = cache.getStats(shard); assertEquals(0L, stats.getCacheSize()); assertEquals(20L, stats.getCacheCount()); assertEquals(1L, stats.getHitCount()); @@ -170,7 +169,7 @@ public void testBasics() throws IOException { cache.onClose(shard); // forgot everything - stats = cache.getStats(shard, 0L); + stats = cache.getStats(shard); assertEquals(0L, stats.getCacheSize()); assertEquals(0L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); @@ -210,13 +209,13 @@ public void testTwoShards() throws IOException { assertEquals(1, s1.count(new DummyQuery(0))); - QueryCacheStats stats1 = cache.getStats(shard1, 0L); + QueryCacheStats stats1 = cache.getStats(shard1); assertEquals(1L, stats1.getCacheSize()); assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(2L, stats1.getMissCount()); - QueryCacheStats stats2 = cache.getStats(shard2, 0L); + QueryCacheStats stats2 = cache.getStats(shard2); assertEquals(0L, stats2.getCacheSize()); assertEquals(0L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); @@ -224,13 +223,13 @@ public void testTwoShards() throws IOException { assertEquals(1, s2.count(new DummyQuery(0))); - stats1 = cache.getStats(shard1, 0L); + stats1 = cache.getStats(shard1); assertEquals(1L, stats1.getCacheSize()); assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(2L, stats1.getMissCount()); - stats2 = cache.getStats(shard2, 0L); + stats2 = cache.getStats(shard2); assertEquals(1L, stats2.getCacheSize()); assertEquals(1L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); @@ -240,13 +239,13 @@ public void testTwoShards() throws IOException { assertEquals(1, s2.count(new DummyQuery(i))); } - stats1 = cache.getStats(shard1, 0L); + stats1 = cache.getStats(shard1); assertEquals(0L, stats1.getCacheSize()); // evicted assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(2L, stats1.getMissCount()); - stats2 = cache.getStats(shard2, 0L); + stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); @@ -255,13 +254,13 @@ public void testTwoShards() throws IOException { IOUtils.close(r1, dir1); // no changes - stats1 = cache.getStats(shard1, 0L); + stats1 = cache.getStats(shard1); assertEquals(0L, stats1.getCacheSize()); assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(2L, stats1.getMissCount()); - stats2 = cache.getStats(shard2, 0L); + stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); @@ -270,13 +269,13 @@ public void testTwoShards() throws IOException { cache.onClose(shard1); // forgot everything about shard1 - stats1 = cache.getStats(shard1, 0L); + stats1 = cache.getStats(shard1); assertEquals(0L, stats1.getCacheSize()); assertEquals(0L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(0L, stats1.getMissCount()); - stats2 = cache.getStats(shard2, 0L); + stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); @@ -286,14 +285,14 @@ public void testTwoShards() throws IOException { cache.onClose(shard2); // forgot everything about shard2 - stats1 = cache.getStats(shard1, 0L); + stats1 = cache.getStats(shard1); assertEquals(0L, stats1.getCacheSize()); assertEquals(0L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(0L, stats1.getMissCount()); assertEquals(0L, stats1.getMemorySizeInBytes()); - stats2 = cache.getStats(shard2, 0L); + stats2 = cache.getStats(shard2); assertEquals(0L, stats2.getCacheSize()); assertEquals(0L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); @@ -342,7 +341,7 @@ public void testStatsOnEviction() throws IOException { assertEquals(1, s2.count(new DummyQuery(i))); } - QueryCacheStats stats1 = cache.getStats(shard1, 0L); + QueryCacheStats stats1 = cache.getStats(shard1); assertEquals(0L, stats1.getCacheSize()); assertEquals(1L, stats1.getCacheCount()); @@ -421,7 +420,7 @@ public void testDelegatesScorerSupplier() throws Exception { assertNotSame(weight, cached); assertFalse(weight.scorerCalled); assertFalse(weight.scorerSupplierCalled); - cached.scorerSupplier(s.getIndexReader().leaves().getFirst()); + cached.scorerSupplier(s.getIndexReader().leaves().get(0)); assertFalse(weight.scorerCalled); assertTrue(weight.scorerSupplierCalled); IOUtils.close(r, dir); @@ -429,49 +428,6 @@ public void testDelegatesScorerSupplier() throws Exception { cache.close(); } - public void testGetSharedRamSizeForShard() { - ShardId shardId1 = new ShardId("index", "_na_", 0); - ShardId shardId2 = new ShardId("index", "_na_", 1); - IndexShard shard1 = mock(IndexShard.class); - IndexShard shard2 = mock(IndexShard.class); - when(shard1.shardId()).thenReturn(shardId1); - when(shard2.shardId()).thenReturn(shardId2); - - // Case 1: sharedRamBytesUsed = 0 - IndicesService indicesService = getTestIndicesService(List.of(Tuple.tuple(shard1, 100L), Tuple.tuple(shard2, 200L)), 0); - long sharedRam = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shardId1); - assertEquals(0L, sharedRam); - // Case 2: sharedRamBytesUsed > 0, totalSize > 0, proportional - indicesService = getTestIndicesService(List.of(Tuple.tuple(shard1, 100L), Tuple.tuple(shard2, 200L)), 600); - long sharedRam1 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shardId1); - assertEquals(200L, sharedRam1); - long sharedRam2 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shardId2); - assertEquals(400L, sharedRam2); - // Case 3: totalSize == 0, shared equally - indicesService = getTestIndicesService(List.of(Tuple.tuple(shard1, 0L), Tuple.tuple(shard2, 0L)), 600); - long sharedRamEq = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shardId1); - assertEquals(300L, sharedRamEq); - } - - private IndicesService getTestIndicesService(List> shardsAndSizes, long sharedRamBytesUsed) { - IndicesQueryCache queryCache = mock(IndicesQueryCache.class); - for (Tuple shardAndSize : shardsAndSizes) { - when(queryCache.getCacheSizeForShard(shardAndSize.v1().shardId())).thenReturn(shardAndSize.v2()); - } - when(queryCache.getSharedRamBytesUsed()).thenReturn(sharedRamBytesUsed); - return getTestIndicesService(shardsAndSizes.stream().map(Tuple::v1).toList(), queryCache); - } - - private IndicesService getTestIndicesService(List shards, IndicesQueryCache queryCache) { - IndexService indexService = mock(IndexService.class); - when(indexService.iterator()).thenAnswer(ignored -> shards.iterator()); - IndicesService indicesService = mock(IndicesService.class); - when(indicesService.iterator()).thenAnswer(ignored -> List.of(indexService).iterator()); - when(indicesService.getIndicesQueryCache()).thenReturn(queryCache); - when(indicesService.getIndicesQueryCache()).thenReturn(queryCache); - return indicesService; - } - public void testGetStatsMemory() throws Exception { /* * This test creates 2 shards, one with two segments and one with one. It makes unique queries against all 3 segments (so that each @@ -502,7 +458,7 @@ public void testGetStatsMemory() throws Exception { shard1Segment2Searcher.setQueryCache(cache); shard2Searcher.setQueryCache(cache); - assertEquals(0L, cache.getStats(shard1, 0).getMemorySizeInBytes()); + assertEquals(0L, cache.getStats(shard1).getMemorySizeInBytes()); final long largeQuerySize = randomIntBetween(100, 1000); final long smallQuerySize = randomIntBetween(10, 50); @@ -513,16 +469,9 @@ public void testGetStatsMemory() throws Exception { for (int i = 0; i < shard1Queries; ++i) { shard1Segment1Searcher.count(new DummyQuery("ingest1-" + i, largeQuerySize)); } - IndexShard indexShard1 = mock(IndexShard.class); - when(indexShard1.shardId()).thenReturn(shard1); - IndexShard indexShard2 = mock(IndexShard.class); - when(indexShard2.shardId()).thenReturn(shard2); - IndicesService indicesService = getTestIndicesService(List.of(indexShard1), cache); - long sharedRamSizeShard1 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard1); - long sharedRamSizeShard2 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard2); long shard1Segment1CacheMemory = calculateActualCacheMemoryForSegment(shard1Queries, largeQuerySize, shard1Segment1Docs); - assertThat(cache.getStats(shard1, sharedRamSizeShard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); - assertThat(cache.getStats(shard2, sharedRamSizeShard2).getMemorySizeInBytes(), equalTo(0L)); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { shard2Searcher.count(new DummyQuery("ingest2-" + i, smallQuerySize)); } @@ -531,12 +480,9 @@ public void testGetStatsMemory() throws Exception { * report cache memory proportional to the number of segments for each shard, ignoring the number of documents or the actual * document sizes. Since the shard2 requests were smaller, the average cache memory size per segment has now gone down. */ - indicesService = getTestIndicesService(List.of(indexShard1, indexShard2), cache); - sharedRamSizeShard1 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard1); - sharedRamSizeShard2 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard2); - assertThat(cache.getStats(shard1, sharedRamSizeShard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); - long shard1CacheBytes = cache.getStats(shard1, sharedRamSizeShard1).getMemorySizeInBytes(); - long shard2CacheBytes = cache.getStats(shard2, sharedRamSizeShard2).getMemorySizeInBytes(); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); + long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); + long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); long shard2Segment1CacheMemory = calculateActualCacheMemoryForSegment(shard2Queries, smallQuerySize, shard2Segment1Docs); long totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory; @@ -570,21 +516,16 @@ public void testGetStatsMemory() throws Exception { */ shard1Segment1CacheMemoryShare = ((double) (2 * shard1Queries) / ((2 * shard1Queries) + shard2Queries)) * (totalMemoryMinusOverhead) + shard1Overhead; - indicesService = getTestIndicesService(List.of(indexShard1, indexShard2), cache); - sharedRamSizeShard1 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard1); - shard1CacheBytes = cache.getStats(shard1, sharedRamSizeShard1).getMemorySizeInBytes(); + shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); assertThat((double) shard1CacheBytes, closeTo(shard1Segment1CacheMemoryShare, 1)); // accounting for rounding // Now make sure the cache only has items for shard2: for (int i = 0; i < (maxCacheSize * 2); ++i) { shard2Searcher.count(new DummyQuery("ingest4-" + i, smallQuerySize)); } - indicesService = getTestIndicesService(List.of(indexShard2), cache); - sharedRamSizeShard1 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard1); - sharedRamSizeShard2 = IndicesQueryCache.getSharedRamSizeForShard(indicesService, shard2); - assertThat(cache.getStats(shard1, sharedRamSizeShard1).getMemorySizeInBytes(), equalTo(0L)); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); assertThat( - cache.getStats(shard2, sharedRamSizeShard2).getMemorySizeInBytes(), + cache.getStats(shard2).getMemorySizeInBytes(), equalTo(calculateActualCacheMemoryForSegment(maxCacheSize, smallQuerySize, shard2Segment1Docs)) ); diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java index 8c7b376d0d1ff..78df6c9e88c65 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java @@ -180,17 +180,17 @@ public void testCloseAfterRequestHasUsedQueryCache() throws Exception { assertEquals(1, searcher.getIndexReader().maxDoc()); Query query = LongPoint.newRangeQuery("foo", 0, 5); - assertEquals(0L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize()); searcher.search(new ConstantScoreQuery(query), 1); - assertEquals(1L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(1L, cache.getStats(shard.shardId()).getCacheSize()); searcher.close(); assertEquals(2, indicesService.indicesRefCount.refCount()); - assertEquals(1L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(1L, cache.getStats(shard.shardId()).getCacheSize()); node.close(); assertEquals(0, indicesService.indicesRefCount.refCount()); - assertEquals(0L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize()); } public void testCloseWhileOngoingRequestUsesQueryCache() throws Exception { @@ -221,13 +221,13 @@ public void testCloseWhileOngoingRequestUsesQueryCache() throws Exception { assertEquals(1, indicesService.indicesRefCount.refCount()); Query query = LongPoint.newRangeQuery("foo", 0, 5); - assertEquals(0L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize()); searcher.search(new ConstantScoreQuery(query), 1); - assertEquals(1L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(1L, cache.getStats(shard.shardId()).getCacheSize()); searcher.close(); assertEquals(0, indicesService.indicesRefCount.refCount()); - assertEquals(0L, cache.getStats(shard.shardId(), 0L).getCacheSize()); + assertEquals(0L, cache.getStats(shard.shardId()).getCacheSize()); } public void testCloseWhileOngoingRequestUsesRequestCache() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 2a034103478b8..7f2aa62f7cf83 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -613,16 +613,14 @@ public void testStatsByShardDoesNotDieFromExpectedExceptions() { shardStats.add(successfulShardStats); - when(mockIndicesService.indexShardStats(mockIndicesService, shard, CommonStatsFlags.ALL, 0L)).thenReturn( - successfulShardStats - ); + when(mockIndicesService.indexShardStats(mockIndicesService, shard, CommonStatsFlags.ALL)).thenReturn(successfulShardStats); } else { - when(mockIndicesService.indexShardStats(mockIndicesService, shard, CommonStatsFlags.ALL, 0L)).thenThrow(expectedException); + when(mockIndicesService.indexShardStats(mockIndicesService, shard, CommonStatsFlags.ALL)).thenThrow(expectedException); } } - when(mockIndicesService.iterator()).thenAnswer(invocation -> Collections.singleton(indexService).iterator()); - when(indexService.iterator()).thenAnswer(unused -> shards.iterator()); + when(mockIndicesService.iterator()).thenReturn(Collections.singleton(indexService).iterator()); + when(indexService.iterator()).thenReturn(shards.iterator()); when(indexService.index()).thenReturn(index); // real one, which has a logger defined