Skip to content

Commit ff2f1e5

Browse files
author
Peter Alfonsi
committed
Addressed Ankit's comments
Signed-off-by: Peter Alfonsi <[email protected]>
1 parent 7f9c5f5 commit ff2f1e5

File tree

9 files changed

+38
-13
lines changed

9 files changed

+38
-13
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,14 @@ void updateStatsOnPut(String destinationTierValue, ICacheKey<K> key, V value) {
526526
statsHolder.incrementSizeInBytes(dimensionValues, weigher.applyAsLong(key, value));
527527
}
528528

529+
@Override
530+
public long getMaximumWeight() {
531+
if (caches.get(diskCache).isEnabled()) {
532+
return onHeapCache.getMaximumWeight() + diskCache.getMaximumWeight();
533+
}
534+
return onHeapCache.getMaximumWeight();
535+
}
536+
529537
/**
530538
* A class which receives removal events from the heap tier.
531539
*/
@@ -692,6 +700,11 @@ long diskCacheCount() {
692700
return diskCacheEntries;
693701
}
694702

703+
@Override
704+
public long getMaximumWeight() {
705+
return tieredSpilloverCacheSegments[0].getMaximumWeight() * numberOfSegments;
706+
}
707+
695708
/**
696709
* ConcatenatedIterables which combines cache iterables and supports remove() functionality as well if underlying
697710
* iterator supports it.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@ public class TieredSpilloverCacheSettings {
9999

100100
/**
101101
* Setting which defines the disk cache size to be used within tiered cache.
102-
* Similarly, this setting overrides the size setting from the disk tier implementation.
102+
* This setting overrides the size setting from the disk tier implementation.
103+
* For example, if EhcacheDiskCache is the disk tier in the request cache, and
104+
* indices.requests.cache.ehcache_disk.max_size_in_bytes is set, that value will be ignored in favor of this setting.
103105
*/
104106
public static final Setting.AffixSetting<Long> TIERED_SPILLOVER_DISK_STORE_SIZE = Setting.suffixKeySetting(
105107
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME + ".disk.store.size",

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ public void close() {
128128

129129
}
130130

131-
long getMaxSize() {
131+
@Override
132+
public long getMaximumWeight() {
132133
return maxSize;
133134
}
134135

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2297,7 +2297,12 @@ private void checkSegmentSizes(TieredSpilloverCache<String, String> cache, long
22972297

22982298
MockDiskCache<String, String> segmentDiskCache = (MockDiskCache<String, String>) cache.tieredSpilloverCacheSegments[0]
22992299
.getDiskCache();
2300-
assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segmentDiskCache.getMaxSize());
2300+
assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segmentDiskCache.getMaximumWeight());
2301+
assertEquals(
2302+
expectedHeapSize / cache.getNumberOfSegments() + expectedDiskSize / cache.getNumberOfSegments(),
2303+
cache.tieredSpilloverCacheSegments[0].getMaximumWeight()
2304+
);
2305+
assertEquals(expectedHeapSize + expectedDiskSize, cache.getMaximumWeight());
23012306
}
23022307

23032308
private List<String> getMockDimensions() {

plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,8 @@ private V deserializeValue(ByteArrayWrapper binary) {
681681
}
682682

683683
// Pkg-private for testing.
684-
long getMaxWeightInBytes() {
684+
@Override
685+
public long getMaximumWeight() {
685686
return maxWeightInBytes;
686687
}
687688

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,10 +1209,10 @@ public void testWithCacheConfigSizeSettings() throws Exception {
12091209
long maxSizeFromConfig = between(MINIMUM_MAX_SIZE_IN_BYTES + 3000, MINIMUM_MAX_SIZE_IN_BYTES + 4000);
12101210

12111211
EhcacheDiskCache<String, String> cache = setupMaxSizeTest(maxSizeFromSetting, maxSizeFromConfig, false);
1212-
assertEquals(maxSizeFromSetting, cache.getMaxWeightInBytes());
1212+
assertEquals(maxSizeFromSetting, cache.getMaximumWeight());
12131213

12141214
cache = setupMaxSizeTest(maxSizeFromSetting, maxSizeFromConfig, true);
1215-
assertEquals(maxSizeFromConfig, cache.getMaxWeightInBytes());
1215+
assertEquals(maxSizeFromConfig, cache.getMaximumWeight());
12161216
}
12171217

12181218
// Modified from OpenSearchOnHeapCacheTests. Can't reuse, as we can't add a dependency on the server.test module.

server/src/main/java/org/opensearch/common/cache/ICache.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ default ImmutableCacheStatsHolder stats() {
5353
// Return stats aggregated by the provided levels. If levels is null or an empty array, return total stats only.
5454
ImmutableCacheStatsHolder stats(String[] levels);
5555

56+
long getMaximumWeight();
57+
5658
/**
5759
* Factory to create objects.
5860
*/

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,7 @@ public CacheService(Map<String, ICache.Factory> cacheStoreTypeFactories, Setting
4646
}
4747

4848
public <K, V> ICache<K, V> createCache(CacheConfig<K, V> config, CacheType cacheType) {
49-
Setting<String> cacheSettingForCacheType = CacheSettings.CACHE_TYPE_STORE_NAME.getConcreteSettingForNamespace(
50-
cacheType.getSettingPrefix()
51-
);
52-
String storeName = cacheSettingForCacheType.get(settings);
49+
String storeName = getStoreNameFromSetting(cacheType, settings);
5350
if (!pluggableCachingEnabled(cacheType, settings)) {
5451
// Condition 1: In case feature flag is off, we default to onHeap.
5552
// Condition 2: In case storeName is not explicitly mentioned, we assume user is looking to use older
@@ -79,10 +76,14 @@ public NodeCacheStats stats(CommonStatsFlags flags) {
7976
* Check if pluggable caching is on, and if a store type is present for this cache type.
8077
*/
8178
public static boolean pluggableCachingEnabled(CacheType cacheType, Settings settings) {
79+
String storeName = getStoreNameFromSetting(cacheType, settings);
80+
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
81+
}
82+
83+
private static String getStoreNameFromSetting(CacheType cacheType, Settings settings) {
8284
Setting<String> cacheSettingForCacheType = CacheSettings.CACHE_TYPE_STORE_NAME.getConcreteSettingForNamespace(
8385
cacheType.getSettingPrefix()
8486
);
85-
String storeName = cacheSettingForCacheType.get(settings);
86-
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
87+
return cacheSettingForCacheType.get(settings);
8788
}
8889
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public OpenSearchOnHeapCache(Builder<K, V> builder) {
8181
this.weigher = builder.getWeigher();
8282
}
8383

84-
// public for testing
84+
@Override
8585
public long getMaximumWeight() {
8686
return this.maximumWeight;
8787
}

0 commit comments

Comments
 (0)