Skip to content

Commit

Permalink
Addressed Ankit's comments
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed Jan 8, 2025
1 parent 7f9c5f5 commit ff2f1e5
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,14 @@ void updateStatsOnPut(String destinationTierValue, ICacheKey<K> key, V value) {
statsHolder.incrementSizeInBytes(dimensionValues, weigher.applyAsLong(key, value));
}

@Override
public long getMaximumWeight() {
if (caches.get(diskCache).isEnabled()) {
return onHeapCache.getMaximumWeight() + diskCache.getMaximumWeight();
}
return onHeapCache.getMaximumWeight();
}

/**
* A class which receives removal events from the heap tier.
*/
Expand Down Expand Up @@ -692,6 +700,11 @@ long diskCacheCount() {
return diskCacheEntries;
}

@Override
public long getMaximumWeight() {
return tieredSpilloverCacheSegments[0].getMaximumWeight() * numberOfSegments;
}

/**
* ConcatenatedIterables which combines cache iterables and supports remove() functionality as well if underlying
* iterator supports it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ public class TieredSpilloverCacheSettings {

/**
* Setting which defines the disk cache size to be used within tiered cache.
* Similarly, this setting overrides the size setting from the disk tier implementation.
* This setting overrides the size setting from the disk tier implementation.
* For example, if EhcacheDiskCache is the disk tier in the request cache, and
* indices.requests.cache.ehcache_disk.max_size_in_bytes is set, that value will be ignored in favor of this setting.
*/
public static final Setting.AffixSetting<Long> TIERED_SPILLOVER_DISK_STORE_SIZE = Setting.suffixKeySetting(
TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME + ".disk.store.size",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public void close() {

}

long getMaxSize() {
@Override
public long getMaximumWeight() {
return maxSize;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2297,7 +2297,12 @@ private void checkSegmentSizes(TieredSpilloverCache<String, String> cache, long

MockDiskCache<String, String> segmentDiskCache = (MockDiskCache<String, String>) cache.tieredSpilloverCacheSegments[0]
.getDiskCache();
assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segmentDiskCache.getMaxSize());
assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segmentDiskCache.getMaximumWeight());
assertEquals(
expectedHeapSize / cache.getNumberOfSegments() + expectedDiskSize / cache.getNumberOfSegments(),
cache.tieredSpilloverCacheSegments[0].getMaximumWeight()
);
assertEquals(expectedHeapSize + expectedDiskSize, cache.getMaximumWeight());
}

private List<String> getMockDimensions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,8 @@ private V deserializeValue(ByteArrayWrapper binary) {
}

// Pkg-private for testing.
long getMaxWeightInBytes() {
@Override
public long getMaximumWeight() {
return maxWeightInBytes;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1209,10 +1209,10 @@ public void testWithCacheConfigSizeSettings() throws Exception {
long maxSizeFromConfig = between(MINIMUM_MAX_SIZE_IN_BYTES + 3000, MINIMUM_MAX_SIZE_IN_BYTES + 4000);

EhcacheDiskCache<String, String> cache = setupMaxSizeTest(maxSizeFromSetting, maxSizeFromConfig, false);
assertEquals(maxSizeFromSetting, cache.getMaxWeightInBytes());
assertEquals(maxSizeFromSetting, cache.getMaximumWeight());

cache = setupMaxSizeTest(maxSizeFromSetting, maxSizeFromConfig, true);
assertEquals(maxSizeFromConfig, cache.getMaxWeightInBytes());
assertEquals(maxSizeFromConfig, cache.getMaximumWeight());
}

// Modified from OpenSearchOnHeapCacheTests. Can't reuse, as we can't add a dependency on the server.test module.
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/opensearch/common/cache/ICache.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ default ImmutableCacheStatsHolder stats() {
// Return stats aggregated by the provided levels. If levels is null or an empty array, return total stats only.
ImmutableCacheStatsHolder stats(String[] levels);

long getMaximumWeight();

/**
* Factory to create objects.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ public CacheService(Map<String, ICache.Factory> cacheStoreTypeFactories, Setting
}

public <K, V> ICache<K, V> createCache(CacheConfig<K, V> config, CacheType cacheType) {
Setting<String> cacheSettingForCacheType = CacheSettings.CACHE_TYPE_STORE_NAME.getConcreteSettingForNamespace(
cacheType.getSettingPrefix()
);
String storeName = cacheSettingForCacheType.get(settings);
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
Expand Down Expand Up @@ -79,10 +76,14 @@ public NodeCacheStats stats(CommonStatsFlags flags) {
* Check if pluggable caching is on, and if a store type is present for this cache type.
*/
public static boolean pluggableCachingEnabled(CacheType cacheType, Settings settings) {
String storeName = getStoreNameFromSetting(cacheType, settings);
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
}

private static String getStoreNameFromSetting(CacheType cacheType, Settings settings) {
Setting<String> cacheSettingForCacheType = CacheSettings.CACHE_TYPE_STORE_NAME.getConcreteSettingForNamespace(
cacheType.getSettingPrefix()
);
String storeName = cacheSettingForCacheType.get(settings);
return FeatureFlags.PLUGGABLE_CACHE_SETTING.get(settings) && storeName != null && !storeName.isBlank();
return cacheSettingForCacheType.get(settings);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public OpenSearchOnHeapCache(Builder<K, V> builder) {
this.weigher = builder.getWeigher();
}

// public for testing
@Override
public long getMaximumWeight() {
return this.maximumWeight;
}
Expand Down

0 comments on commit ff2f1e5

Please sign in to comment.