From 5519dab77d8763400ca4d70f1dfcfdd3cd2f4a63 Mon Sep 17 00:00:00 2001 From: hwware Date: Wed, 20 Nov 2024 21:09:34 +0000 Subject: [PATCH] Update eviction logic Signed-off-by: hwware --- src/evict.c | 32 +++++++++++++++++++------------- src/module.c | 6 +++--- src/server.h | 2 +- tests/unit/maxmemory.tcl | 20 +------------------- 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/evict.c b/src/evict.c index 590236195d..05a49c7ff0 100644 --- 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 index 1e98b36f30..ae484a5542 100644 --- 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 index 3c85912445..d42206dd9c 100644 --- 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 index d71b23ce47..f4b3ece038 100644 --- 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} } -} \ No newline at end of file +}