From 224cc376b1bf1d913d49d3b6af8451f0219e3c48 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 8 Jan 2025 16:28:53 -0800 Subject: [PATCH] Address Sagar's comment Signed-off-by: Peter Alfonsi --- .../common/tier/TieredSpilloverCache.java | 24 ++++++++++--------- .../cache/common/tier/MockDiskCache.java | 3 +-- .../tier/TieredSpilloverCacheTests.java | 10 +++----- .../cache/store/disk/EhcacheDiskCache.java | 3 +-- .../org/opensearch/common/cache/ICache.java | 2 -- .../cache/store/OpenSearchOnHeapCache.java | 4 ++-- 6 files changed, 20 insertions(+), 26 deletions(-) diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java index 8b8f76e181292..9879235812377 100644 --- a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java +++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java @@ -150,6 +150,9 @@ static class TieredSpilloverCacheSegment implements ICache { private final TieredSpilloverCacheStatsHolder statsHolder; + private final long onHeapCacheMaxWeight; + private final long diskCacheMaxWeight; + /** * This map is used to handle concurrent requests for same key in computeIfAbsent() to ensure we load the value * only once. @@ -218,6 +221,8 @@ static class TieredSpilloverCacheSegment implements ICache { cacheListMap.put(diskCache, new TierInfo(isDiskCacheEnabled, TIER_DIMENSION_VALUE_DISK)); this.caches = Collections.synchronizedMap(cacheListMap); this.policies = builder.policies; // Will never be null; builder initializes it to an empty list + this.onHeapCacheMaxWeight = onHeapCacheSizeInBytes; + this.diskCacheMaxWeight = diskCacheSizeInBytes; } // Package private for testing @@ -526,12 +531,14 @@ void updateStatsOnPut(String destinationTierValue, ICacheKey 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(); + // pkg-private for testing + long getOnHeapCacheMaxWeight() { + return onHeapCacheMaxWeight; + } + + // pkg-private for testing + long getDiskCacheMaxWeight() { + return diskCacheMaxWeight; } /** @@ -700,11 +707,6 @@ 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. diff --git a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/MockDiskCache.java b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/MockDiskCache.java index 25468b086b736..78302cede402f 100644 --- a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/MockDiskCache.java +++ b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/MockDiskCache.java @@ -128,8 +128,7 @@ public void close() { } - @Override - public long getMaximumWeight() { + long getMaximumWeight() { return maxSize; } diff --git a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java index cb23892977b9f..8dd8508d2b004 100644 --- a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java +++ b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java @@ -2293,16 +2293,12 @@ private void checkSegmentSizes(TieredSpilloverCache cache, long OpenSearchOnHeapCache segmentHeapCache = (OpenSearchOnHeapCache< String, String>) cache.tieredSpilloverCacheSegments[0].getOnHeapCache(); - assertEquals(expectedHeapSize / cache.getNumberOfSegments(), segmentHeapCache.getMaximumWeight()); + TieredSpilloverCache.TieredSpilloverCacheSegment segment = cache.tieredSpilloverCacheSegments[0]; + assertEquals(expectedHeapSize / cache.getNumberOfSegments(), segment.getOnHeapCacheMaxWeight()); MockDiskCache segmentDiskCache = (MockDiskCache) cache.tieredSpilloverCacheSegments[0] .getDiskCache(); - assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segmentDiskCache.getMaximumWeight()); - assertEquals( - expectedHeapSize / cache.getNumberOfSegments() + expectedDiskSize / cache.getNumberOfSegments(), - cache.tieredSpilloverCacheSegments[0].getMaximumWeight() - ); - assertEquals(expectedHeapSize + expectedDiskSize, cache.getMaximumWeight()); + assertEquals(expectedDiskSize / cache.getNumberOfSegments(), segment.getDiskCacheMaxWeight()); } private List getMockDimensions() { diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java index 0cc5fcaef8d14..33c27eb301ad1 100644 --- a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java +++ b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java @@ -681,8 +681,7 @@ private V deserializeValue(ByteArrayWrapper binary) { } // Pkg-private for testing. - @Override - public long getMaximumWeight() { + long getMaximumWeight() { return maxWeightInBytes; } diff --git a/server/src/main/java/org/opensearch/common/cache/ICache.java b/server/src/main/java/org/opensearch/common/cache/ICache.java index 7471a64f3fc5a..f5dd644db6d6b 100644 --- a/server/src/main/java/org/opensearch/common/cache/ICache.java +++ b/server/src/main/java/org/opensearch/common/cache/ICache.java @@ -53,8 +53,6 @@ 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. */ 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 index aec6c9100f128..e1039c5d9ee55 100644 --- a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java +++ b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java @@ -81,8 +81,8 @@ public OpenSearchOnHeapCache(Builder builder) { this.weigher = builder.getWeigher(); } - @Override - public long getMaximumWeight() { + // pkg-private for testing + long getMaximumWeight() { return this.maximumWeight; }