freq=1

x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java

@@ -255,6 +255,13 @@ public void validate(ByteSizeValue value, Map<Setting<?>, Object> settings, bool
         Setting.Property.NodeScope
     );
 
+    // used in tests
+    void computeDecay() {
+        if (cache instanceof LFUCache lfuCache) {
+            lfuCache.computeDecay();
+        }
+    }
+
     private interface Cache<K, T> extends Releasable {
         CacheEntry<T> get(K cacheKey, long fileLength, int region);
 
@@ -1101,6 +1108,7 @@ class LFUCacheEntry extends CacheEntry<CacheFileRegion> {
             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;
                     }
                 }
             }

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();