From 3305bbd4194a231740cd7f142af30343618bd647 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 23 Sep 2025 10:25:22 -0500 Subject: [PATCH 01/14] Adding a test to show how IndicesQueryCache reports memory usage --- .../indices/IndicesQueryCacheTests.java | 86 ++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 4f73672471942..668644e139455 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -36,14 +36,23 @@ import java.io.IOException; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.lessThan; + public class IndicesQueryCacheTests extends ESTestCase { - private static class DummyQuery extends Query { + private static class DummyQuery extends Query implements org.apache.lucene.util.Accountable { private final int id; + private final long sizeInCache; DummyQuery(int id) { + this(id, 10); + } + + DummyQuery(int id, long sizeInCache) { this.id = id; + this.sizeInCache = sizeInCache; } @Override @@ -82,6 +91,10 @@ public boolean isCacheable(LeafReaderContext ctx) { }; } + @Override + public long ramBytesUsed() { + return sizeInCache; + } } public void testBasics() throws IOException { @@ -404,4 +417,75 @@ public void testDelegatesScorerSupplier() throws Exception { cache.onClose(shard); cache.close(); } + + public void testGetStatsMemory() throws IOException { + Directory dir1 = newDirectory(); + IndexWriter indexWriter1 = new IndexWriter(dir1, newIndexWriterConfig()); + indexWriter1.addDocument(new Document()); + DirectoryReader directoryReader1 = DirectoryReader.open(indexWriter1); + indexWriter1.close(); + ShardId shard1 = new ShardId("index1", "_na_", 0); + directoryReader1 = ElasticsearchDirectoryReader.wrap(directoryReader1, shard1); + IndexSearcher indexSearcher1 = new IndexSearcher(directoryReader1); + indexSearcher1.setQueryCachingPolicy(TrivialQueryCachingPolicy.ALWAYS); + + Directory dir2 = newDirectory(); + IndexWriter indexWriter2 = new IndexWriter(dir2, newIndexWriterConfig()); + indexWriter2.addDocument(new Document()); + DirectoryReader directoryReader2 = DirectoryReader.open(indexWriter2); + indexWriter2.close(); + ShardId shard2 = new ShardId("index2", "_na_", 0); + directoryReader2 = ElasticsearchDirectoryReader.wrap(directoryReader2, shard2); + IndexSearcher indexSearcher2 = new IndexSearcher(directoryReader2); + indexSearcher2.setQueryCachingPolicy(TrivialQueryCachingPolicy.ALWAYS); + + final int maxCacheSize = 100; + Settings settings = Settings.builder() + .put(IndicesQueryCache.INDICES_CACHE_QUERY_COUNT_SETTING.getKey(), maxCacheSize) + .put(IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING.getKey(), true) + .build(); + IndicesQueryCache cache = new IndicesQueryCache(settings); + indexSearcher1.setQueryCache(cache); + indexSearcher2.setQueryCache(cache); + + assertEquals(0L, cache.getStats(shard1).getMemorySizeInBytes()); + + final long extraCacheSizePerObject = 136; + final long shard1QuerySize = 1000; + final int shard1Queries = 20; + final int shard2Queries = 5; + final long shard2QuerySize = 10; + + for (int i = 0; i < shard1Queries; ++i) { + indexSearcher1.count(new DummyQuery(i, shard1QuerySize)); + } + // After caching 20 big things on shard1, the cache memory is exactly 20 * the object size: + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Queries * (shard1QuerySize + extraCacheSizePerObject))); + for (int i = 0; i < shard2Queries; ++i) { + indexSearcher2.count(new DummyQuery(i, shard2QuerySize)); + } + /* + * Now that we have cached some smaller things for shard2, the cache memory for shard1 has gone down. This is expected because we + * report cache memory proportional to the number of documents for each shard, ignoring the actual document sizes. Since the shard2 + * requests were smaller, the average cache memory size per document has now gone down. + */ + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Queries * (shard1QuerySize + extraCacheSizePerObject))); + long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); + long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); + // Asserting that the memory reported is proportional to the number of documents, ignoring their sizes: + assertThat(shard1CacheBytes, equalTo(shard2CacheBytes * (shard1Queries / shard2Queries))); + + // Now make sure the cache only has items for shard2: + for (int i = 0; i < (maxCacheSize * 2); ++i) { + indexSearcher2.count(new DummyQuery(i, shard2QuerySize)); + } + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (shard2QuerySize + extraCacheSizePerObject))); + + IOUtils.close(directoryReader1, dir1); + IOUtils.close(directoryReader2, dir2); + cache.onClose(shard1); + cache.onClose(shard2); + cache.close(); + } } From 2daac3b634ebd4bda49079714230ecfd73f92a76 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 23 Sep 2025 10:36:18 -0500 Subject: [PATCH 02/14] Adding an additional test assertion --- .../java/org/elasticsearch/indices/IndicesQueryCacheTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 668644e139455..1ab1d9486b67d 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -461,6 +461,7 @@ public void testGetStatsMemory() throws IOException { } // After caching 20 big things on shard1, the cache memory is exactly 20 * the object size: assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Queries * (shard1QuerySize + extraCacheSizePerObject))); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { indexSearcher2.count(new DummyQuery(i, shard2QuerySize)); } From bedeae43437392e9e6a3ba36948a32f571b282a5 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Tue, 23 Sep 2025 10:38:39 -0500 Subject: [PATCH 03/14] Updating comments and variable names for clarity --- .../indices/IndicesQueryCache.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java index 9bca59e9e4d62..15973889083e1 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java @@ -89,16 +89,19 @@ private static QueryCacheStats toQueryCacheStatsSafe(@Nullable Stats stats) { return stats == null ? new QueryCacheStats() : stats.toQueryCacheStats(); } - private long getShareOfAdditionalRamBytesUsed(long cacheSize) { + private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { if (sharedRamBytesUsed == 0L) { return 0L; } - // We also have some shared ram usage that we try to distribute proportionally to the cache footprint of each shard. + /* + * We have some shared ram usage that we try to distribute proportionally to the number of documents in the cache cache for each + * shard. + */ // TODO avoid looping over all local shards here - see https://github.com/elastic/elasticsearch/issues/97222 - long totalSize = 0L; + long totalItemsInCache = 0L; int shardCount = 0; - if (cacheSize == 0L) { + if (itemsInCacheForShard == 0L) { for (final var stats : shardStats.values()) { shardCount += 1; if (stats.cacheSize > 0L) { @@ -110,7 +113,7 @@ private long getShareOfAdditionalRamBytesUsed(long cacheSize) { // branchless loop for the common case for (final var stats : shardStats.values()) { shardCount += 1; - totalSize += stats.cacheSize; + totalItemsInCache += stats.cacheSize; } } @@ -121,12 +124,15 @@ private long getShareOfAdditionalRamBytesUsed(long cacheSize) { } final long additionalRamBytesUsed; - if (totalSize == 0) { + if (totalItemsInCache == 0) { // all shards have zero cache footprint, so we apportion the size of the shared bytes equally across all shards additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed / shardCount); } else { - // some shards have nonzero cache footprint, so we apportion the size of the shared bytes proportionally to cache footprint - additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed * cacheSize / totalSize); + /* + * some shards have nonzero cache footprint, so we apportion the size of the shared bytes proportionally to the number of + * documents in the cache for this shard + */ + additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed * itemsInCacheForShard / totalItemsInCache); } assert additionalRamBytesUsed >= 0L : additionalRamBytesUsed; return additionalRamBytesUsed; From 44e0548a273584102bd8af9c666dcff6ea588a2a Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Wed, 24 Sep 2025 15:49:06 -0500 Subject: [PATCH 04/14] switching test to segments, work in progress --- .../indices/IndicesQueryCacheTests.java | 138 ++++++++++++------ 1 file changed, 93 insertions(+), 45 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 1ab1d9486b67d..eae6bed036d47 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -34,8 +34,13 @@ import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; +import java.io.Closeable; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.lessThan; @@ -43,26 +48,30 @@ public class IndicesQueryCacheTests extends ESTestCase { private static class DummyQuery extends Query implements org.apache.lucene.util.Accountable { - private final int id; + private final String id; private final long sizeInCache; DummyQuery(int id) { + this(Integer.toString(id), 10); + } + + DummyQuery(String id) { this(id, 10); } - DummyQuery(int id, long sizeInCache) { + DummyQuery(String id, long sizeInCache) { this.id = id; this.sizeInCache = sizeInCache; } @Override public boolean equals(Object obj) { - return sameClassAs(obj) && id == ((DummyQuery) obj).id; + return sameClassAs(obj) && id.equals(((DummyQuery) obj).id); } @Override public int hashCode() { - return 31 * classHash() + id; + return 31 * classHash() + id.hashCode(); } @Override @@ -418,75 +427,114 @@ public void testDelegatesScorerSupplier() throws Exception { cache.close(); } - public void testGetStatsMemory() throws IOException { - Directory dir1 = newDirectory(); - IndexWriter indexWriter1 = new IndexWriter(dir1, newIndexWriterConfig()); - indexWriter1.addDocument(new Document()); - DirectoryReader directoryReader1 = DirectoryReader.open(indexWriter1); - indexWriter1.close(); - ShardId shard1 = new ShardId("index1", "_na_", 0); - directoryReader1 = ElasticsearchDirectoryReader.wrap(directoryReader1, shard1); - IndexSearcher indexSearcher1 = new IndexSearcher(directoryReader1); - indexSearcher1.setQueryCachingPolicy(TrivialQueryCachingPolicy.ALWAYS); - - Directory dir2 = newDirectory(); - IndexWriter indexWriter2 = new IndexWriter(dir2, newIndexWriterConfig()); - indexWriter2.addDocument(new Document()); - DirectoryReader directoryReader2 = DirectoryReader.open(indexWriter2); - indexWriter2.close(); - ShardId shard2 = new ShardId("index2", "_na_", 0); - directoryReader2 = ElasticsearchDirectoryReader.wrap(directoryReader2, shard2); - IndexSearcher indexSearcher2 = new IndexSearcher(directoryReader2); - indexSearcher2.setQueryCachingPolicy(TrivialQueryCachingPolicy.ALWAYS); - - final int maxCacheSize = 100; + @SuppressWarnings("cast") + public void testGetStatsMemory() throws Exception { + String indexName = randomIdentifier(); + String uuid = randomUUID(); + ShardId shard1 = new ShardId(indexName, uuid, 0); + ShardId shard2 = new ShardId(indexName, uuid, 1); + List closeableList = new ArrayList<>(); + int shard1Segment1Docs = randomIntBetween(11, 1000); + int shard1Segment2Docs = randomIntBetween(1, 10); + int shardSegment1Docs = randomIntBetween(1, 10); + IndexSearcher shard1Searcher1 = initializeSegment(shard1, shard1Segment1Docs, closeableList); + IndexSearcher shard1Searcher2 = initializeSegment(shard1, shard1Segment2Docs, closeableList); + IndexSearcher shard2Searcher1 = initializeSegment(shard2, shardSegment1Docs, closeableList); + + final int maxCacheSize = 200; Settings settings = Settings.builder() .put(IndicesQueryCache.INDICES_CACHE_QUERY_COUNT_SETTING.getKey(), maxCacheSize) .put(IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING.getKey(), true) .build(); IndicesQueryCache cache = new IndicesQueryCache(settings); - indexSearcher1.setQueryCache(cache); - indexSearcher2.setQueryCache(cache); + shard1Searcher1.setQueryCache(cache); + shard1Searcher2.setQueryCache(cache); + shard2Searcher1.setQueryCache(cache); assertEquals(0L, cache.getStats(shard1).getMemorySizeInBytes()); - final long extraCacheSizePerObject = 136; - final long shard1QuerySize = 1000; - final int shard1Queries = 20; - final int shard2Queries = 5; - final long shard2QuerySize = 10; + final long extraCacheSizePerQuery = 24; + + final long shard1QuerySize = randomIntBetween(100, 1000); + final int shard1Queries = 40;// randomIntBetween(20, 50); // 40 works, impacts 465 + final int shard2Queries = randomIntBetween(5, 10); + final long shard2QuerySize = randomIntBetween(10, 50); for (int i = 0; i < shard1Queries; ++i) { - indexSearcher1.count(new DummyQuery(i, shard1QuerySize)); + shard1Searcher1.count(new DummyQuery("ingest1-" + i, shard1QuerySize)); } - // After caching 20 big things on shard1, the cache memory is exactly 20 * the object size: - assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Queries * (shard1QuerySize + extraCacheSizePerObject))); + // After caching a number of big things on shard1, the cache memory is exactly 20 * the object size: + long shard1Segment1MinimumCacheMemory = (shard1Queries * (shard1QuerySize + extraCacheSizePerQuery)) + ((shard1Queries * 112) + + ((((shard1Segment1Docs) - 1) / 64) * 320L)) + (shard1Queries * 16) - 640; + + System.out.println(shard1Queries + "\t" + shard1Segment1Docs + "\t" + cache.getStats(shard1).getMemorySizeInBytes()); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1MinimumCacheMemory)); assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { - indexSearcher2.count(new DummyQuery(i, shard2QuerySize)); + shard2Searcher1.count(new DummyQuery("ingest2-" + i, shard2QuerySize)); } /* * Now that we have cached some smaller things for shard2, the cache memory for shard1 has gone down. This is expected because we * report cache memory proportional to the number of documents for each shard, ignoring the actual document sizes. Since the shard2 * requests were smaller, the average cache memory size per document has now gone down. */ - assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Queries * (shard1QuerySize + extraCacheSizePerObject))); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1MinimumCacheMemory)); long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); - // Asserting that the memory reported is proportional to the number of documents, ignoring their sizes: - assertThat(shard1CacheBytes, equalTo(shard2CacheBytes * (shard1Queries / shard2Queries))); - + // Asserting that the memory reported is proportional to the number of segments, ignoring their sizes: + assertThat( + (double) shard1CacheBytes, + closeTo( + (double) (shard2CacheBytes * ((double) shard1Queries / shard2Queries) + ((int) (((shard1Segment1Docs) - 1) / 64) * 320L)) + + (shard1Queries * 24) - 960, + shard1CacheBytes * 0.01 + ) + ); + + // Now we cache just 20 more "big" searches on shard1, but on a different segment: + for (int i = 0; i < shard1Queries; ++i) { + shard1Searcher2.count(new DummyQuery("ingest3-" + i, shard1QuerySize)); + } + assertThat( + (double) cache.getStats(shard1).getMemorySizeInBytes(), + closeTo( + cache.getStats(shard2).getMemorySizeInBytes() * 2 * ((double) shard1Queries / shard2Queries) + ((int) (((shard1Segment1Docs) + - 1) / 64) * 320L) + (shard1Queries * 24) - 960, + (double) cache.getStats(shard1).getMemorySizeInBytes() * 0.01 + ) + ); // Now make sure the cache only has items for shard2: for (int i = 0; i < (maxCacheSize * 2); ++i) { - indexSearcher2.count(new DummyQuery(i, shard2QuerySize)); + shard2Searcher1.count(new DummyQuery("ingest4-" + i, shard2QuerySize)); } assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); - assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (shard2QuerySize + extraCacheSizePerObject))); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (shard2QuerySize + extraCacheSizePerQuery + 112))); - IOUtils.close(directoryReader1, dir1); - IOUtils.close(directoryReader2, dir2); + IOUtils.close(closeableList); cache.onClose(shard1); cache.onClose(shard2); cache.close(); } + + private IndexSearcher initializeSegment(ShardId shard, int numDocs, List closeableList) throws Exception { + AtomicReference indexSearcherReference = new AtomicReference<>(); + assertBusy(() -> { + Directory dir = newDirectory(); + IndexWriter indexWriter = new IndexWriter(dir, newIndexWriterConfig()); + for (int i = 0; i < numDocs; i++) { + indexWriter.addDocument(new Document()); + } + System.out.println("here"); + DirectoryReader directoryReader = DirectoryReader.open(indexWriter); + indexWriter.close(); + directoryReader = ElasticsearchDirectoryReader.wrap(directoryReader, shard); + IndexSearcher indexSearcher = new IndexSearcher(directoryReader); + indexSearcherReference.set(indexSearcher); + indexSearcher.setQueryCachingPolicy(TrivialQueryCachingPolicy.ALWAYS); + closeableList.add(directoryReader); + closeableList.add(dir); + assertThat(indexSearcher.getLeafContexts().size(), equalTo(1)); + }); + return indexSearcherReference.get(); + } } From 1272a9b286cd8dba9f9cd020173ea862f293898b Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 25 Sep 2025 10:53:15 -0500 Subject: [PATCH 05/14] simplifying an expression --- .../org/elasticsearch/indices/IndicesQueryCacheTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index eae6bed036d47..09a3c79836939 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -464,8 +464,8 @@ public void testGetStatsMemory() throws Exception { shard1Searcher1.count(new DummyQuery("ingest1-" + i, shard1QuerySize)); } // After caching a number of big things on shard1, the cache memory is exactly 20 * the object size: - long shard1Segment1MinimumCacheMemory = (shard1Queries * (shard1QuerySize + extraCacheSizePerQuery)) + ((shard1Queries * 112) - + ((((shard1Segment1Docs) - 1) / 64) * 320L)) + (shard1Queries * 16) - 640; + long shard1Segment1MinimumCacheMemory = (shard1Queries * (128 + shard1QuerySize + extraCacheSizePerQuery)) + + (((shard1Segment1Docs - 1) / 64) * 320L) - 640; System.out.println(shard1Queries + "\t" + shard1Segment1Docs + "\t" + cache.getStats(shard1).getMemorySizeInBytes()); assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1MinimumCacheMemory)); From 2b0c3c4c1c1b114b6756ceb26ca1ced5c2073ad6 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 25 Sep 2025 14:26:16 -0500 Subject: [PATCH 06/14] cleanup/commenting --- .../indices/IndicesQueryCacheTests.java | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 09a3c79836939..e8384d3533762 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -429,17 +429,24 @@ public void testDelegatesScorerSupplier() throws Exception { @SuppressWarnings("cast") 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 + * query will be cached, up to the max cache size), and then asserts various things about the cache memory. Most importantly, it + * asserts that the memory the cache attributes to each shard is proportional to the number of segment-queries for the shard in the + * cache (and not to the number of documents in the query). + */ String indexName = randomIdentifier(); String uuid = randomUUID(); ShardId shard1 = new ShardId(indexName, uuid, 0); ShardId shard2 = new ShardId(indexName, uuid, 1); List closeableList = new ArrayList<>(); + // We're going to create 2 segments for shard1, and 1 segment for shard2: int shard1Segment1Docs = randomIntBetween(11, 1000); int shard1Segment2Docs = randomIntBetween(1, 10); int shardSegment1Docs = randomIntBetween(1, 10); - IndexSearcher shard1Searcher1 = initializeSegment(shard1, shard1Segment1Docs, closeableList); - IndexSearcher shard1Searcher2 = initializeSegment(shard1, shard1Segment2Docs, closeableList); - IndexSearcher shard2Searcher1 = initializeSegment(shard2, shardSegment1Docs, closeableList); + IndexSearcher shard1Segment1Searcher = initializeSegment(shard1, shard1Segment1Docs, closeableList); + IndexSearcher shard1Segment2Searcher = initializeSegment(shard1, shard1Segment2Docs, closeableList); + IndexSearcher shard2Searcher = initializeSegment(shard2, shardSegment1Docs, closeableList); final int maxCacheSize = 200; Settings settings = Settings.builder() @@ -447,38 +454,37 @@ public void testGetStatsMemory() throws Exception { .put(IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING.getKey(), true) .build(); IndicesQueryCache cache = new IndicesQueryCache(settings); - shard1Searcher1.setQueryCache(cache); - shard1Searcher2.setQueryCache(cache); - shard2Searcher1.setQueryCache(cache); + shard1Segment1Searcher.setQueryCache(cache); + shard1Segment2Searcher.setQueryCache(cache); + shard2Searcher.setQueryCache(cache); assertEquals(0L, cache.getStats(shard1).getMemorySizeInBytes()); final long extraCacheSizePerQuery = 24; - final long shard1QuerySize = randomIntBetween(100, 1000); + final long largeQuerySize = randomIntBetween(100, 1000); + final long smallQuerySize = randomIntBetween(10, 50); final int shard1Queries = 40;// randomIntBetween(20, 50); // 40 works, impacts 465 final int shard2Queries = randomIntBetween(5, 10); - final long shard2QuerySize = randomIntBetween(10, 50); for (int i = 0; i < shard1Queries; ++i) { - shard1Searcher1.count(new DummyQuery("ingest1-" + i, shard1QuerySize)); + shard1Segment1Searcher.count(new DummyQuery("ingest1-" + i, largeQuerySize)); } // After caching a number of big things on shard1, the cache memory is exactly 20 * the object size: - long shard1Segment1MinimumCacheMemory = (shard1Queries * (128 + shard1QuerySize + extraCacheSizePerQuery)) - + (((shard1Segment1Docs - 1) / 64) * 320L) - 640; + long shard1Segment1CacheMemory = (shard1Queries * (128 + largeQuerySize + extraCacheSizePerQuery)) + (((shard1Segment1Docs - 1) + / 64) * 320L) - 640; - System.out.println(shard1Queries + "\t" + shard1Segment1Docs + "\t" + cache.getStats(shard1).getMemorySizeInBytes()); - assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1MinimumCacheMemory)); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { - shard2Searcher1.count(new DummyQuery("ingest2-" + i, shard2QuerySize)); + shard2Searcher.count(new DummyQuery("ingest2-" + i, smallQuerySize)); } /* * Now that we have cached some smaller things for shard2, the cache memory for shard1 has gone down. This is expected because we - * report cache memory proportional to the number of documents for each shard, ignoring the actual document sizes. Since the shard2 - * requests were smaller, the average cache memory size per document has now gone down. + * 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. */ - assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1MinimumCacheMemory)); + assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); // Asserting that the memory reported is proportional to the number of segments, ignoring their sizes: @@ -491,9 +497,9 @@ public void testGetStatsMemory() throws Exception { ) ); - // Now we cache just 20 more "big" searches on shard1, but on a different segment: + // Now we cache just more "big" searches on shard1, but on a different segment: for (int i = 0; i < shard1Queries; ++i) { - shard1Searcher2.count(new DummyQuery("ingest3-" + i, shard1QuerySize)); + shard1Segment2Searcher.count(new DummyQuery("ingest3-" + i, largeQuerySize)); } assertThat( (double) cache.getStats(shard1).getMemorySizeInBytes(), @@ -505,10 +511,10 @@ public void testGetStatsMemory() throws Exception { ); // Now make sure the cache only has items for shard2: for (int i = 0; i < (maxCacheSize * 2); ++i) { - shard2Searcher1.count(new DummyQuery("ingest4-" + i, shard2QuerySize)); + shard2Searcher.count(new DummyQuery("ingest4-" + i, smallQuerySize)); } assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); - assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (shard2QuerySize + extraCacheSizePerQuery + 112))); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (smallQuerySize + extraCacheSizePerQuery + 112))); IOUtils.close(closeableList); cache.onClose(shard1); @@ -516,15 +522,21 @@ public void testGetStatsMemory() throws Exception { cache.close(); } + /* + * This returns an IndexSearcher for a single new segment in the given shard. + */ private IndexSearcher initializeSegment(ShardId shard, int numDocs, List closeableList) throws Exception { AtomicReference indexSearcherReference = new AtomicReference<>(); + /* + * Usually creating an IndexWriter like this results in a single segment getting created, but sometimes it results in more. For the + * sake of keeping the calculations in this test simple we want just a single segment. So we do this in an assertBusy. + */ assertBusy(() -> { Directory dir = newDirectory(); IndexWriter indexWriter = new IndexWriter(dir, newIndexWriterConfig()); for (int i = 0; i < numDocs; i++) { indexWriter.addDocument(new Document()); } - System.out.println("here"); DirectoryReader directoryReader = DirectoryReader.open(indexWriter); indexWriter.close(); directoryReader = ElasticsearchDirectoryReader.wrap(directoryReader, shard); From 39e25acde2bab77c98806116e9d35bcad109c707 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 25 Sep 2025 17:51:25 -0500 Subject: [PATCH 07/14] working test for segments --- .../indices/IndicesQueryCacheTests.java | 65 +++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index e8384d3533762..28feeade31f26 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -443,10 +443,10 @@ public void testGetStatsMemory() throws Exception { // We're going to create 2 segments for shard1, and 1 segment for shard2: int shard1Segment1Docs = randomIntBetween(11, 1000); int shard1Segment2Docs = randomIntBetween(1, 10); - int shardSegment1Docs = randomIntBetween(1, 10); + int shard2Segment1Docs = randomIntBetween(1, 10); IndexSearcher shard1Segment1Searcher = initializeSegment(shard1, shard1Segment1Docs, closeableList); IndexSearcher shard1Segment2Searcher = initializeSegment(shard1, shard1Segment2Docs, closeableList); - IndexSearcher shard2Searcher = initializeSegment(shard2, shardSegment1Docs, closeableList); + IndexSearcher shard2Searcher = initializeSegment(shard2, shard2Segment1Docs, closeableList); final int maxCacheSize = 200; Settings settings = Settings.builder() @@ -460,20 +460,16 @@ public void testGetStatsMemory() throws Exception { assertEquals(0L, cache.getStats(shard1).getMemorySizeInBytes()); - final long extraCacheSizePerQuery = 24; - final long largeQuerySize = randomIntBetween(100, 1000); final long smallQuerySize = randomIntBetween(10, 50); - final int shard1Queries = 40;// randomIntBetween(20, 50); // 40 works, impacts 465 + + final int shard1Queries = randomIntBetween(20, 50); final int shard2Queries = randomIntBetween(5, 10); for (int i = 0; i < shard1Queries; ++i) { shard1Segment1Searcher.count(new DummyQuery("ingest1-" + i, largeQuerySize)); } - // After caching a number of big things on shard1, the cache memory is exactly 20 * the object size: - long shard1Segment1CacheMemory = (shard1Queries * (128 + largeQuerySize + extraCacheSizePerQuery)) + (((shard1Segment1Docs - 1) - / 64) * 320L) - 640; - + long shard1Segment1CacheMemory = shard1Queries * ((largeQuerySize + 128) + 8 * (((shard1Segment1Docs - 1) / 64) + 1)); assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { @@ -487,34 +483,49 @@ public void testGetStatsMemory() throws Exception { assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); - // Asserting that the memory reported is proportional to the number of segments, ignoring their sizes: - assertThat( - (double) shard1CacheBytes, - closeTo( - (double) (shard2CacheBytes * ((double) shard1Queries / shard2Queries) + ((int) (((shard1Segment1Docs) - 1) / 64) * 320L)) - + (shard1Queries * 24) - 960, - shard1CacheBytes * 0.01 - ) - ); + long shard2Segment1CacheMemory = shard2Queries * ((smallQuerySize + 128) + 8 * (((shard2Segment1Docs - 1) / 64) + 1)); + + long totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory; + // Each shard has some fixed overhead that we need to account for: + long shard1Overhead = shard1Queries * (112 + 8 * ((shard1Segment1Docs - 1) / 64)); + long shard2Overhead = shard2Queries * (112 + 8 * ((shard2Segment1Docs - 1) / 64)); + long totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); + /* + * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each shard + * (since each shard currently only has queries to one segment) + */ + double shard1Segment1CacheMemoryShare = ((double) shard1Queries / (shard1Queries + shard2Queries)) * (totalMemoryMinusOverhead) + + shard1Overhead; + double shard2Segment1CacheMemoryShare = ((double) shard2Queries / (shard1Queries + shard2Queries)) * (totalMemoryMinusOverhead) + + shard2Overhead; + assertThat((double) shard1CacheBytes, closeTo(shard1Segment1CacheMemoryShare, 1)); // accounting for rounding + assertThat((double) shard2CacheBytes, closeTo(shard2Segment1CacheMemoryShare, 1)); // accounting for rounding // Now we cache just more "big" searches on shard1, but on a different segment: for (int i = 0; i < shard1Queries; ++i) { shard1Segment2Searcher.count(new DummyQuery("ingest3-" + i, largeQuerySize)); } - assertThat( - (double) cache.getStats(shard1).getMemorySizeInBytes(), - closeTo( - cache.getStats(shard2).getMemorySizeInBytes() * 2 * ((double) shard1Queries / shard2Queries) + ((int) (((shard1Segment1Docs) - - 1) / 64) * 320L) + (shard1Queries * 24) - 960, - (double) cache.getStats(shard1).getMemorySizeInBytes() * 0.01 - ) - ); + long shard1Segment2CacheMemory = shard1Queries * ((largeQuerySize + 128) + 8 * (((shard1Segment2Docs - 1) / 64) + 1)); + totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory + shard1Segment2CacheMemory; + System.err.println(shard1Queries + "\t" + shard1Segment1Docs + "\t" + shard2Queries + "\t" + shard2Segment1Docs); + // Each shard has some fixed overhead that we need to account for: + shard1Overhead = shard1Overhead + shard1Queries * (112 + 8 * ((shard1Segment2Docs - 1) / 64)); + totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); + /* + * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each segment. + * The number of documents and the size of documents is irrelevant (aside from computing the fixed overhead). + */ + shard1Segment1CacheMemoryShare = ((double) (2 * shard1Queries) / ((2 * shard1Queries) + shard2Queries)) * (totalMemoryMinusOverhead) + + shard1Overhead; + 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)); } assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); - assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (smallQuerySize + extraCacheSizePerQuery + 112))); + assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (smallQuerySize + 136))); IOUtils.close(closeableList); cache.onClose(shard1); From 12698dd92835c7dfcc9ddcbc06aa5c727a5fc777 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 25 Sep 2025 18:05:59 -0500 Subject: [PATCH 08/14] cleaning up test --- .../indices/IndicesQueryCacheTests.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 28feeade31f26..3aa77ab476b3e 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -469,7 +469,7 @@ public void testGetStatsMemory() throws Exception { for (int i = 0; i < shard1Queries; ++i) { shard1Segment1Searcher.count(new DummyQuery("ingest1-" + i, largeQuerySize)); } - long shard1Segment1CacheMemory = shard1Queries * ((largeQuerySize + 128) + 8 * (((shard1Segment1Docs - 1) / 64) + 1)); + long shard1Segment1CacheMemory = calculateActualCacheMemoryForShard(shard1Queries, largeQuerySize, shard1Segment1Docs); assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { @@ -483,12 +483,12 @@ public void testGetStatsMemory() throws Exception { assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); - long shard2Segment1CacheMemory = shard2Queries * ((smallQuerySize + 128) + 8 * (((shard2Segment1Docs - 1) / 64) + 1)); + long shard2Segment1CacheMemory = calculateActualCacheMemoryForShard(shard2Queries, smallQuerySize, shard2Segment1Docs); long totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory; // Each shard has some fixed overhead that we need to account for: - long shard1Overhead = shard1Queries * (112 + 8 * ((shard1Segment1Docs - 1) / 64)); - long shard2Overhead = shard2Queries * (112 + 8 * ((shard2Segment1Docs - 1) / 64)); + long shard1Overhead = calculateOverheadForShard(shard1Queries, shard1Segment1Docs); + long shard2Overhead = calculateOverheadForShard(shard2Queries, shard2Segment1Docs); long totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); /* * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each shard @@ -505,11 +505,10 @@ public void testGetStatsMemory() throws Exception { for (int i = 0; i < shard1Queries; ++i) { shard1Segment2Searcher.count(new DummyQuery("ingest3-" + i, largeQuerySize)); } - long shard1Segment2CacheMemory = shard1Queries * ((largeQuerySize + 128) + 8 * (((shard1Segment2Docs - 1) / 64) + 1)); + long shard1Segment2CacheMemory = calculateActualCacheMemoryForShard(shard1Queries, largeQuerySize, shard1Segment2Docs); totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory + shard1Segment2CacheMemory; - System.err.println(shard1Queries + "\t" + shard1Segment1Docs + "\t" + shard2Queries + "\t" + shard2Segment1Docs); // Each shard has some fixed overhead that we need to account for: - shard1Overhead = shard1Overhead + shard1Queries * (112 + 8 * ((shard1Segment2Docs - 1) / 64)); + shard1Overhead = shard1Overhead + calculateOverheadForShard(shard1Queries, shard1Segment2Docs); totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); /* * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each segment. @@ -525,7 +524,10 @@ public void testGetStatsMemory() throws Exception { shard2Searcher.count(new DummyQuery("ingest4-" + i, smallQuerySize)); } assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); - assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(maxCacheSize * (smallQuerySize + 136))); + assertThat( + cache.getStats(shard2).getMemorySizeInBytes(), + equalTo(calculateActualCacheMemoryForShard(maxCacheSize, smallQuerySize, shard2Segment1Docs)) + ); IOUtils.close(closeableList); cache.onClose(shard1); @@ -533,6 +535,14 @@ public void testGetStatsMemory() throws Exception { cache.close(); } + private long calculateActualCacheMemoryForShard(long queryCount, long querySize, long numDocs) { + return queryCount * (querySize + 24) + calculateOverheadForShard(queryCount, numDocs); + } + + private long calculateOverheadForShard(long queryCount, long numDocs) { + return queryCount * (112 + 8 * ((numDocs - 1) / 64)); + } + /* * This returns an IndexSearcher for a single new segment in the given shard. */ From ce6958eb66d47ac51e93c0d872426f37df24303c Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Thu, 25 Sep 2025 23:12:35 +0000 Subject: [PATCH 09/14] [CI] Update transport version definitions --- server/src/main/resources/transport/upper_bounds/8.18.csv | 2 +- server/src/main/resources/transport/upper_bounds/8.19.csv | 2 +- server/src/main/resources/transport/upper_bounds/9.0.csv | 2 +- server/src/main/resources/transport/upper_bounds/9.1.csv | 2 +- server/src/main/resources/transport/upper_bounds/9.2.csv | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/resources/transport/upper_bounds/8.18.csv b/server/src/main/resources/transport/upper_bounds/8.18.csv index ffc592e1809ee..266bfbbd3bf78 100644 --- a/server/src/main/resources/transport/upper_bounds/8.18.csv +++ b/server/src/main/resources/transport/upper_bounds/8.18.csv @@ -1 +1 @@ -initial_elasticsearch_8_18_8,8840010 +transform_check_for_dangling_tasks,8840011 diff --git a/server/src/main/resources/transport/upper_bounds/8.19.csv b/server/src/main/resources/transport/upper_bounds/8.19.csv index 3cc6f439c5ea5..3600b3f8c633a 100644 --- a/server/src/main/resources/transport/upper_bounds/8.19.csv +++ b/server/src/main/resources/transport/upper_bounds/8.19.csv @@ -1 +1 @@ -initial_elasticsearch_8_19_5,8841069 +transform_check_for_dangling_tasks,8841070 diff --git a/server/src/main/resources/transport/upper_bounds/9.0.csv b/server/src/main/resources/transport/upper_bounds/9.0.csv index 8ad2ed1a4cacf..c11e6837bb813 100644 --- a/server/src/main/resources/transport/upper_bounds/9.0.csv +++ b/server/src/main/resources/transport/upper_bounds/9.0.csv @@ -1 +1 @@ -initial_elasticsearch_9_0_8,9000017 +transform_check_for_dangling_tasks,9000018 diff --git a/server/src/main/resources/transport/upper_bounds/9.1.csv b/server/src/main/resources/transport/upper_bounds/9.1.csv index 1cea5dc4d929b..80b97d85f7511 100644 --- a/server/src/main/resources/transport/upper_bounds/9.1.csv +++ b/server/src/main/resources/transport/upper_bounds/9.1.csv @@ -1 +1 @@ -initial_elasticsearch_9_1_5,9112008 +transform_check_for_dangling_tasks,9112009 diff --git a/server/src/main/resources/transport/upper_bounds/9.2.csv b/server/src/main/resources/transport/upper_bounds/9.2.csv index b1209b927d8a5..e60434a3e2189 100644 --- a/server/src/main/resources/transport/upper_bounds/9.2.csv +++ b/server/src/main/resources/transport/upper_bounds/9.2.csv @@ -1 +1 @@ -inference_api_openai_embeddings_headers,9169000 +sampling_configuration,9173000 From 1f64ae01ec0aea1bf39af22df9fb6663055dec5e Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 26 Sep 2025 08:57:38 -0500 Subject: [PATCH 10/14] adding some comments --- .../indices/IndicesQueryCacheTests.java | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 3aa77ab476b3e..8505c9c866bde 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -469,7 +469,7 @@ public void testGetStatsMemory() throws Exception { for (int i = 0; i < shard1Queries; ++i) { shard1Segment1Searcher.count(new DummyQuery("ingest1-" + i, largeQuerySize)); } - long shard1Segment1CacheMemory = calculateActualCacheMemoryForShard(shard1Queries, largeQuerySize, shard1Segment1Docs); + long shard1Segment1CacheMemory = calculateActualCacheMemoryForSegment(shard1Queries, largeQuerySize, shard1Segment1Docs); assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(shard1Segment1CacheMemory)); assertThat(cache.getStats(shard2).getMemorySizeInBytes(), equalTo(0L)); for (int i = 0; i < shard2Queries; ++i) { @@ -483,12 +483,12 @@ public void testGetStatsMemory() throws Exception { assertThat(cache.getStats(shard1).getMemorySizeInBytes(), lessThan(shard1Segment1CacheMemory)); long shard1CacheBytes = cache.getStats(shard1).getMemorySizeInBytes(); long shard2CacheBytes = cache.getStats(shard2).getMemorySizeInBytes(); - long shard2Segment1CacheMemory = calculateActualCacheMemoryForShard(shard2Queries, smallQuerySize, shard2Segment1Docs); + long shard2Segment1CacheMemory = calculateActualCacheMemoryForSegment(shard2Queries, smallQuerySize, shard2Segment1Docs); long totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory; // Each shard has some fixed overhead that we need to account for: - long shard1Overhead = calculateOverheadForShard(shard1Queries, shard1Segment1Docs); - long shard2Overhead = calculateOverheadForShard(shard2Queries, shard2Segment1Docs); + long shard1Overhead = calculateOverheadForSegment(shard1Queries, shard1Segment1Docs); + long shard2Overhead = calculateOverheadForSegment(shard2Queries, shard2Segment1Docs); long totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); /* * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each shard @@ -505,10 +505,10 @@ public void testGetStatsMemory() throws Exception { for (int i = 0; i < shard1Queries; ++i) { shard1Segment2Searcher.count(new DummyQuery("ingest3-" + i, largeQuerySize)); } - long shard1Segment2CacheMemory = calculateActualCacheMemoryForShard(shard1Queries, largeQuerySize, shard1Segment2Docs); + long shard1Segment2CacheMemory = calculateActualCacheMemoryForSegment(shard1Queries, largeQuerySize, shard1Segment2Docs); totalMemory = shard1Segment1CacheMemory + shard2Segment1CacheMemory + shard1Segment2CacheMemory; // Each shard has some fixed overhead that we need to account for: - shard1Overhead = shard1Overhead + calculateOverheadForShard(shard1Queries, shard1Segment2Docs); + shard1Overhead = shard1Overhead + calculateOverheadForSegment(shard1Queries, shard1Segment2Docs); totalMemoryMinusOverhead = totalMemory - (shard1Overhead + shard2Overhead); /* * Note that the expected amount of memory we're calculating is based on the proportion of the number of queries to each segment. @@ -526,7 +526,7 @@ public void testGetStatsMemory() throws Exception { assertThat(cache.getStats(shard1).getMemorySizeInBytes(), equalTo(0L)); assertThat( cache.getStats(shard2).getMemorySizeInBytes(), - equalTo(calculateActualCacheMemoryForShard(maxCacheSize, smallQuerySize, shard2Segment1Docs)) + equalTo(calculateActualCacheMemoryForSegment(maxCacheSize, smallQuerySize, shard2Segment1Docs)) ); IOUtils.close(closeableList); @@ -535,12 +535,20 @@ public void testGetStatsMemory() throws Exception { cache.close(); } - private long calculateActualCacheMemoryForShard(long queryCount, long querySize, long numDocs) { - return queryCount * (querySize + 24) + calculateOverheadForShard(queryCount, numDocs); + /* + * This calculates the memory that actually used by a segment in the IndicesQueryCache. It assumes queryCount queries are made to the + * segment, and query is querySize bytes in size. It assumes that the shard contains numDocs documents. + */ + private long calculateActualCacheMemoryForSegment(long queryCount, long querySize, long numDocs) { + return (queryCount * (querySize + 24)) + calculateOverheadForSegment(queryCount, numDocs); } - private long calculateOverheadForShard(long queryCount, long numDocs) { - return queryCount * (112 + 8 * ((numDocs - 1) / 64)); + /* + * This computes the part of the recorded IndicesQueryCache memory that is assigned to a segment and *not* divided up proportionally + * when the cache reports the memory usage of each shard. + */ + private long calculateOverheadForSegment(long queryCount, long numDocs) { + return queryCount * (112 + (8 * ((numDocs - 1) / 64))); } /* From 346b0d7bae186a9f78948aa10b772dce4c28080a Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 26 Sep 2025 11:13:26 -0500 Subject: [PATCH 11/14] updating comments --- .../java/org/elasticsearch/indices/IndicesQueryCache.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java index 15973889083e1..275d733b021b2 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java @@ -95,8 +95,8 @@ private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { } /* - * We have some shared ram usage that we try to distribute proportionally to the number of documents in the cache cache for each - * shard. + * We have some shared ram usage that we try to distribute proportionally to the number of segment-requestss in the cache cache for + * each shard. */ // TODO avoid looping over all local shards here - see https://github.com/elastic/elasticsearch/issues/97222 long totalItemsInCache = 0L; @@ -130,7 +130,8 @@ private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { } else { /* * some shards have nonzero cache footprint, so we apportion the size of the shared bytes proportionally to the number of - * documents in the cache for this shard + * segment-requests in the cache for this shard (the number and size of documents associated with those requests is irrelevant + * for this calculation). */ additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed * itemsInCacheForShard / totalItemsInCache); } From 34a3df95e6abdcfa4c0ebfd583bb1fe95ed47ba1 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 26 Sep 2025 13:41:25 -0500 Subject: [PATCH 12/14] very minor test code cleanup --- .../org/elasticsearch/indices/IndicesQueryCacheTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java index 8505c9c866bde..5629fe81511c6 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesQueryCacheTests.java @@ -26,6 +26,7 @@ import org.apache.lucene.search.ScorerSupplier; import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; +import org.apache.lucene.util.Accountable; import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.IOUtils; @@ -46,7 +47,7 @@ public class IndicesQueryCacheTests extends ESTestCase { - private static class DummyQuery extends Query implements org.apache.lucene.util.Accountable { + private static class DummyQuery extends Query implements Accountable { private final String id; private final long sizeInCache; @@ -427,7 +428,6 @@ public void testDelegatesScorerSupplier() throws Exception { cache.close(); } - @SuppressWarnings("cast") 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 From bc56cf854c85e0f0f4afad2f110a3bb4e86bf361 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 26 Sep 2025 13:50:12 -0500 Subject: [PATCH 13/14] fixing typos --- .../java/org/elasticsearch/indices/IndicesQueryCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java index 275d733b021b2..73f17d63682ac 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java @@ -95,8 +95,8 @@ private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { } /* - * We have some shared ram usage that we try to distribute proportionally to the number of segment-requestss in the cache cache for - * each shard. + * 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; From e29dd3a1e8a9227363449993192ccc7e3227ad1e Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 26 Sep 2025 14:07:53 -0500 Subject: [PATCH 14/14] adding wording to a comment --- .../java/org/elasticsearch/indices/IndicesQueryCache.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java index 73f17d63682ac..6c4694b4f8235 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java @@ -129,9 +129,13 @@ private long getShareOfAdditionalRamBytesUsed(long itemsInCacheForShard) { additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed / shardCount); } else { /* - * some shards have nonzero cache footprint, so we apportion the size of the shared bytes proportionally to the number of + * Some shards have nonzero cache footprint, so we apportion the size of the shared bytes proportionally to the number of * segment-requests in the cache for this shard (the number and size of documents associated with those requests is irrelevant * for this calculation). + * Note that this was a somewhat arbitrary decision. Calculating it by number of documents might have been better. Calculating + * it by number of documents weighted by size would also be good, but possibly more expensive. But the decision to attribute + * memory proportionally to the number of segment-requests was made a long time ago, and we're sticking with that here for the + * sake of consistency and backwards compatibility. */ additionalRamBytesUsed = Math.round((double) sharedRamBytesUsed * itemsInCacheForShard / totalItemsInCache); }