Skip to content

Commit f8ebfd4

Browse files
author
Peter Alfonsi
committed
Remove PLUGGABLE_CACHE feature flag
Signed-off-by: Peter Alfonsi <[email protected]>
1 parent d0a65d3 commit f8ebfd4

File tree

17 files changed

+59
-210
lines changed

17 files changed

+59
-210
lines changed

‎modules/cache-common/src/internalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheBaseIT.java

-2
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@
1212
import org.opensearch.common.cache.settings.CacheSettings;
1313
import org.opensearch.common.cache.store.OpenSearchOnHeapCache;
1414
import org.opensearch.common.settings.Settings;
15-
import org.opensearch.common.util.FeatureFlags;
1615
import org.opensearch.test.OpenSearchIntegTestCase;
1716

1817
public class TieredSpilloverCacheBaseIT extends OpenSearchIntegTestCase {
1918

2019
public Settings defaultSettings(String onHeapCacheSizeInBytesOrPercentage, int numberOfSegments) {
2120
return Settings.builder()
22-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
2321
.put(
2422
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
2523
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME

‎modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCachePlugin.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.opensearch.common.cache.ICache;
1313
import org.opensearch.common.settings.Setting;
1414
import org.opensearch.common.settings.Settings;
15-
import org.opensearch.common.util.FeatureFlags;
1615
import org.opensearch.plugins.CachePlugin;
1716
import org.opensearch.plugins.Plugin;
1817

@@ -62,9 +61,7 @@ public List<Setting<?>> getSettings() {
6261
TieredSpilloverCacheSettings.TIERED_SPILLOVER_DISK_STORE_NAME.getConcreteSettingForNamespace(cacheType.getSettingPrefix())
6362
);
6463
settingList.add(TOOK_TIME_POLICY_CONCRETE_SETTINGS_MAP.get(cacheType));
65-
if (FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings)) {
66-
settingList.add(DISK_CACHE_ENABLED_SETTING_MAP.get(cacheType));
67-
}
64+
settingList.add(DISK_CACHE_ENABLED_SETTING_MAP.get(cacheType));
6865
settingList.add(
6966
TieredSpilloverCacheSettings.TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(cacheType.getSettingPrefix())
7067
);

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

+2-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.opensearch.common.cache.ICache;
1212
import org.opensearch.common.settings.Settings;
13-
import org.opensearch.common.util.FeatureFlags;
1413
import org.opensearch.test.OpenSearchTestCase;
1514

1615
import java.util.Map;
@@ -24,10 +23,8 @@ public void testGetCacheFactoryMap() {
2423
assertEquals(TieredSpilloverCachePlugin.TIERED_CACHE_SPILLOVER_PLUGIN_NAME, tieredSpilloverCachePlugin.getName());
2524
}
2625

27-
public void testGetSettingsWithFeatureFlagOn() {
28-
TieredSpilloverCachePlugin tieredSpilloverCachePlugin = new TieredSpilloverCachePlugin(
29-
Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE_SETTING.getKey(), true).build()
30-
);
26+
public void testGetSettings() {
27+
TieredSpilloverCachePlugin tieredSpilloverCachePlugin = new TieredSpilloverCachePlugin(Settings.builder().build());
3128
assertFalse(tieredSpilloverCachePlugin.getSettings().isEmpty());
3229
}
3330
}

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

-13
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.opensearch.common.settings.Setting;
3030
import org.opensearch.common.settings.Settings;
3131
import org.opensearch.common.unit.TimeValue;
32-
import org.opensearch.common.util.FeatureFlags;
3332
import org.opensearch.env.NodeEnvironment;
3433
import org.opensearch.test.OpenSearchTestCase;
3534
import org.junit.Before;
@@ -179,7 +178,6 @@ public void testComputeIfAbsentWithFactoryBasedCacheCreation() throws Exception
179178
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
180179
)
181180
.put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 1)
182-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
183181
.build();
184182
String storagePath = getStoragePath(settings);
185183
ICache<String, String> tieredSpilloverICache = new TieredSpilloverCache.TieredSpilloverCacheFactory().create(
@@ -279,7 +277,6 @@ public void testComputeIfAbsentWithSegmentedCache() throws Exception {
279277
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
280278
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
281279
)
282-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
283280
.build();
284281
String storagePath = getStoragePath(settings);
285282
ICache<String, String> tieredSpilloverICache = new TieredSpilloverCache.TieredSpilloverCacheFactory().create(
@@ -402,7 +399,6 @@ public void testWithFactoryCreationWithOnHeapCacheNotPresent() {
402399
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
403400
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
404401
)
405-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
406402
.build();
407403

408404
IllegalArgumentException ex = assertThrows(
@@ -487,7 +483,6 @@ public void testComputeIfAbsentWithEvictionsFromOnHeapCache() throws Exception {
487483
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
488484
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
489485
)
490-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
491486
.put(
492487
TieredSpilloverCacheSettings.TIERED_SPILLOVER_ONHEAP_STORE_SIZE.getConcreteSettingForNamespace(
493488
CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()
@@ -1270,7 +1265,6 @@ public void testConcurrencyForEvictionFlowFromOnHeapToDiskTier() throws Exceptio
12701265
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
12711266
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
12721267
)
1273-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
12741268
.put(
12751269
TieredSpilloverCacheSettings.TIERED_SPILLOVER_ONHEAP_STORE_SIZE.getConcreteSettingForNamespace(
12761270
CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()
@@ -1956,7 +1950,6 @@ public void testWithInvalidSegmentNumber() throws Exception {
19561950
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
19571951
)
19581952
.put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 1)
1959-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
19601953
.put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 3)
19611954
.build();
19621955
String storagePath = getStoragePath(settings);
@@ -2022,7 +2015,6 @@ public void testWithVeryLowDiskCacheSize() throws Exception {
20222015
).getKey(),
20232016
1L
20242017
)
2025-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
20262018
.put(TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(), 2)
20272019
.build();
20282020
String storagePath = getStoragePath(settings);
@@ -2081,7 +2073,6 @@ public void testTieredCacheDefaultSegmentCount() {
20812073
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
20822074
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
20832075
)
2084-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
20852076
.build();
20862077
String storagePath = getStoragePath(settings);
20872078

@@ -2215,7 +2206,6 @@ public void testSegmentSizesWhenUsingFactory() {
22152206
).getKey(),
22162207
heapSizeFromImplSetting + "b"
22172208
)
2218-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
22192209
.put(
22202210
TIERED_SPILLOVER_SEGMENTS.getConcreteSettingForNamespace(CacheType.INDICES_REQUEST_CACHE.getSettingPrefix()).getKey(),
22212211
numSegments
@@ -2262,7 +2252,6 @@ public void testSegmentSizesWhenNotUsingFactory() {
22622252
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
22632253
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
22642254
)
2265-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
22662255
// The size setting from the OpenSearchOnHeapCache implementation should not be honored
22672256
.put(
22682257
OpenSearchOnHeapCacheSettings.MAXIMUM_SIZE_IN_BYTES.getConcreteSettingForNamespace(
@@ -2462,7 +2451,6 @@ private TieredSpilloverCache<String, String> intializeTieredSpilloverCache(
24622451
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
24632452
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
24642453
)
2465-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
24662454
.put(settings)
24672455
.build()
24682456
)
@@ -2514,7 +2502,6 @@ private CacheConfig<String, String> getCacheConfig(
25142502
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
25152503
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME
25162504
)
2517-
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
25182505
.put(settings)
25192506
.build()
25202507
)

‎plugins/cache-ehcache/src/internalClusterTest/java/org/opensearch/cache/EhcacheDiskCacheIT.java

-6
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.opensearch.common.cache.settings.CacheSettings;
2727
import org.opensearch.common.settings.Settings;
2828
import org.opensearch.common.unit.TimeValue;
29-
import org.opensearch.common.util.FeatureFlags;
3029
import org.opensearch.env.NodeEnvironment;
3130
import org.opensearch.index.cache.request.RequestCacheStats;
3231
import org.opensearch.index.query.QueryBuilders;
@@ -71,11 +70,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
7170
return Arrays.asList(EhcacheCachePlugin.class);
7271
}
7372

74-
@Override
75-
protected Settings featureFlagSettings() {
76-
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.PLUGGABLE_CACHE, "true").build();
77-
}
78-
7973
private Settings defaultSettings(long sizeInBytes, TimeValue expirationTime) {
8074
if (expirationTime == null) {
8175
expirationTime = TimeValue.MAX_VALUE;

‎plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java

-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.opensearch.common.metrics.CounterMetric;
2727
import org.opensearch.common.settings.Settings;
2828
import org.opensearch.common.unit.TimeValue;
29-
import org.opensearch.common.util.FeatureFlags;
3029
import org.opensearch.common.util.io.IOUtils;
3130
import org.opensearch.core.common.bytes.BytesArray;
3231
import org.opensearch.core.common.bytes.BytesReference;
@@ -1221,7 +1220,6 @@ private EhcacheDiskCache<String, String> setupMaxSizeTest(long maxSizeFromSettin
12211220
MockRemovalListener<String, String> listener = new MockRemovalListener<>();
12221221
try (NodeEnvironment env = newNodeEnvironment(Settings.builder().build())) {
12231222
Settings settings = Settings.builder()
1224-
.put(FeatureFlags.PLUGGABLE_CACHE, true)
12251223
.put(
12261224
CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(),
12271225
EhcacheDiskCache.EhcacheDiskCacheFactory.EHCACHE_DISK_CACHE_NAME

‎server/src/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java

-10
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88

99
package org.opensearch.indices;
1010

11-
import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
12-
1311
import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
1412
import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
1513
import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
@@ -24,7 +22,6 @@
2422
import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder;
2523
import org.opensearch.common.settings.Settings;
2624
import org.opensearch.common.unit.TimeValue;
27-
import org.opensearch.common.util.FeatureFlags;
2825
import org.opensearch.common.xcontent.XContentFactory;
2926
import org.opensearch.common.xcontent.XContentHelper;
3027
import org.opensearch.core.xcontent.MediaTypeRegistry;
@@ -39,8 +36,6 @@
3936
import org.opensearch.transport.client.Client;
4037

4138
import java.io.IOException;
42-
import java.util.Arrays;
43-
import java.util.Collection;
4439
import java.util.HashMap;
4540
import java.util.List;
4641
import java.util.Map;
@@ -55,11 +50,6 @@ public CacheStatsAPIIndicesRequestCacheIT(Settings settings) {
5550
super(settings);
5651
}
5752

58-
@ParametersFactory
59-
public static Collection<Object[]> parameters() {
60-
return Arrays.<Object[]>asList(new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "true").build() });
61-
}
62-
6353
/**
6454
* Test aggregating by indices, indices+shards, shards, or no levels, and check the resulting stats
6555
* are as we expect.

‎server/src/internalClusterTest/java/org/opensearch/indices/IndicesRequestCacheIT.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import org.opensearch.common.settings.Settings;
5757
import org.opensearch.common.time.DateFormatter;
5858
import org.opensearch.common.unit.TimeValue;
59-
import org.opensearch.common.util.FeatureFlags;
6059
import org.opensearch.core.index.Index;
6160
import org.opensearch.core.index.shard.ShardId;
6261
import org.opensearch.env.NodeEnvironment;
@@ -110,9 +109,7 @@ public IndicesRequestCacheIT(Settings settings) {
110109
public static Collection<Object[]> parameters() {
111110
return Arrays.asList(
112111
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
113-
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() },
114-
new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "true").build() },
115-
new Object[] { Settings.builder().put(FeatureFlags.PLUGGABLE_CACHE, "false").build() }
112+
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
116113
);
117114
}
118115

‎server/src/main/java/org/opensearch/common/cache/service/CacheService.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.opensearch.common.cache.store.config.CacheConfig;
1919
import org.opensearch.common.settings.Setting;
2020
import org.opensearch.common.settings.Settings;
21-
import org.opensearch.common.util.FeatureFlags;
2221

2322
import java.util.HashMap;
2423
import java.util.Map;
@@ -47,10 +46,9 @@ public CacheService(Map<String, ICache.Factory> cacheStoreTypeFactories, Setting
4746

4847
public <K, V> ICache<K, V> createCache(CacheConfig<K, V> config, CacheType cacheType) {
4948
String storeName = getStoreNameFromSetting(cacheType, settings);
50-
if (!pluggableCachingEnabled(cacheType, settings)) {
51-
// Condition 1: In case feature flag is off, we default to onHeap.
52-
// Condition 2: In case storeName is not explicitly mentioned, we assume user is looking to use older
53-
// settings, so we again fallback to onHeap to maintain backward compatibility.
49+
if (!storeNamePresent(cacheType, settings)) {
50+
// In case storeName is not explicitly mentioned, we assume user is looking to use older
51+
// settings, so we fallback to onHeap to maintain backward compatibility.
5452
// It is guaranteed that we will have this store name registered, so
5553
// should be safe.
5654
storeName = OpenSearchOnHeapCache.OpenSearchOnHeapCacheFactory.NAME;
@@ -73,11 +71,11 @@ public NodeCacheStats stats(CommonStatsFlags flags) {
7371
}
7472

7573
/**
76-
* Check if pluggable caching is on, and if a store type is present for this cache type.
74+
* Check if a store type is present for this cache type.
7775
*/
78-
public static boolean pluggableCachingEnabled(CacheType cacheType, Settings settings) {
76+
public static boolean storeNamePresent(CacheType cacheType, Settings settings) {
7977
String storeName = getStoreNameFromSetting(cacheType, settings);
80-
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
78+
return storeName != null && !storeName.isBlank();
8179
}
8280

8381
private static String getStoreNameFromSetting(CacheType cacheType, Settings settings) {

‎server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java

+3-9
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.opensearch.common.settings.Setting;
3030
import org.opensearch.common.settings.Settings;
3131
import org.opensearch.common.unit.TimeValue;
32-
import org.opensearch.common.util.FeatureFlags;
3332
import org.opensearch.core.common.unit.ByteSizeValue;
3433

3534
import java.util.List;
@@ -182,7 +181,7 @@ public static class OpenSearchOnHeapCacheFactory implements Factory {
182181
public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType, Map<String, Factory> cacheFactories) {
183182
Map<String, Setting<?>> settingList = OpenSearchOnHeapCacheSettings.getSettingListForCacheType(cacheType);
184183
Settings settings = config.getSettings();
185-
boolean statsTrackingEnabled = statsTrackingEnabled(config.getSettings(), config.getStatsTrackingEnabled());
184+
boolean statsTrackingEnabled = config.getStatsTrackingEnabled();
186185
ICacheBuilder<K, V> builder = new Builder<K, V>().setDimensionNames(config.getDimensionNames())
187186
.setStatsTrackingEnabled(statsTrackingEnabled)
188187
.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,
197196
/*
198197
Use the cache config value if present.
199198
This can be passed down from the TieredSpilloverCache when creating individual segments,
200-
but is not passed in from the IRC if pluggable caching is on.
199+
but is not passed in from the IRC if a store name setting is present.
201200
*/
202201
builder.setMaximumWeightInBytes(config.getMaxSizeInBytes());
203202
} else {
@@ -209,7 +208,7 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType,
209208
builder.setNumberOfSegments(-1); // By default it will use 256 segments.
210209
}
211210

212-
if (!CacheService.pluggableCachingEnabled(cacheType, settings)) {
211+
if (!CacheService.storeNamePresent(cacheType, settings)) {
213212
// For backward compatibility as the user intent is to use older settings.
214213
builder.setMaximumWeightInBytes(config.getMaxSizeInBytes());
215214
builder.setExpireAfterAccess(config.getExpireAfterAccess());
@@ -223,11 +222,6 @@ public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType,
223222
public String getCacheName() {
224223
return NAME;
225224
}
226-
227-
private boolean statsTrackingEnabled(Settings settings, boolean statsTrackingEnabledConfig) {
228-
// Don't track stats when pluggable caching is off, or when explicitly set to false in the CacheConfig
229-
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && statsTrackingEnabledConfig;
230-
}
231225
}
232226

233227
/**

0 commit comments

Comments
 (0)