Update eviction logic

valkey-io · Nov 20, 2024 · 83bc5d7 · 83bc5d7
1 parent cd1a07f
commit 83bc5d7
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 36 deletions.
diff --git a/src/evict.c b/src/evict.c
@@ -389,7 +389,7 @@ size_t freeMemoryGetNotCountedMemory(void) {
  *              limit.
  *              (Populated both for C_ERR and C_OK)
  */
-int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level) {
+int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level, unsigned long long checked_maxmemory) {
     size_t mem_reported, mem_used, mem_tofree;
 
     /* Check if we are over the memory usage limit. If we are not, no need
@@ -398,12 +398,12 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev
     if (total) *total = mem_reported;
 
     /* We may return ASAP if there is no need to compute the level. */
-    if (!server.maxmemory_soft) {
+    if (!checked_maxmemory) {
         if (level) *level = 0;
         return C_OK;
     }
 
-    if (mem_reported <= server.maxmemory_soft && !level) return C_OK;
+    if (mem_reported <= checked_maxmemory && !level) return C_OK;
 
     /* Remove the size of replicas output buffers and AOF buffer from the
      * count of used memory. */
@@ -416,10 +416,12 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev
         *level = (float)mem_used / (float)server.maxmemory;
     }
 
-    if (mem_reported <= server.maxmemory_soft) return C_OK;
+    if (mem_reported <= checked_maxmemory) return C_OK;
 
     /* Check if we are still over the memory limit. */
-    if (mem_used <= server.maxmemory_soft) return C_OK;
+    /* if checked_maxmemory is equal to maxmemory and mem_used > checked_maxmemory then OOM */
+    /* if checked_maxmemory is equal to maxmemory_soft and mem_used > checked_maxmemory then there is no OOM but eviction happens */
+    if (mem_used <= checked_maxmemory) return C_OK;
 
     /* Compute how much memory we need to free. */
     mem_tofree = mem_used - server.maxmemory_soft;
@@ -544,7 +546,7 @@ int performEvictions(void) {
     int replicas = listLength(server.replicas);
     int result = EVICT_FAIL;
 
-    if (getMaxmemoryState(&mem_reported, NULL, &mem_tofree, NULL) == C_OK) {
+    if (getMaxmemoryState(&mem_reported, NULL, &mem_tofree, NULL, server.maxmemory) == C_OK) {
         result = EVICT_OK;
         goto update_metrics;
     }
@@ -710,7 +712,7 @@ int performEvictions(void) {
                  * across the dbAsyncDelete() call, while the thread can
                  * release the memory all the time. */
                 if (server.lazyfree_lazy_eviction) {
-                    if (getMaxmemoryState(NULL, NULL, NULL, NULL) == C_OK) {
+                    if (getMaxmemoryState(NULL, NULL, NULL, NULL, server.maxmemory) == C_OK) {
                         break;
                     }
                 }
@@ -725,19 +727,23 @@ int performEvictions(void) {
                 }
             }
         } else {
-            goto cant_free; /* nothing to free... */
+            if (server.maxmemory_soft_scale) {
+                break;
+            } else {
+                goto cant_free; /* nothing to free... */
+            }
         }
     }
-    /* at this point, the memory is OK, or we have reached the time limit */
     if (server.maxmemory_soft_scale) {
         size_t mem_used = zmalloc_used_memory();
         size_t overhead = freeMemoryGetNotCountedMemory();
         mem_used = (mem_used > overhead) ? mem_used - overhead : 0;
-        if (mem_used > server.maxmemory) {
-            result = EVICT_FAIL;
-            goto cant_free;
+        if (mem_used < server.maxmemory) {
+            result = EVICT_OK;
+            goto update_metrics;
         }
     }
+    /* at this point, the memory is OK, or we have reached the time limit */
     result = (isEvictionProcRunning) ? EVICT_RUNNING : EVICT_OK;
 
 cant_free:
@@ -748,7 +754,7 @@ int performEvictions(void) {
         mstime_t lazyfree_latency;
         latencyStartMonitor(lazyfree_latency);
         while (bioPendingJobsOfType(BIO_LAZY_FREE) && elapsedUs(evictionTimer) < eviction_time_limit_us) {
-            if (getMaxmemoryState(NULL, NULL, NULL, NULL) == C_OK) {
+            if (getMaxmemoryState(NULL, NULL, NULL, NULL, server.maxmemory) == C_OK) {
                 result = EVICT_OK;
                 break;
             }

diff --git a/src/module.c b/src/module.c
@@ -3913,7 +3913,7 @@ int VM_GetContextFlags(ValkeyModuleCtx *ctx) {
 
     /* OOM flag. */
     float level;
-    int retval = getMaxmemoryState(NULL, NULL, NULL, &level);
+    int retval = getMaxmemoryState(NULL, NULL, NULL, &level, server.maxmemory);
     if (retval == C_ERR) flags |= VALKEYMODULE_CTX_FLAGS_OOM;
     if (level > 0.75) flags |= VALKEYMODULE_CTX_FLAGS_OOM_WARNING;
 
@@ -6310,7 +6310,7 @@ ValkeyModuleCallReply *VM_Call(ValkeyModuleCtx *ctx, const char *cmdname, const
                 /* On background thread we can not count on server.pre_command_oom_state.
                  * Because it is only set on the main thread, in such case we will check
                  * the actual memory usage. */
-                oom_state = (getMaxmemoryState(NULL, NULL, NULL, NULL) == C_ERR);
+                oom_state = (getMaxmemoryState(NULL, NULL, NULL, NULL, server.maxmemory) == C_ERR);
             } else {
                 oom_state = server.pre_command_oom_state;
             }
@@ -10855,7 +10855,7 @@ size_t VM_MallocSizeDict(ValkeyModuleDict *dict) {
  */
 float VM_GetUsedMemoryRatio(void) {
     float level;
-    getMaxmemoryState(NULL, NULL, NULL, &level);
+    getMaxmemoryState(NULL, NULL, NULL, &level, server.maxmemory);
     return level;
 }
 

diff --git a/src/server.h b/src/server.h
@@ -3282,7 +3282,7 @@ int zslLexValueGteMin(sds value, zlexrangespec *spec);
 int zslLexValueLteMax(sds value, zlexrangespec *spec);
 
 /* Core functions */
-int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level);
+int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level, unsigned long long checked_maxmemory);
 size_t freeMemoryGetNotCountedMemory(void);
 int overMaxmemoryAfterAlloc(size_t moremem);
 uint64_t getCommandFlags(client *c);

diff --git a/tests/unit/maxmemory.tcl b/tests/unit/maxmemory.tcl
@@ -146,24 +146,6 @@ start_server {tags {"maxmemory" "external:skip"}} {
 
 start_server {tags {"maxmemory external:skip"}} {
 
-    test "maxmemory-soft-scale" {
-        r flushdb
-	r config set maxmemory 10mb
-        set  current_dbsize 0
-
-	foreach scale_value {30 10 0} {
-	    r config set maxmemory-soft-scale $scale_value
-	    # fill 20mb using 200 keys of 100kb
-            catch { for {set j 0} {$j < 200} {incr j} {
-                r setrange $j 100000 x
-            }} e
-	    assert_match {*OOM*} $e
-	    assert_lessthan $current_dbsize [r dbsize]
-	    set current_dbsize [r dbsize]
-	}
-	r flushdb
-    }
-
     test "Without maxmemory small integers are shared" {
         r config set maxmemory 0
         r set a 1
@@ -647,4 +629,4 @@ start_server {tags {"maxmemory" "external:skip"}} {
 
         assert_equal [r dbsize] {0}
     }
-}
+}