Skip to content

Commit 1717cc7

Browse files
sgup432sgup432
authored
[Tiered Caching] Fixing flaky tiered cache test (#12650)
* Fixing flaky tiered cache test Signed-off-by: Sagar Upadhyaya <[email protected]> * Removing unnecessary comment Signed-off-by: Sagar Upadhyaya <[email protected]> * Removing unused variable Signed-off-by: Sagar Upadhyaya <[email protected]> --------- Signed-off-by: Sagar Upadhyaya <[email protected]>
1 parent f37b6ac commit 1717cc7

File tree

3 files changed

+22
-23
lines changed

3 files changed

+22
-23
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public void onRemoval(RemovalNotification<K, V> notification) {
6565
try (ReleasableLock ignore = writeLock.acquire()) {
6666
diskCache.put(notification.getKey(), notification.getValue());
6767
}
68-
removalListener.onRemoval(notification);
6968
}
7069
})
7170
.setKeyType(builder.cacheConfig.getKeyType())

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
import org.opensearch.common.cache.CacheType;
1212
import org.opensearch.common.cache.ICache;
1313
import org.opensearch.common.cache.LoadAwareCacheLoader;
14+
import org.opensearch.common.cache.RemovalListener;
15+
import org.opensearch.common.cache.RemovalNotification;
16+
import org.opensearch.common.cache.RemovalReason;
1417
import org.opensearch.common.cache.store.builders.ICacheBuilder;
1518
import org.opensearch.common.cache.store.config.CacheConfig;
1619

