Addressed Sagar's other comment

Signed-off-by: Peter Alfonsi <[email protected]>
peteralfonsi · Feb 27, 2024 · c34c218 · c34c218
1 parent d579c51
commit c34c218
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 6 deletions.
diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java
@@ -115,6 +115,8 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {
     private final Serializer<K, byte[]> keySerializer;
     private final Serializer<V, byte[]> valueSerializer;
 
+    public final static String TIER_DIMENSION_VALUE = "disk";
+
     /**
      * Used in computeIfAbsent to synchronize loading of a given key. This is needed as ehcache doesn't provide a
      * computeIfAbsent method.
@@ -154,7 +156,7 @@ private EhcacheDiskCache(Builder<K, V> builder) {
             this.valueSerializer);
         this.cache = buildCache(Duration.ofMillis(expireAfterAccess.getMillis()), builder);
         List<String> dimensionNames = Objects.requireNonNull(builder.dimensionNames, "Dimension names can't be null");
-        this.stats = new MultiDimensionCacheStats(dimensionNames, CacheStatsDimension.TIER_DIMENSION_VALUE_DISK);
+        this.stats = new MultiDimensionCacheStats(dimensionNames, TIER_DIMENSION_VALUE);
     }
 
     private Cache<ICacheKey, byte[]> buildCache(Duration expireAfterAccess, Builder<K, V> builder) {

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
@@ -632,8 +632,8 @@ public void testGetStatsByTierName() throws Exception {
             for (int i = 0; i < randomKeys; i++) {
                 ehcacheTest.put(getICacheKey(UUID.randomUUID().toString()), UUID.randomUUID().toString());
             }
-            assertEquals(randomKeys, ehcacheTest.stats().getEntriesByDimensions(List.of(new CacheStatsDimension(CacheStatsDimension.TIER_DIMENSION_NAME, CacheStatsDimension.TIER_DIMENSION_VALUE_DISK))));
-            assertEquals(0, ehcacheTest.stats().getEntriesByDimensions(List.of(new CacheStatsDimension(CacheStatsDimension.TIER_DIMENSION_NAME, CacheStatsDimension.TIER_DIMENSION_VALUE_ON_HEAP))));
+            assertEquals(randomKeys, ehcacheTest.stats().getEntriesByDimensions(List.of(new CacheStatsDimension(CacheStatsDimension.TIER_DIMENSION_NAME, EhcacheDiskCache.TIER_DIMENSION_VALUE))));
+            assertEquals(0, ehcacheTest.stats().getEntriesByDimensions(List.of(new CacheStatsDimension(CacheStatsDimension.TIER_DIMENSION_NAME, "other_tier_value"))));
 
             ehcacheTest.close();
         }

diff --git a/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsDimension.java b/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsDimension.java
@@ -18,8 +18,6 @@
 public class CacheStatsDimension implements Writeable {
     // Values for tier dimensions, that are reused across CacheStats implementations
     public static final String TIER_DIMENSION_NAME = "tier";
-    public static final String TIER_DIMENSION_VALUE_ON_HEAP = "on_heap";
-    public static final String TIER_DIMENSION_VALUE_DISK = "disk";
     public final String dimensionName;
     public final String dimensionValue;
     public CacheStatsDimension(String dimensionName, String dimensionValue) {

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
@@ -45,6 +45,7 @@ public class OpenSearchOnHeapCache<K, V> implements ICache<K, V>, RemovalListene
     private final Cache<ICacheKey<K>, V> cache;
     private final CacheStats stats;
     private final RemovalListener<ICacheKey<K>, V> removalListener;
+    public static final String TIER_DIMENSION_VALUE = "on_heap";
 
     public OpenSearchOnHeapCache(Builder<K, V> builder) {
         CacheBuilder<ICacheKey<K>, V> cacheBuilder = CacheBuilder.<ICacheKey<K>, V>builder()
@@ -56,7 +57,7 @@ public OpenSearchOnHeapCache(Builder<K, V> builder) {
         }
         cache = cacheBuilder.build();
         List<String> dimensionNames = Objects.requireNonNull(builder.dimensionNames, "Dimension names can't be null");
-        this.stats = new MultiDimensionCacheStats(dimensionNames, CacheStatsDimension.TIER_DIMENSION_VALUE_ON_HEAP);
+        this.stats = new MultiDimensionCacheStats(dimensionNames, TIER_DIMENSION_VALUE);
         this.removalListener = builder.getRemovalListener();
     }