From 2609cf7a63222f862ed59facb54893a3f16e6d54 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 29 Apr 2024 19:26:44 -0700 Subject: [PATCH] Fixes tests using incorrect null levels Signed-off-by: Peter Alfonsi --- .../cache/store/disk/EhCacheDiskCacheTests.java | 5 +++-- .../java/org/opensearch/common/cache/ICache.java | 4 ++-- .../org/opensearch/indices/IndicesRequestCache.java | 4 ++-- .../cache/stats/ImmutableCacheStatsHolderTests.java | 12 +++++++----- .../cache/store/OpenSearchOnHeapCacheTests.java | 6 +++--- .../opensearch/indices/IndicesRequestCacheTests.java | 9 ++++++--- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java index f93b09cf2d4f4..f2bfe1209a4c7 100644 --- a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java +++ b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java @@ -829,7 +829,8 @@ public void testInvalidateWithDropDimensions() throws Exception { ICacheKey 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); @@ -837,7 +838,7 @@ public void testInvalidateWithDropDimensions() throws Exception { // Now assert the stats are gone for any key that has this combination of dimensions, but still there otherwise for (ICacheKey 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 index bc69ccee0c2fb..f5dd644db6d6b 100644 --- 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 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 index f5e7ba26539a6..35826d45f969f 100644 --- 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 index 46483e92b76bf..285840a3451c6 100644 --- 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 dimensionNames = List.of("dim1", "dim2", "dim3"); + String[] levels = dimensionNames.toArray(new String[0]); DefaultCacheStatsHolder statsHolder = new DefaultCacheStatsHolder(dimensionNames, storeName); Map> 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 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> 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 index 3208fde306e5a..f5885d03f1850 100644 --- 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,8 +145,8 @@ public void testInvalidateWithDropDimensions() throws Exception { } ICacheKey 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); @@ -154,7 +154,7 @@ public void testInvalidateWithDropDimensions() throws Exception { // Now assert the stats are gone for any key that has this combination of dimensions, but still there otherwise for (ICacheKey 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 index 9e2c33998abd6..bbf2867a0087c 100644 --- 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 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 dimensionValues : initialDimensionValues) { - ImmutableCacheStats snapshot = cache.stats().getStatsForDimensionValues(dimensionValues); + ImmutableCacheStats snapshot = cache.stats(levels).getStatsForDimensionValues(dimensionValues); if (dimensionValues.get(0).equals(indexToCloseName)) { assertNull(snapshot); } else {