Remove PLUGGABLE_CACHE feature flag

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
opensearch-project · Feb 12, 2025 · f8ebfd4 · f8ebfd4
1 parent d0a65d3
commit f8ebfd4
Show file tree

Hide file tree

Showing 17 changed files with 59 additions and 210 deletions.
diff --git a/...internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheBaseIT.java b/...internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheBaseIT.java
@@ -12,14 +12,12 @@
 import org.opensearch.common.cache.settings.CacheSettings;
 import org.opensearch.common.cache.store.OpenSearchOnHeapCache;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
 public class TieredSpilloverCacheBaseIT extends OpenSearchIntegTestCase {
 
     public Settings defaultSettings(String onHeapCacheSizeInBytesOrPercentage, int numberOfSegments) {
         return Settings.builder()
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME

diff --git a/...che-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCachePlugin.java b/...che-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCachePlugin.java
@@ -12,7 +12,6 @@
 import org.opensearch.common.cache.ICache;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.plugins.CachePlugin;
 import org.opensearch.plugins.Plugin;
 
@@ -62,9 +61,7 @@ public List<Setting<?>> getSettings() {
                 TieredSpilloverCacheSettings.TIERED_SPILLOVER_DISK_STORE_NAME.getConcreteSettingForNamespace(cacheType.getSettingPrefix())
             );
             settingList.add(TOOK_TIME_POLICY_CONCRETE_SETTINGS_MAP.get(cacheType));
-            if (FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings)) {
-                settingList.add(DISK_CACHE_ENABLED_SETTING_MAP.get(cacheType));
-            }
+            settingList.add(DISK_CACHE_ENABLED_SETTING_MAP.get(cacheType));
             settingList.add(
                 TieredSpilloverCacheSettings.TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(cacheType.getSettingPrefix())
             );

diff --git a/...ommon/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCachePluginTests.java b/...ommon/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCachePluginTests.java
@@ -10,7 +10,6 @@
 
 import org.opensearch.common.cache.ICache;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.test.OpenSearchTestCase;
 
 import java.util.Map;
@@ -24,10 +23,8 @@ public void testGetCacheFactoryMap() {
         assertEquals(TieredSpilloverCachePlugin.TIERED_CACHE_SPILLOVER_PLUGIN_NAME, tieredSpilloverCachePlugin.getName());
     }
 
-    public void testGetSettingsWithFeatureFlagOn() {
-        TieredSpilloverCachePlugin tieredSpilloverCachePlugin = new TieredSpilloverCachePlugin(
-            Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE_SETTING.getKey(), true).build()
-        );
+    public void testGetSettings() {
+        TieredSpilloverCachePlugin tieredSpilloverCachePlugin = new TieredSpilloverCachePlugin(Settings.builder().build());
         assertFalse(tieredSpilloverCachePlugin.getSettings().isEmpty());
     }
 }
diff --git a/...ache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java b/...ache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java
@@ -29,7 +29,6 @@
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.env.NodeEnvironment;
 import org.opensearch.test.OpenSearchTestCase;
 import org.junit.Before;
