Removed count() from NoopCacheStatsHolder

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
opensearch-project · Apr 18, 2024 · 8b63a72 · 8b63a72
1 parent bafb918
commit 8b63a72
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 21 deletions.
diff --git a/server/src/main/java/org/opensearch/common/cache/stats/NoopCacheStatsHolder.java b/server/src/main/java/org/opensearch/common/cache/stats/NoopCacheStatsHolder.java
@@ -8,20 +8,15 @@
 
 package org.opensearch.common.cache.stats;
 
-import org.opensearch.common.metrics.CounterMetric;
-
 import java.util.List;
 
 /**
  * A dummy version of CacheStatsHolder, which cache implementations use when FeatureFlags.PLUGGABLE_CACHES is false.
- * Returns all-zero stats when calling getImmutableCacheStatsHolder(). Does keep track of entries for use in ICache.count().
+ * Returns all-zero stats when calling getImmutableCacheStatsHolder(). Always returns 0 for count().
  */
 public class NoopCacheStatsHolder implements CacheStatsHolder {
-    private CounterMetric entries;
 
-    public NoopCacheStatsHolder() {
-        this.entries = new CounterMetric();
-    }
+    public NoopCacheStatsHolder() {}
 
     @Override
     public void incrementHits(List<String> dimensionValues) {}
@@ -39,23 +34,17 @@ public void incrementSizeInBytes(List<String> dimensionValues, long amountBytes)
     public void decrementSizeInBytes(List<String> dimensionValues, long amountBytes) {}
 
     @Override
-    public void incrementEntries(List<String> dimensionValues) {
-        entries.inc();
-    }
+    public void incrementEntries(List<String> dimensionValues) {}
 
     @Override
-    public void decrementEntries(List<String> dimensionValues) {
-        entries.dec();
-    }
+    public void decrementEntries(List<String> dimensionValues) {}
 
     @Override
-    public void reset() {
-        this.entries = new CounterMetric();
-    }
+    public void reset() {}
 
     @Override
     public long count() {
-        return entries.count();
+        return 0;
     }
 
     @Override

diff --git a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java
@@ -54,6 +54,8 @@ public class OpenSearchOnHeapCache<K, V> implements ICache<K, V>, RemovalListene
     private final List<String> dimensionNames;
     private final ToLongBiFunction<ICacheKey<K>, V> weigher;
 
+    private final Settings settings;
+
     public OpenSearchOnHeapCache(Builder<K, V> builder) {
         CacheBuilder<ICacheKey<K>, V> cacheBuilder = CacheBuilder.<ICacheKey<K>, V>builder()
             .setMaximumWeight(builder.getMaxWeightInBytes())
@@ -64,13 +66,14 @@ public OpenSearchOnHeapCache(Builder<K, V> builder) {
         }
         cache = cacheBuilder.build();
         this.dimensionNames = Objects.requireNonNull(builder.dimensionNames, "Dimension names can't be null");
-        if (FeatureFlags.PLUGGABLE_CACHE_SETTING.get(builder.getSettings())) {
-            this.cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames);
-        } else {
+        if (useNoopStats(builder.getSettings())) {
             this.cacheStatsHolder = new NoopCacheStatsHolder();
+        } else {
+            this.cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames);
         }
         this.removalListener = builder.getRemovalListener();
         this.weigher = builder.getWeigher();
+        this.settings = builder.getSettings();
     }
 
     @Override
@@ -127,7 +130,11 @@ public Iterable<ICacheKey<K>> keys() {
 
     @Override
     public long count() {
-        return cacheStatsHolder.count();
+        if (useNoopStats(settings)) {
+            return cache.count();
+        } else {
+            return cacheStatsHolder.count();
+        }
     }
 
     @Override
@@ -158,6 +165,11 @@ public void onRemoval(RemovalNotification<ICacheKey<K>, V> notification) {
         }
     }
 
+    private boolean useNoopStats(Settings settings) {
+        // Use noop stats when pluggable caching is off
+        return !FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings);
+    }
+
     /**
      * Factory to create OpenSearchOnheap cache.
      */