Fixes tests using incorrect null levels

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
opensearch-project · Apr 30, 2024 · 2609cf7 · 2609cf7
1 parent 79331c4
commit 2609cf7
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 17 deletions.
diff --git a/...ns/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java b/...ns/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java
@@ -829,15 +829,16 @@ public void testInvalidateWithDropDimensions() throws Exception {
 
             ICacheKey<String> keyToDrop = keysAdded.get(0);
 
-            ImmutableCacheStats snapshot = ehCacheDiskCachingTier.stats().getStatsForDimensionValues(keyToDrop.dimensions);
+            String[] levels = dimensionNames.toArray(new String[0]);
+            ImmutableCacheStats snapshot = ehCacheDiskCachingTier.stats(levels).getStatsForDimensionValues(keyToDrop.dimensions);
             assertNotNull(snapshot);
 
             keyToDrop.setDropStatsForDimensions(true);
             ehCacheDiskCachingTier.invalidate(keyToDrop);
 
             // Now assert the stats are gone for any key that has this combination of dimensions, but still there otherwise
             for (ICacheKey<String> keyAdded : keysAdded) {
-                snapshot = ehCacheDiskCachingTier.stats().getStatsForDimensionValues(keyAdded.dimensions);
+                snapshot = ehCacheDiskCachingTier.stats(levels).getStatsForDimensionValues(keyAdded.dimensions);
                 if (keyAdded.dimensions.equals(keyToDrop.dimensions)) {
                     assertNull(snapshot);
                 } else {

diff --git a/server/src/main/java/org/opensearch/common/cache/ICache.java b/server/src/main/java/org/opensearch/common/cache/ICache.java
@@ -45,12 +45,12 @@ public interface ICache<K, V> extends Closeable {
 
     void refresh();
 
-    // Return all stats without aggregation.
+    // Return total stats only
     default ImmutableCacheStatsHolder stats() {
         return stats(null);
     }
 
-    // Return stats aggregated by the provided levels. If levels is null, do not aggregate and return all stats.
+    // Return stats aggregated by the provided levels. If levels is null or an empty array, return total stats only.
     ImmutableCacheStatsHolder stats(String[] levels);
 
     /**

diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java
@@ -826,8 +826,8 @@ long count() {
     /**
      * Returns the current cache stats. Pkg-private for testing.
      */
-    ImmutableCacheStatsHolder stats() {
-        return cache.stats();
+    ImmutableCacheStatsHolder stats(String[] levels) {
+        return cache.stats(levels);
     }
 
     int numRegisteredCloseListeners() { // for testing

diff --git a/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java b/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java
@@ -29,10 +29,11 @@ public class ImmutableCacheStatsHolderTests extends OpenSearchTestCase {
 
     public void testSerialization() throws Exception {
         List<String> dimensionNames = List.of("dim1", "dim2", "dim3");
+        String[] levels = dimensionNames.toArray(new String[0]);
         DefaultCacheStatsHolder statsHolder = new DefaultCacheStatsHolder(dimensionNames, storeName);
         Map<String, List<String>> usedDimensionValues = DefaultCacheStatsHolderTests.getUsedDimensionValues(statsHolder, 10);
         DefaultCacheStatsHolderTests.populateStats(statsHolder, usedDimensionValues, 100, 10);
-        ImmutableCacheStatsHolder stats = statsHolder.getImmutableCacheStatsHolder(null);
+        ImmutableCacheStatsHolder stats = statsHolder.getImmutableCacheStatsHolder(levels);
         assertNotEquals(0, stats.getStatsRoot().children.size());
 
         BytesStreamOutput os = new BytesStreamOutput();
@@ -57,19 +58,20 @@ public void testSerialization() throws Exception {
 
     public void testEquals() throws Exception {
         List<String> dimensionNames = List.of("dim1", "dim2", "dim3");
+        String[] levels = dimensionNames.toArray(new String[0]);
         DefaultCacheStatsHolder statsHolder = new DefaultCacheStatsHolder(dimensionNames, storeName);
         DefaultCacheStatsHolder differentStoreNameStatsHolder = new DefaultCacheStatsHolder(dimensionNames, "nonMatchingStoreName");
         DefaultCacheStatsHolder nonMatchingStatsHolder = new DefaultCacheStatsHolder(dimensionNames, storeName);
         Map<String, List<String>> usedDimensionValues = DefaultCacheStatsHolderTests.getUsedDimensionValues(statsHolder, 10);
         DefaultCacheStatsHolderTests.populateStats(List.of(statsHolder, differentStoreNameStatsHolder), usedDimensionValues, 100, 10);
         DefaultCacheStatsHolderTests.populateStats(nonMatchingStatsHolder, usedDimensionValues, 100, 10);
-        ImmutableCacheStatsHolder stats = statsHolder.getImmutableCacheStatsHolder(null);
+        ImmutableCacheStatsHolder stats = statsHolder.getImmutableCacheStatsHolder(levels);
 
-        ImmutableCacheStatsHolder secondStats = statsHolder.getImmutableCacheStatsHolder(null);
+        ImmutableCacheStatsHolder secondStats = statsHolder.getImmutableCacheStatsHolder(levels);
         assertEquals(stats, secondStats);
-        ImmutableCacheStatsHolder nonMatchingStats = nonMatchingStatsHolder.getImmutableCacheStatsHolder(null);
+        ImmutableCacheStatsHolder nonMatchingStats = nonMatchingStatsHolder.getImmutableCacheStatsHolder(levels);
         assertNotEquals(stats, nonMatchingStats);
-        ImmutableCacheStatsHolder differentStoreNameStats = differentStoreNameStatsHolder.getImmutableCacheStatsHolder(null);
+        ImmutableCacheStatsHolder differentStoreNameStats = differentStoreNameStatsHolder.getImmutableCacheStatsHolder(levels);
         assertNotEquals(stats, differentStoreNameStats);
     }
 

diff --git a/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java b/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java
@@ -145,16 +145,16 @@ public void testInvalidateWithDropDimensions() throws Exception {
         }
 
         ICacheKey<String> keyToDrop = keysAdded.get(0);
-
-        ImmutableCacheStats snapshot = cache.stats().getStatsForDimensionValues(keyToDrop.dimensions);
+        String[] levels = dimensionNames.toArray(new String[0]);
+        ImmutableCacheStats snapshot = cache.stats(levels).getStatsForDimensionValues(keyToDrop.dimensions);
         assertNotNull(snapshot);
 
         keyToDrop.setDropStatsForDimensions(true);
         cache.invalidate(keyToDrop);
 
         // Now assert the stats are gone for any key that has this combination of dimensions, but still there otherwise
         for (ICacheKey<String> keyAdded : keysAdded) {
-            snapshot = cache.stats().getStatsForDimensionValues(keyAdded.dimensions);
+            snapshot = cache.stats(levels).getStatsForDimensionValues(keyAdded.dimensions);
             if (keyAdded.dimensions.equals(keyToDrop.dimensions)) {
                 assertNull(snapshot);
             } else {

diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java
@@ -89,7 +89,9 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.opensearch.indices.IndicesRequestCache.INDEX_DIMENSION_NAME;
 import static org.opensearch.indices.IndicesRequestCache.INDICES_REQUEST_CACHE_STALENESS_THRESHOLD_SETTING;
+import static org.opensearch.indices.IndicesRequestCache.SHARD_ID_DIMENSION_NAME;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -799,6 +801,7 @@ private String getReaderCacheKeyId(DirectoryReader reader) {
 
     public void testClosingIndexWipesStats() throws Exception {
         IndicesService indicesService = getInstanceFromNode(IndicesService.class);
+        String[] levels = { INDEX_DIMENSION_NAME, SHARD_ID_DIMENSION_NAME };
         // Create two indices each with multiple shards
         int numShards = 3;
         Settings indexSettings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards).build();
@@ -873,8 +876,8 @@ public void testClosingIndexWipesStats() throws Exception {
                 ShardId shardId = indexService.getShard(i).shardId();
                 List<String> dimensionValues = List.of(shardId.getIndexName(), shardId.toString());
                 initialDimensionValues.add(dimensionValues);
-                ImmutableCacheStatsHolder holder = cache.stats();
-                ImmutableCacheStats snapshot = cache.stats().getStatsForDimensionValues(dimensionValues);
+                ImmutableCacheStatsHolder holder = cache.stats(levels);
+                ImmutableCacheStats snapshot = cache.stats(levels).getStatsForDimensionValues(dimensionValues);
                 assertNotNull(snapshot);
                 // check the values are not empty by confirming entries != 0, this should always be true since the missed value is loaded
                 // into the cache
@@ -895,7 +898,7 @@ public void testClosingIndexWipesStats() throws Exception {
 
         // Now stats for the closed index should be gone
         for (List<String> dimensionValues : initialDimensionValues) {
-            ImmutableCacheStats snapshot = cache.stats().getStatsForDimensionValues(dimensionValues);
+            ImmutableCacheStats snapshot = cache.stats(levels).getStatsForDimensionValues(dimensionValues);
             if (dimensionValues.get(0).equals(indexToCloseName)) {
                 assertNull(snapshot);
             } else {