Avoid negative memory result in IndicesQueryCache stats calculation

Signed-off-by: Daniel Widdis <[email protected]>
opensearch-project · Mar 31, 2023 · 4555cc0 · 4555cc0
1 parent 805e6d9
commit 4555cc0
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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);
 
         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);
 
         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);
 
         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);
 
         cache.onClose(shard);
 
@@ -187,6 +192,7 @@ public void testBasics() throws IOException {
         assertEquals(0L, stats.getCacheCount());
         assertEquals(0L, stats.getHitCount());
         assertEquals(0L, stats.getMissCount());
+        assertEquals(0L, stats.getMemorySizeInBytes());
 
         cache.close(); // this triggers some assertions
     }