@@ -179,7 +178,6 @@ public void testComputeIfAbsentWithFactoryBasedCacheCreation() throws Exception
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
             .put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 1)
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .build();
         String storagePath = getStoragePath(settings);
         ICache<String, String> tieredSpilloverICache = new TieredSpilloverCache.TieredSpilloverCacheFactory().create(
@@ -279,7 +277,6 @@ public void testComputeIfAbsentWithSegmentedCache() throws Exception {
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .build();
         String storagePath = getStoragePath(settings);
         ICache<String, String> tieredSpilloverICache = new TieredSpilloverCache.TieredSpilloverCacheFactory().create(
@@ -402,7 +399,6 @@ public void testWithFactoryCreationWithOnHeapCacheNotPresent() {
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .build();
 
         IllegalArgumentException ex = assertThrows(
@@ -487,7 +483,6 @@ public void testComputeIfAbsentWithEvictionsFromOnHeapCache() throws Exception {
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(
                 TieredSpilloverCacheSettings.TIERED_SPILLOVER_ONHEAP_STORE_SIZE.getConcreteSettingForNamespace(
                     CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()
@@ -1270,7 +1265,6 @@ public void testConcurrencyForEvictionFlowFromOnHeapToDiskTier() throws Exceptio
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(
                 TieredSpilloverCacheSettings.TIERED_SPILLOVER_ONHEAP_STORE_SIZE.getConcreteSettingForNamespace(
                     CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()
@@ -1956,7 +1950,6 @@ public void testWithInvalidSegmentNumber() throws Exception {
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
             .put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 1)
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 3)
             .build();
         String storagePath = getStoragePath(settings);
@@ -2022,7 +2015,6 @@ public void testWithVeryLowDiskCacheSize() throws Exception {
                 ).getKey(),
                 1L
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 2)
             .build();
         String storagePath = getStoragePath(settings);
@@ -2081,7 +2073,6 @@ public void testTieredCacheDefaultSegmentCount() {
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .build();
         String storagePath = getStoragePath(settings);
 
@@ -2215,7 +2206,6 @@ public void testSegmentSizesWhenUsingFactory() {
                 ).getKey(),
                 heapSizeFromImplSetting + "b"
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(
                 TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(),
                 numSegments
@@ -2262,7 +2252,6 @@ public void testSegmentSizesWhenNotUsingFactory() {
                 CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                 TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             // The size setting from the OpenSearchOnHeapCache implementation should not be honored
             .put(
                 OpenSearchOnHeapCacheSettings.MAXIMUM_SIZE_IN_BYTES.getConcreteSettingForNamespace(
@@ -2462,7 +2451,6 @@ private TieredSpilloverCache<String, String> intializeTieredSpilloverCache(
                         CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                         TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
                     )
-                    .put(FeatureFlags.PLUGGABLE_CACHE, "true")
                     .put(settings)
                     .build()
             )
@@ -2514,7 +2502,6 @@ private CacheConfig<String, String> getCacheConfig(
                         CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                         TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
                     )
-                    .put(FeatureFlags.PLUGGABLE_CACHE, "true")
                     .put(settings)
                     .build()
             )

diff --git a/...s/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java b/...s/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java
@@ -26,7 +26,6 @@
 import org.opensearch.common.cache.settings.CacheSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.env.NodeEnvironment;
 import org.opensearch.index.cache.request.RequestCacheStats;
 import org.opensearch.index.query.QueryBuilders;
@@ -71,11 +70,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(EhcacheCachePlugin.class);
     }
 
-    @Override
-    protected Settings featureFlagSettings() {
-        return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.PLUGGABLE_CACHE, "true").build();
-    }
-
     private Settings defaultSettings(long sizeInBytes, TimeValue expirationTime) {
         if (expirationTime == null) {
             expirationTime = TimeValue.MAX_VALUE;

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
@@ -26,7 +26,6 @@
 import org.opensearch.common.metrics.CounterMetric;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.util.io.IOUtils;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
@@ -1221,7 +1220,6 @@ private EhcacheDiskCache<String, String> setupMaxSizeTest(long maxSizeFromSettin
         MockRemovalListener<String, String> listener = new MockRemovalListener<>();
         try (NodeEnvironment env = newNodeEnvironment(Settings.builder().build())) {
             Settings settings = Settings.builder()
-                .put(FeatureFlags.PLUGGABLE_CACHE, true)
                 .put(
                     CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                     EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME

diff --git a/...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java b/...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java
@@ -8,8 +8,6 @@
 
 package org.opensearch.indices;
 
-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.cache.clear.ClearIndicesCacheRequest;
@@ -24,7 +22,6 @@
 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.common.xcontent.XContentFactory;
 import org.opensearch.common.xcontent.XContentHelper;
 import org.opensearch.core.xcontent.MediaTypeRegistry;
@@ -39,8 +36,6 @@
 import org.opensearch.transport.client.Client;
 
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -55,11 +50,6 @@ public CacheStatsAPIIndicesRequestCacheIT(Settings settings) {
         super(settings);
     }
 
-    @ParametersFactory
-    public static Collection<Object[]> parameters() {
-        return Arrays.<Object[]>asList(new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "true").build() });
-    }
-
     /**
      * Test aggregating by indices, indices+shards, shards, or no levels, and check the resulting stats
      * are as we expect.

diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java
@@ -56,7 +56,6 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.time.DateFormatter;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.index.Index;
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.env.NodeEnvironment;
@@ -110,9 +109,7 @@ public IndicesRequestCacheIT(Settings settings) {
     public static Collection<Object[]> parameters() {
         return Arrays.asList(
             new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
-            new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() },
-            new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "true").build() },
-            new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "false").build() }
+            new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
         );
     }
 

diff --git a/server/src/main/java/org/opensearch/common/cache/service/CacheService.java b/server/src/main/java/org/opensearch/common/cache/service/CacheService.java
@@ -18,7 +18,6 @@
 import org.opensearch.common.cache.store.config.CacheConfig;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.util.FeatureFlags;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -47,10 +46,9 @@ public CacheService(Map<String, ICache.Factory> cacheStoreTypeFactories, Setting
 
     public <K, V> ICache<K, V> createCache(CacheConfig<K, V> config, CacheType cacheType) {
         String storeName = getStoreNameFromSetting(cacheType, settings);
-        if (!pluggableCachingEnabled(cacheType, settings)) {
-            // Condition 1: In case feature flag is off, we default to onHeap.
-            // Condition 2: In case storeName is not explicitly mentioned, we assume user is looking to use older
-            // settings, so we again fallback to onHeap to maintain backward compatibility.
+        if (!storeNamePresent(cacheType, settings)) {
+            // In case storeName is not explicitly mentioned, we assume user is looking to use older
+            // settings, so we fallback to onHeap to maintain backward compatibility.
             // It is guaranteed that we will have this store name registered, so
             // should be safe.
             storeName = OpenSearchOnHeapCache.OpenSearchOnHeapCacheFactory.NAME;
@@ -73,11 +71,11 @@ public NodeCacheStats stats(CommonStatsFlags flags) {
     }
 
     /**
-     * Check if pluggable caching is on, and if a store type is present for this cache type.
+     * Check if a store type is present for this cache type.
      */
-    public static boolean pluggableCachingEnabled(CacheType cacheType, Settings settings) {
+    public static boolean storeNamePresent(CacheType cacheType, Settings settings) {
         String storeName = getStoreNameFromSetting(cacheType, settings);
-        return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
+        return storeName != null && !storeName.isBlank();
     }
 
     private static String getStoreNameFromSetting(CacheType cacheType, Settings settings) {

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
@@ -29,7 +29,6 @@
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
-import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.core.common.unit.ByteSizeValue;
 
 import java.util.List;
@@ -182,7 +181,7 @@ public static class OpenSearchOnHeapCacheFactory implements Factory {
         public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType, Map<String, Factory> cacheFactories) {
             Map<String, Setting<?>> settingList = OpenSearchOnHeapCacheSettings.getSettingListForCacheType(cacheType);
             Settings settings = config.getSettings();
-            boolean statsTrackingEnabled = statsTrackingEnabled(config.getSettings(), config.getStatsTrackingEnabled());
+            boolean statsTrackingEnabled = config.getStatsTrackingEnabled();
             ICacheBuilder<K, V> builder = new Builder<K, V>().setDimensionNames(config.getDimensionNames())
                 .setStatsTrackingEnabled(statsTrackingEnabled)
                 .setExpireAfterAccess(((TimeValue) settingList.get(EXPIRE_AFTER_ACCESS_KEY).get(settings)))
@@ -197,7 +196,7 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType,
                 /*
                 Use the cache config value if present.
                 This can be passed down from the TieredSpilloverCache when creating individual segments,
-                but is not passed in from the IRC if pluggable caching is on.
+                but is not passed in from the IRC if a store name setting is present.
                  */
                 builder.setMaximumWeightInBytes(config.getMaxSizeInBytes());
             } else {
@@ -209,7 +208,7 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType,
                 builder.setNumberOfSegments(-1); // By default it will use 256 segments.
             }
 
-            if (!CacheService.pluggableCachingEnabled(cacheType, settings)) {
+            if (!CacheService.storeNamePresent(cacheType, settings)) {
                 // For backward compatibility as the user intent is to use older settings.
                 builder.setMaximumWeightInBytes(config.getMaxSizeInBytes());
                 builder.setExpireAfterAccess(config.getExpireAfterAccess());
@@ -223,11 +222,6 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType,
         public String getCacheName() {
             return NAME;
         }
-
-        private boolean statsTrackingEnabled(Settings settings, boolean statsTrackingEnabledConfig) {
-            // Don't track stats when pluggable caching is off, or when explicitly set to false in the CacheConfig
-            return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && statsTrackingEnabledConfig;
-        }
     }
 
     /**