@@ -23,9 +26,12 @@ public class MockDiskCache<K, V> implements ICache<K, V> {
2326
int maxSize;
2427
long delay;
2528

26-
public MockDiskCache(int maxSize, long delay) {
29+
private final RemovalListener<K, V> removalListener;
30+
31+
public MockDiskCache(int maxSize, long delay, RemovalListener<K, V> removalListener) {
2732
this.maxSize = maxSize;
2833
this.delay = delay;
34+
this.removalListener = removalListener;
2935
this.cache = new ConcurrentHashMap<K, V>();
3036
}
3137

@@ -38,7 +44,7 @@ public V get(K key) {
3844
@Override
3945
public void put(K key, V value) {
4046
if (this.cache.size() >= maxSize) { // For simplification
41-
return;
47+
this.removalListener.onRemoval(new RemovalNotification<>(key, value, RemovalReason.EVICTED));
4248
}
4349
try {
4450
Thread.sleep(delay);
@@ -101,7 +107,10 @@ public MockDiskCacheFactory(long delay, int maxSize) {
101107

102108
@Override
103109
public <K, V> ICache<K, V> create(CacheConfig<K, V> config, CacheType cacheType, Map<String, Factory> cacheFactories) {
104-
return new Builder<K, V>().setMaxSize(maxSize).setDeliberateDelay(delay).build();
110+
return new Builder<K, V>().setMaxSize(maxSize)
111+
.setDeliberateDelay(delay)
112+
.setRemovalListener(config.getRemovalListener())
113+
.build();
105114
}
106115

107116
@Override
@@ -117,7 +126,7 @@ public static class Builder<K, V> extends ICacheBuilder<K, V> {
117126

118127
@Override
119128
public ICache<K, V> build() {
120-
return new MockDiskCache<K, V>(this.maxSize, this.delay);
129+
return new MockDiskCache<K, V>(this.maxSize, this.delay, this.getRemovalListener());
121130
}
122131

123132
public Builder<K, V> setMaxSize(int maxSize) {

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

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public void testComputeIfAbsentWithoutAnyOnHeapCacheEviction() throws Exception
4242

4343
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
4444
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
45-
onHeapCacheSize,
45+
keyValueSize,
4646
randomIntBetween(1, 4),
4747
removalListener,
4848
Settings.builder()
@@ -142,10 +142,6 @@ public void testComputeIfAbsentWithFactoryBasedCacheCreation() throws Exception
142142
LoadAwareCacheLoader<String, String> tieredCacheLoader = getLoadAwareCacheLoader();
143143
tieredSpilloverCache.computeIfAbsent(key, tieredCacheLoader);
144144
}
145-
long actualDiskCacheSize = tieredSpilloverCache.getDiskCache().count();
146-
assertEquals(actualDiskCacheSize, removalListener.evictionsMetric.count()); // Evictions from onHeap equal to
147-
// disk cache size.
148-
149145
tieredSpilloverCache.getOnHeapCache().keys().forEach(onHeapKeys::add);
150146
tieredSpilloverCache.getDiskCache().keys().forEach(diskTierKeys::add);
151147

@@ -290,9 +286,6 @@ public void testComputeIfAbsentWithEvictionsFromOnHeapCache() throws Exception {
290286
LoadAwareCacheLoader<String, String> tieredCacheLoader = getLoadAwareCacheLoader();
291287
tieredSpilloverCache.computeIfAbsent(key, tieredCacheLoader);
292288
}
293-
long actualDiskCacheSize = tieredSpilloverCache.getDiskCache().count();
294-
assertEquals(actualDiskCacheSize, removalListener.evictionsMetric.count()); // Evictions from onHeap equal to
295-
// disk cache size.
296289

297290
tieredSpilloverCache.getOnHeapCache().keys().forEach(onHeapKeys::add);
298291
tieredSpilloverCache.getDiskCache().keys().forEach(diskTierKeys::add);
@@ -328,15 +321,15 @@ public void testComputeIfAbsentWithEvictionsFromOnHeapCache() throws Exception {
328321
}
329322
}
330323

331-
public void testComputeIfAbsentWithEvictionsFromBothTier() throws Exception {
324+
public void testComputeIfAbsentWithEvictionsFromTieredCache() throws Exception {
332325
int onHeapCacheSize = randomIntBetween(10, 30);
333326
int diskCacheSize = randomIntBetween(onHeapCacheSize + 1, 100);
334327
int totalSize = onHeapCacheSize + diskCacheSize;
335328
int keyValueSize = 50;
336329

337330
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
338331
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
339-
onHeapCacheSize,
332+
keyValueSize,
340333
diskCacheSize,
341334
removalListener,
342335
Settings.builder()
@@ -349,13 +342,13 @@ public void testComputeIfAbsentWithEvictionsFromBothTier() throws Exception {
349342
.build(),
350343
0
351344
);
352-
353345
int numOfItems = randomIntBetween(totalSize + 1, totalSize * 3);
354346
for (int iter = 0; iter < numOfItems; iter++) {
355347
LoadAwareCacheLoader<String, String> tieredCacheLoader = getLoadAwareCacheLoader();
356348
tieredSpilloverCache.computeIfAbsent(UUID.randomUUID().toString(), tieredCacheLoader);
357349
}
358-
assertTrue(removalListener.evictionsMetric.count() > 0);
350+
int evictions = numOfItems - (totalSize);
351+
assertEquals(evictions, removalListener.evictionsMetric.count());
359352
}
360353

361354
public void testGetAndCount() throws Exception {
@@ -366,7 +359,7 @@ public void testGetAndCount() throws Exception {
366359

367360
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
368361
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
369-
onHeapCacheSize,
362+
keyValueSize,
370363
diskCacheSize,
371364
removalListener,
372365
Settings.builder()
@@ -418,7 +411,7 @@ public void testPut() {
418411

419412
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
420413
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
421-
onHeapCacheSize,
414+
keyValueSize,
422415
diskCacheSize,
423416
removalListener,
424417
Settings.builder()
@@ -519,7 +512,7 @@ public void testInvalidate() {
519512

520513
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
521514
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
522-
onHeapCacheSize,
515+
keyValueSize,
523516
diskCacheSize,
524517
removalListener,
525518
Settings.builder()
@@ -744,7 +737,7 @@ public String load(String key) {
744737
assertEquals(1, numberOfTimesKeyLoaded); // It should be loaded only once.
745738
}
746739

747-
public void testConcurrencyForEvictionFlow() throws Exception {
740+
public void testConcurrencyForEvictionFlowFromOnHeapToDiskTier() throws Exception {
748741
int diskCacheSize = randomIntBetween(450, 800);
749742

750743
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
@@ -828,7 +821,6 @@ public String load(String key) {
828821
countDownLatch.await();
829822
assertNotNull(actualValue.get());
830823
countDownLatch1.await();
831-
assertEquals(1, removalListener.evictionsMetric.count());
832824
assertEquals(1, tieredSpilloverCache.getOnHeapCache().count());
833825
assertEquals(1, onDiskCache.count());
834826
assertNotNull(onDiskCache.get(keyToBeEvicted));
@@ -883,7 +875,6 @@ private TieredSpilloverCache<String, String> intializeTieredSpilloverCache(
883875
.build()
884876
)
885877
.build();
886-
887878
ICache.Factory mockDiskCacheFactory = new MockDiskCache.MockDiskCacheFactory(diskDeliberateDelay, diskCacheSize);
888879

889880
return new TieredSpilloverCache.Builder<String, String>().setCacheType(CacheType.INDICES_REQUEST_CACHE)

0 commit comments

Comments
 (0)