diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ded8bd0fe36..08ae27bb583fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827)) - Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944)) - Support OpenSSL Provider with default Netty allocator ([#5460](https://github.com/opensearch-project/OpenSearch/pull/5460)) +- Avoid negative memory result in IndicesQueryCache stats calculation ([#6917](https://github.com/opensearch-project/OpenSearch/pull/6917)) ### Security diff --git a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java index 78970267f67c2..2669da3f417c3 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesQueryCache.java @@ -128,13 +128,17 @@ public QueryCacheStats getStats(ShardId shard) { // We also have some shared ram usage that we try to distribute to // proportionally to their number of cache entries of each shard - long totalSize = 0; - for (QueryCacheStats s : stats.values()) { - totalSize += s.getCacheSize(); + if (stats.isEmpty()) { + shardStats.add(new QueryCacheStats(sharedRamBytesUsed, 0, 0, 0, 0)); + } else { + long totalSize = 0; + for (QueryCacheStats s : stats.values()) { + totalSize += s.getCacheSize(); + } + final double weight = totalSize == 0 ? 1d / stats.size() : ((double) shardStats.getCacheSize()) / totalSize; + final long additionalRamBytesUsed = Math.round(weight * sharedRamBytesUsed); + shardStats.add(new QueryCacheStats(additionalRamBytesUsed, 0, 0, 0, 0)); } - final double weight = totalSize == 0 ? 1d / stats.size() : ((double) shardStats.getCacheSize()) / totalSize; - final long additionalRamBytesUsed = Math.round(weight * sharedRamBytesUsed); - shardStats.add(new QueryCacheStats(additionalRamBytesUsed, 0, 0, 0, 0)); return shardStats; } diff --git a/server/src/test/java/org/opensearch/indices/IndicesQueryCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesQueryCacheTests.java index b1a44d94aa0e1..5d293e6c598f6 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesQueryCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesQueryCacheTests.java @@ -143,6 +143,7 @@ public void testBasics() throws IOException { assertEquals(0L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); assertEquals(0L, stats.getMissCount()); + assertEquals(0L, stats.getMemorySizeInBytes()); assertEquals(1, s.count(new DummyQuery(0))); @@ -151,6 +152,7 @@ public void testBasics() throws IOException { assertEquals(1L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); assertEquals(1L, stats.getMissCount()); + assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE); for (int i = 1; i < 20; ++i) { assertEquals(1, s.count(new DummyQuery(i))); @@ -161,6 +163,7 @@ public void testBasics() throws IOException { assertEquals(20L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); assertEquals(20L, stats.getMissCount()); + assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE); s.count(new DummyQuery(10)); @@ -169,6 +172,7 @@ public void testBasics() throws IOException { assertEquals(20L, stats.getCacheCount()); assertEquals(1L, stats.getHitCount()); assertEquals(20L, stats.getMissCount()); + assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE); IOUtils.close(r, dir); @@ -178,6 +182,7 @@ public void testBasics() throws IOException { assertEquals(20L, stats.getCacheCount()); assertEquals(1L, stats.getHitCount()); assertEquals(20L, stats.getMissCount()); + assertTrue(stats.getMemorySizeInBytes() > 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE); cache.onClose(shard); @@ -187,6 +192,7 @@ public void testBasics() throws IOException { assertEquals(0L, stats.getCacheCount()); assertEquals(0L, stats.getHitCount()); assertEquals(0L, stats.getMissCount()); + assertTrue(stats.getMemorySizeInBytes() >= 0L && stats.getMemorySizeInBytes() < Long.MAX_VALUE); cache.close(); // this triggers some assertions } @@ -227,12 +233,14 @@ public void testTwoShards() throws IOException { assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(1L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); QueryCacheStats stats2 = cache.getStats(shard2); assertEquals(0L, stats2.getCacheSize()); assertEquals(0L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); assertEquals(0L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); assertEquals(1, s2.count(new DummyQuery(0))); @@ -241,12 +249,14 @@ public void testTwoShards() throws IOException { assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(1L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); stats2 = cache.getStats(shard2); assertEquals(1L, stats2.getCacheSize()); assertEquals(1L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); assertEquals(1L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); for (int i = 0; i < 20; ++i) { assertEquals(1, s2.count(new DummyQuery(i))); @@ -257,12 +267,14 @@ public void testTwoShards() throws IOException { assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(1L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); assertEquals(20L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); IOUtils.close(r1, dir1); @@ -272,12 +284,14 @@ public void testTwoShards() throws IOException { assertEquals(1L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(1L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); assertEquals(20L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); cache.onClose(shard1); @@ -287,12 +301,14 @@ public void testTwoShards() throws IOException { assertEquals(0L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(0L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); stats2 = cache.getStats(shard2); assertEquals(10L, stats2.getCacheSize()); assertEquals(20L, stats2.getCacheCount()); assertEquals(1L, stats2.getHitCount()); assertEquals(20L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); IOUtils.close(r2, dir2); cache.onClose(shard2); @@ -303,12 +319,14 @@ public void testTwoShards() throws IOException { assertEquals(0L, stats1.getCacheCount()); assertEquals(0L, stats1.getHitCount()); assertEquals(0L, stats1.getMissCount()); + assertTrue(stats1.getMemorySizeInBytes() >= 0L && stats1.getMemorySizeInBytes() < Long.MAX_VALUE); stats2 = cache.getStats(shard2); assertEquals(0L, stats2.getCacheSize()); assertEquals(0L, stats2.getCacheCount()); assertEquals(0L, stats2.getHitCount()); assertEquals(0L, stats2.getMissCount()); + assertTrue(stats2.getMemorySizeInBytes() >= 0L && stats2.getMemorySizeInBytes() < Long.MAX_VALUE); cache.close(); // this triggers some assertions }