From 60934a4023b640e5288629906391e115e8eb1f67 Mon Sep 17 00:00:00 2001
From: Peter Alfonsi <petealft@amazon.com>
Date: Mon, 3 Jun 2024 13:17:18 -0700
Subject: [PATCH] Addressed Sorabh's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
---
 .../common/tier/TieredSpilloverCacheStatsIT.java  | 15 +--------------
 .../tier/TieredSpilloverCacheStatsHolder.java     |  6 ++++++
 .../opensearch/indices/IndicesRequestCache.java   |  1 +
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/modules/cache-common/src/internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java b/modules/cache-common/src/internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java
index 98771c7538aea..537caccbac652 100644
--- a/modules/cache-common/src/internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java
+++ b/modules/cache-common/src/internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java
@@ -8,8 +8,6 @@
 
 package org.opensearch.cache.common.tier;
 
-import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
-
 import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
 import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
 import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
@@ -22,13 +20,11 @@
 import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.index.cache.request.RequestCacheStats;
 import org.opensearch.index.query.QueryBuilders;
 import org.opensearch.indices.IndicesRequestCache;
 import org.opensearch.plugins.Plugin;
 import org.opensearch.test.OpenSearchIntegTestCase;
-import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
 import org.opensearch.test.hamcrest.OpenSearchAssertions;
 
 import java.io.IOException;
@@ -47,26 +43,17 @@
 
 // Use a single data node to simplify accessing cache stats across different shards.
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 0)
-public class TieredSpilloverCacheStatsIT extends ParameterizedStaticSettingsOpenSearchIntegTestCase {
+public class TieredSpilloverCacheStatsIT extends OpenSearchIntegTestCase {
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(TieredSpilloverCachePlugin.class, TieredSpilloverCacheIT.MockDiskCachePlugin.class);
     }
 
-    public TieredSpilloverCacheStatsIT(Settings settings) {
-        super(settings);
-    }
-
     private final String HEAP_CACHE_SIZE_STRING = "10000B";
     private final int HEAP_CACHE_SIZE = 10_000;
     private final String index1Name = "index1";
     private final String index2Name = "index2";
 
-    @ParametersFactory
-    public static Collection<Object[]> parameters() {
-        return Arrays.<Object[]>asList(new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "true").build() });
-    }
-
     /**
      * Test aggregating by indices
      */
diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsHolder.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsHolder.java
index 7c6e052494088..b40724430454b 100644
--- a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsHolder.java
+++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsHolder.java
@@ -105,6 +105,12 @@ public void incrementMisses(List<String> dimensionValues) {
         internalIncrement(dimensionValues, missIncrementer, true);
     }
 
+    /**
+     * This method shouldn't be used in this class. Instead, use incrementEvictions(dimensionValues, includeInTotal)
+     * which specifies whether the eviction should be included in the cache's total evictions, or if it should
+     * just count towards that tier's evictions.
+     * @param dimensionValues The dimension values
+     */
     @Override
     public void incrementEvictions(List<String> dimensionValues) {
         throw new UnsupportedOperationException(
diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java
index 845995fb657fe..57f7e402536f2 100644
--- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java
+++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java
@@ -753,6 +753,7 @@ private synchronized void cleanCache(double stalenessThreshold) {
                     if (cacheEntity == null) {
                         // If cache entity is null, it means that index or shard got deleted/closed meanwhile.
                         // So we will delete this key.
+                        dimensionListsToDrop.add(key.dimensions);
                         iterator.remove();
                     } else {
                         CleanupKey cleanupKey = new CleanupKey(cacheEntity, delegatingKey.readerCacheKeyId);