[Tiered Caching] Remove PLUGGABLE_CACHE feature flag (#17344)

* Remove PLUGGABLE_CACHE feature flag

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* changelog

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* move changelog entry

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* fix IT init failure

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <peter.alfonsi@gmail.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>

Author: peteralfonsi

CHANGELOG-3.0.md

@@ -73,6 +73,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Remove package org.opensearch.action.support.master ([#4856](https://github.com/opensearch-project/OpenSearch/issues/4856))
 - Remove transport-nio plugin ([#16887](https://github.com/opensearch-project/OpenSearch/issues/16887))
 - Remove deprecated 'gateway' settings used to defer cluster recovery ([#3117](https://github.com/opensearch-project/OpenSearch/issues/3117))
+- Remove FeatureFlags.PLUGGABLE_CACHE as the feature is no longer experimental ([#17344](https://github.com/opensearch-project/OpenSearch/pull/17344))
 
 ### Fixed
 - Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))

distribution/src/config/opensearch.yml

@@ -122,10 +122,6 @@ ${path.logs}
 #
 #opensearch.experimental.optimization.datetime_formatter_caching.enabled: false
 #
-# Gates the functionality of enabling Opensearch to use pluggable caches with respective store names via setting.
-#
-#opensearch.experimental.feature.pluggable.caching.enabled: false
-#
 # Gates the functionality of star tree index, which improves the performance of search aggregations.
 #
 #opensearch.experimental.feature.composite_index.star_tree.enabled: true

modules/cache-common/src/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

modules/cache-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;
 
@@ -64,9 +63,7 @@ public List<Setting<?>> getSettings() {
             );
             settingList.add(TOOK_TIME_POLICY_CONCRETE_SETTINGS_MAP.get(cacheType));
             settingList.add(TOOK_TIME_DISK_TIER_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())
             );

modules/cache-common/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());
     }
 }

modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java

@@ -30,7 +30,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;
@@ -183,7 +182,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(
@@ -283,7 +281,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(
@@ -406,7 +403,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(
@@ -491,7 +487,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()
@@ -1276,7 +1271,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()
@@ -2160,7 +2154,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);
@@ -2226,7 +2219,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);
@@ -2285,7 +2277,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);
 
@@ -2419,7 +2410,6 @@ public void testSegmentSizesWhenUsingFactory() {
                 ).getKey(),
                 heapSizeFromImplSetting + "b"
             )
-            .put(FeatureFlags.PLUGGABLE_CACHE, "true")
             .put(
                 TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(),
                 numSegments
@@ -2466,7 +2456,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(
@@ -2697,7 +2686,6 @@ private TieredSpilloverCache<String, String> initializeTieredSpilloverCache(
                         CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
                         TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
                     )
-                    .put(FeatureFlags.PLUGGABLE_CACHE, "true")
                     .put(settings)
                     .build()
             )
@@ -2750,7 +2738,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()
             )

plugins/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;

plugins/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

server/src/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;
@@ -34,13 +31,10 @@
 import org.opensearch.index.cache.request.RequestCacheStats;
 import org.opensearch.index.query.QueryBuilders;
 import org.opensearch.test.OpenSearchIntegTestCase;
-import org.opensearch.test.ParameterizedStaticSettingsOpenSearchIntegTestCase;
 import org.opensearch.test.hamcrest.OpenSearchAssertions;
 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;
@@ -50,16 +44,7 @@
 
 // Use a single data node to simplify logic about cache stats across different shards.
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 1)
-public class CacheStatsAPIIndicesRequestCacheIT extends ParameterizedStaticSettingsOpenSearchIntegTestCase {
-    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() });
-    }
-
+public class CacheStatsAPIIndicesRequestCacheIT extends OpenSearchIntegTestCase {
     /**
      * Test aggregating by indices, indices+shards, shards, or no levels, and check the resulting stats
      * are as we expect.

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() }
         );
     }
 

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) {