From 16e60ff331ca276a7b94a1fc1c94f64a7ca1707f Mon Sep 17 00:00:00 2001 From: "chentianjie.ctj" Date: Sun, 7 Apr 2024 17:39:06 +0800 Subject: [PATCH 1/3] Unshare object to avoid LRU/LFU being messed up. Signed-off-by: chentianjie.ctj --- src/db.c | 4 ++++ src/object.c | 10 ++++++++-- src/server.h | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/db.c b/src/db.c index a78c8bad2b..2e6d85cf4e 100644 --- a/src/db.c +++ b/src/db.c @@ -122,6 +122,10 @@ robj *lookupKey(serverDb *db, robj *key, int flags) { server.current_client->cmd->proc != touchCommand) flags |= LOOKUP_NOTOUCH; if (!hasActiveChildProcess() && !(flags & LOOKUP_NOTOUCH)) { + if (!canUseSharedObject() && val->refcount == OBJ_SHARED_REFCOUNT) { + val = dupStringObject(val); + kvstoreDictSetVal(db->keys, getKeySlot(key->ptr), de, val); + } if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) { updateLFU(val); } else { diff --git a/src/object.c b/src/object.c index 1a335edd6d..8141c48911 100644 --- a/src/object.c +++ b/src/object.c @@ -616,6 +616,10 @@ void trimStringObjectIfNeeded(robj *o, int trim_small_values) { } } +int canUseSharedObject(void) { + return server.maxmemory == 0 || !(server.maxmemory_policy & MAXMEMORY_FLAG_NO_SHARED_INTEGERS); +} + /* Try to encode a string object in order to save space */ robj *tryObjectEncodingEx(robj *o, int try_trim) { long value; @@ -647,8 +651,10 @@ robj *tryObjectEncodingEx(robj *o, int try_trim) { * Note that we avoid using shared integers when maxmemory is used * because every object needs to have a private LRU field for the LRU * algorithm to work well. */ - if ((server.maxmemory == 0 || !(server.maxmemory_policy & MAXMEMORY_FLAG_NO_SHARED_INTEGERS)) && value >= 0 && - value < OBJ_SHARED_INTEGERS) { + if (canUseSharedObject() && + value >= 0 && + value < OBJ_SHARED_INTEGERS) + { decrRefCount(o); return shared.integers[value]; } else { diff --git a/src/server.h b/src/server.h index 70beb54f43..75336b44fa 100644 --- a/src/server.h +++ b/src/server.h @@ -2852,6 +2852,7 @@ int collateStringObjects(const robj *a, const robj *b); int equalStringObjects(robj *a, robj *b); unsigned long long estimateObjectIdleTime(robj *o); void trimStringObjectIfNeeded(robj *o, int trim_small_values); +int canUseSharedObject(void); #define sdsEncodedObject(objptr) (objptr->encoding == OBJ_ENCODING_RAW || objptr->encoding == OBJ_ENCODING_EMBSTR) /* Synchronous I/O with timeout */ From a590629cd0d9c561a9ff8381146df160f3ed4c3a Mon Sep 17 00:00:00 2001 From: Chen Tianjie Date: Mon, 6 May 2024 09:35:15 +0800 Subject: [PATCH 2/3] Move canUseSharedObject to server.h. Signed-off-by: Chen Tianjie --- src/object.c | 4 ---- src/server.h | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/object.c b/src/object.c index 8141c48911..ea3538b9ee 100644 --- a/src/object.c +++ b/src/object.c @@ -616,10 +616,6 @@ void trimStringObjectIfNeeded(robj *o, int trim_small_values) { } } -int canUseSharedObject(void) { - return server.maxmemory == 0 || !(server.maxmemory_policy & MAXMEMORY_FLAG_NO_SHARED_INTEGERS); -} - /* Try to encode a string object in order to save space */ robj *tryObjectEncodingEx(robj *o, int try_trim) { long value; diff --git a/src/server.h b/src/server.h index 75336b44fa..193f43ec70 100644 --- a/src/server.h +++ b/src/server.h @@ -2852,7 +2852,9 @@ int collateStringObjects(const robj *a, const robj *b); int equalStringObjects(robj *a, robj *b); unsigned long long estimateObjectIdleTime(robj *o); void trimStringObjectIfNeeded(robj *o, int trim_small_values); -int canUseSharedObject(void); +static inline int canUseSharedObject(void) { + return server.maxmemory == 0 || !(server.maxmemory_policy & MAXMEMORY_FLAG_NO_SHARED_INTEGERS); +} #define sdsEncodedObject(objptr) (objptr->encoding == OBJ_ENCODING_RAW || objptr->encoding == OBJ_ENCODING_EMBSTR) /* Synchronous I/O with timeout */ From 43584f98bd93308a072fbeccedc38f679d253f32 Mon Sep 17 00:00:00 2001 From: Chen Tianjie Date: Mon, 6 May 2024 11:22:18 +0800 Subject: [PATCH 3/3] Add test for unsharing object. Signed-off-by: Chen Tianjie --- tests/unit/maxmemory.tcl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unit/maxmemory.tcl b/tests/unit/maxmemory.tcl index 92e68ac1ed..ee1232796d 100644 --- a/tests/unit/maxmemory.tcl +++ b/tests/unit/maxmemory.tcl @@ -168,6 +168,21 @@ start_server {tags {"maxmemory external:skip"}} { r config set maxmemory 0 } + test "Shared integers are unshared with maxmemory and LRU policy" { + r set a 1 + r set b 1 + assert_refcount_morethan a 1 + assert_refcount_morethan b 1 + r config set maxmemory 1073741824 + r config set maxmemory-policy allkeys-lru + r get a + assert_refcount 1 a + r config set maxmemory-policy volatile-lru + r get b + assert_refcount 1 b + r config set maxmemory 0 + } + foreach policy { allkeys-random allkeys-lru allkeys-lfu volatile-lru volatile-lfu volatile-random volatile-ttl } {