From 195ea528bcfd95422d1e90ec63ad29d7c2b282e6 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 16 Jan 2024 14:27:01 +0100 Subject: [PATCH] freq=1 --- .../shared/SharedBlobCacheService.java | 27 +++++++++------ .../shared/SharedBlobCacheServiceTests.java | 34 ++++++++++++------- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java index ad0fa2e5b9d5f..a2ea3cf906aa6 100644 --- a/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java +++ b/x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java @@ -255,6 +255,13 @@ public void validate(ByteSizeValue value, Map, Object> settings, bool Setting.Property.NodeScope ); + // used in tests + void computeDecay() { + if (cache instanceof LFUCache lfuCache) { + lfuCache.computeDecay(); + } + } + private interface Cache extends Releasable { CacheEntry get(K cacheKey, long fileLength, int region); @@ -1101,6 +1108,7 @@ class LFUCacheEntry extends CacheEntry { LFUCacheEntry(CacheFileRegion chunk, long lastAccessed) { super(chunk); this.lastAccessed = lastAccessed; + this.freq = 1; } void touch() { @@ -1201,7 +1209,7 @@ private LFUCacheEntry initChunk(LFUCacheEntry entry) { throwAlreadyClosed("no free region found (contender)"); } // new item - assert entry.freq == 0; + assert entry.freq == 1; assert entry.prev == null; assert entry.next == null; final SharedBytes.IO freeSlot = freeRegions.poll(); @@ -1369,22 +1377,19 @@ private int maybeEvict() { } /** - * This method tries to evict the oldest least used {@link LFUCacheEntry}. Only entries with the lowest possible frequency are - * considered for eviction. + * This method tries to evict the least used {@link LFUCacheEntry}. Only entries with the lowest possible frequency are considered + * for eviction. * * @return true if an entry was evicted, false otherwise. */ public boolean maybeEvictLeastUsed() { synchronized (SharedBlobCacheService.this) { - long now = relativeTimeInMillis(); for (LFUCacheEntry entry = freqs[0]; entry != null; entry = entry.next) { - if (now - entry.lastAccessed >= 2 * minTimeDelta) { - boolean evicted = entry.chunk.tryEvict(); - if (evicted && entry.chunk.io != null) { - unlink(entry); - keyMapping.remove(entry.chunk.regionKey, entry); - return true; - } + boolean evicted = entry.chunk.tryEvict(); + if (evicted && entry.chunk.io != null) { + unlink(entry); + keyMapping.remove(entry.chunk.regionKey, entry); + return true; } } } diff --git a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java index e578dc14ac186..3b75134d17233 100644 --- a/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java +++ b/x-pack/plugin/blob-cache/src/test/java/org/elasticsearch/blobcache/shared/SharedBlobCacheServiceTests.java @@ -261,29 +261,35 @@ public void testDecay() throws IOException { final var region1 = cacheService.get(cacheKey2, size(250), 1); assertEquals(3, cacheService.freeRegionCount()); - assertEquals(0, cacheService.getFreq(region0)); - assertEquals(0, cacheService.getFreq(region1)); + assertEquals(1, cacheService.getFreq(region0)); + assertEquals(1, cacheService.getFreq(region1)); taskQueue.advanceTime(); taskQueue.runAllRunnableTasks(); final var region0Again = cacheService.get(cacheKey1, size(250), 0); assertSame(region0Again, region0); - assertEquals(1, cacheService.getFreq(region0)); - assertEquals(0, cacheService.getFreq(region1)); + assertEquals(2, cacheService.getFreq(region0)); + assertEquals(1, cacheService.getFreq(region1)); taskQueue.advanceTime(); taskQueue.runAllRunnableTasks(); cacheService.get(cacheKey1, size(250), 0); - assertEquals(2, cacheService.getFreq(region0)); + assertEquals(3, cacheService.getFreq(region0)); cacheService.get(cacheKey1, size(250), 0); - assertEquals(2, cacheService.getFreq(region0)); + assertEquals(3, cacheService.getFreq(region0)); // advance 2 ticks (decay only starts after 2 ticks) taskQueue.advanceTime(); taskQueue.runAllRunnableTasks(); taskQueue.advanceTime(); taskQueue.runAllRunnableTasks(); + assertEquals(2, cacheService.getFreq(region0)); + assertEquals(0, cacheService.getFreq(region1)); + + // advance another tick + taskQueue.advanceTime(); + taskQueue.runAllRunnableTasks(); assertEquals(1, cacheService.getFreq(region0)); assertEquals(0, cacheService.getFreq(region1)); @@ -701,7 +707,7 @@ public void testCacheSizeChanges() throws IOException { } public void testMaybeEvictLeastUsed() throws Exception { - final int numRegions = randomIntBetween(1, 500); + final int numRegions = 3;randomIntBetween(1, 500); final long regionSize = size(1L); Settings settings = Settings.builder() .put(NODE_NAME_SETTING.getKey(), "node") @@ -739,6 +745,8 @@ public void testMaybeEvictLeastUsed() throws Exception { taskQueue.getThreadPool().generic(), ActionListener.noop() ); + assertThat(cacheService.getFreq(entry), equalTo(1)); + relativeTimeInMillis.incrementAndGet(); cacheKeys.add(cacheKey); } @@ -759,17 +767,18 @@ public void testMaybeEvictLeastUsed() throws Exception { cacheKeys.forEach(key -> { if (unusedCacheKeys.contains(key) == false) { var entry = cacheService.get(key, regionSize, 0); - assertThat(cacheService.getFreq(entry), equalTo(1)); + assertThat(cacheService.getFreq(entry), equalTo(2)); } }); assertThat("All regions are used", cacheService.freeRegionCount(), equalTo(0)); assertThat("Cache entries are not old enough to be evicted", cacheService.maybeEvictLeastUsed(), is(false)); - // simulate elapsed time - relativeTimeInMillis.addAndGet(minInternalMillis); - for (int i = 1; i <= unusedCacheKeys.size(); i++) { + // need to advance time and compute decay to decrease frequencies in cache and have an evictable entry + relativeTimeInMillis.addAndGet(minInternalMillis); + cacheService.computeDecay(); + assertThat("Cache entry is old enough to be evicted", cacheService.maybeEvictLeastUsed(), is(true)); assertThat(cacheService.freeRegionCount(), equalTo(i)); } @@ -882,9 +891,10 @@ public void execute(Runnable command) { ); } { - // simulate elapsed time + // simulate elapsed time and compute decay var minInternalMillis = SharedBlobCacheService.SHARED_CACHE_MIN_TIME_DELTA_SETTING.getDefault(Settings.EMPTY).millis(); relativeTimeInMillis.addAndGet(minInternalMillis * 2); + cacheService.computeDecay(); // fetch one more region should evict an old cache entry final var cacheKey = generateCacheKey();