From f97efe0cac1653fbadf02679056cba3e5317aad2 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 9 Apr 2018 17:43:50 +0200 Subject: [PATCH] Modules: context flags now include OOM flag. Plus freeMemoryIfNeeded() refactoring to improve legibility. Please review this commit for sanity. --- src/evict.c | 68 ++++++++++++++++++++++++++++++++--------------- src/module.c | 8 ++++++ src/redismodule.h | 26 +++++++++--------- src/server.h | 1 + 4 files changed, 69 insertions(+), 34 deletions(-) diff --git a/src/evict.c b/src/evict.c index 063cf03d1..1853eae23 100644 --- a/src/evict.c +++ b/src/evict.c @@ -369,25 +369,22 @@ size_t freeMemoryGetNotCountedMemory(void) { return overhead; } -/* This function is periodically called to see if there is memory to free - * according to the current "maxmemory" settings. In case we are over the - * memory limit, the function will try to free some memory to return back - * under the limit. +/* Get the memory status from the point of view of the maxmemory directive: + * if the memory used is under the maxmemory setting then C_OK is returned + * and the 'total', 'lgoical', and 'tofree' values are not populated at all. + * Otherwise, if we are over the memory limit, the function returns + * C_ERR and populates (if not NULL) the arguments by reference with the + * following meaning: * - * The function returns C_OK if we are under the memory limit or if we - * were over the limit, but the attempt to free memory was successful. - * Otehrwise if we are over the memory limit, but not enough memory - * was freed to return back under the limit, the function returns C_ERR. */ -int freeMemoryIfNeeded(void) { - size_t mem_reported, mem_used, mem_tofree, mem_freed; - mstime_t latency, eviction_latency; - long long delta; - int slaves = listLength(server.slaves); - - /* When clients are paused the dataset should be static not just from the - * POV of clients not being able to write, but also from the POV of - * expires and evictions of keys not being performed. */ - if (clientsArePaused()) return C_OK; + * 'total' total amount of bytes used. + * + * 'logical' the amount of memory used minus the slaves/AOF buffers. + * + * 'tofree' the amount of memory that should be released + * in order to return back into the memory limits. + */ +int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree) { + size_t mem_reported, mem_used, mem_tofree; /* Check if we are over the memory usage limit. If we are not, no need * to subtract the slaves output buffers. We can just return ASAP. */ @@ -405,6 +402,35 @@ int freeMemoryIfNeeded(void) { /* Compute how much memory we need to free. */ mem_tofree = mem_used - server.maxmemory; + + if (*total) *total = mem_reported; + if (*logical) *logical = mem_used; + if (*tofree) *tofree = mem_tofree; + + return C_ERR; +} + +/* This function is periodically called to see if there is memory to free + * according to the current "maxmemory" settings. In case we are over the + * memory limit, the function will try to free some memory to return back + * under the limit. + * + * The function returns C_OK if we are under the memory limit or if we + * were over the limit, but the attempt to free memory was successful. + * Otehrwise if we are over the memory limit, but not enough memory + * was freed to return back under the limit, the function returns C_ERR. */ +int freeMemoryIfNeeded(void) { + size_t mem_reported, mem_tofree, mem_freed; + mstime_t latency, eviction_latency; + long long delta; + int slaves = listLength(server.slaves); + + /* When clients are paused the dataset should be static not just from the + * POV of clients not being able to write, but also from the POV of + * expires and evictions of keys not being performed. */ + if (clientsArePaused()) return C_OK; + if (getMaxmemoryState(&mem_reported,NULL,&mem_tofree) == C_OK) return C_OK; + mem_freed = 0; if (server.maxmemory_policy == MAXMEMORY_NO_EVICTION) @@ -538,10 +564,8 @@ int freeMemoryIfNeeded(void) { * across the dbAsyncDelete() call, while the thread can * release the memory all the time. */ if (server.lazyfree_lazy_eviction && !(keys_freed % 16)) { - overhead = freeMemoryGetNotCountedMemory(); - mem_used = zmalloc_used_memory(); - mem_used = (mem_used > overhead) ? mem_used-overhead : 0; - if (mem_used <= server.maxmemory) { + if (getMaxmemoryState(NULL,NULL,NULL) == C_OK) { + /* Let's satisfy our stop condition. */ mem_freed = mem_tofree; } } diff --git a/src/module.c b/src/module.c index c0c1401f4..62fada930 100644 --- a/src/module.c +++ b/src/module.c @@ -1327,6 +1327,9 @@ int RM_GetSelectedDb(RedisModuleCtx *ctx) { * * * REDISMODULE_CTX_FLAGS_EVICT: Maxmemory is set and has an eviction * policy that may delete keys + * + * * REDISMODULE_CTX_FLAGS_OOM: Redis is out of memory according to the + * maxmemory setting. */ int RM_GetContextFlags(RedisModuleCtx *ctx) { @@ -1365,6 +1368,11 @@ int RM_GetContextFlags(RedisModuleCtx *ctx) { flags |= REDISMODULE_CTX_FLAGS_READONLY; } + /* OOM flag. */ + if (getMaxmemoryState(NULL,NULL,NULL) == C_ERR) { + flags |= REDISMODULE_CTX_FLAGS_OOM; + } + return flags; } diff --git a/src/redismodule.h b/src/redismodule.h index 4611a77e4..517d13544 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -58,29 +58,31 @@ #define REDISMODULE_HASH_CFIELDS (1<<2) #define REDISMODULE_HASH_EXISTS (1<<3) -/* Context Flags: Info about the current context returned by RM_GetContextFlags */ +/* Context Flags: Info about the current context returned by + * RM_GetContextFlags(). */ /* The command is running in the context of a Lua script */ -#define REDISMODULE_CTX_FLAGS_LUA 0x0001 +#define REDISMODULE_CTX_FLAGS_LUA (1<<0) /* The command is running inside a Redis transaction */ -#define REDISMODULE_CTX_FLAGS_MULTI 0x0002 +#define REDISMODULE_CTX_FLAGS_MULTI (1<<1) /* The instance is a master */ -#define REDISMODULE_CTX_FLAGS_MASTER 0x0004 +#define REDISMODULE_CTX_FLAGS_MASTER (1<<2) /* The instance is a slave */ -#define REDISMODULE_CTX_FLAGS_SLAVE 0x0008 +#define REDISMODULE_CTX_FLAGS_SLAVE (1<<3) /* The instance is read-only (usually meaning it's a slave as well) */ -#define REDISMODULE_CTX_FLAGS_READONLY 0x0010 +#define REDISMODULE_CTX_FLAGS_READONLY (1<<4) /* The instance is running in cluster mode */ -#define REDISMODULE_CTX_FLAGS_CLUSTER 0x0020 +#define REDISMODULE_CTX_FLAGS_CLUSTER (1<<5) /* The instance has AOF enabled */ -#define REDISMODULE_CTX_FLAGS_AOF 0x0040 // +#define REDISMODULE_CTX_FLAGS_AOF (1<<6) /* The instance has RDB enabled */ -#define REDISMODULE_CTX_FLAGS_RDB 0x0080 // +#define REDISMODULE_CTX_FLAGS_RDB (1<<7) /* The instance has Maxmemory set */ -#define REDISMODULE_CTX_FLAGS_MAXMEMORY 0x0100 +#define REDISMODULE_CTX_FLAGS_MAXMEMORY (1<<8) /* Maxmemory is set and has an eviction policy that may delete keys */ -#define REDISMODULE_CTX_FLAGS_EVICT 0x0200 - +#define REDISMODULE_CTX_FLAGS_EVICT (1<<9) +/* Redis is out of memory according to the maxmemory flag. */ +#define REDISMODULE_CTX_FLAGS_OOM (1<<10) #define REDISMODULE_NOTIFY_GENERIC (1<<2) /* g */ #define REDISMODULE_NOTIFY_STRING (1<<3) /* $ */ diff --git a/src/server.h b/src/server.h index 09c99bdd4..523ce2620 100644 --- a/src/server.h +++ b/src/server.h @@ -1643,6 +1643,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); int freeMemoryIfNeeded(void); int processCommand(client *c); void setupSignalHandlers(void);