Unify dbSyncDelete and dbAsyncDelete (#9573)

Just a cleanup to make the code easier to maintain and reduce the risk of something being overlooked.

2 files changed, 27 insertions, 51 deletions

diff --git a/src/db.c b/src/db.c
index 20a210ba7..31c2f914b 100644
--- a/src/db.c
+++ b/src/db.c
@@ -303,8 +303,8 @@ robj *dbRandomKey(redisDb *db) {
     }
 }
 
-/* Delete a key, value, and associated expiration entry if any, from the DB */
-int dbSyncDelete(redisDb *db, robj *key) {
+/* Helper for sync and async delete. */
+static int dbGenericDelete(redisDb *db, robj *key, int async) {
     /* Deleting an entry from the expires dict will not free the sds of
      * the key, because it is shared with the main dictionary. */
     if (dictSize(db->expires) > 0) dictDelete(db->expires,key->ptr);
@@ -313,6 +313,10 @@ int dbSyncDelete(redisDb *db, robj *key) {
         robj *val = dictGetVal(de);
         /* Tells the module that the key has been unlinked from the database. */
         moduleNotifyKeyUnlink(key,val,db->id);
+        if (async) {
+            freeObjAsync(key, val, db->id);
+            dictSetVal(db->dict, de, NULL);
+        }
         if (server.cluster_enabled) slotToKeyDelEntry(de);
         dictFreeUnlinkedEntry(db->dict,de);
         return 1;
@@ -321,11 +325,21 @@ int dbSyncDelete(redisDb *db, robj *key) {
     }
 }
 
+/* Delete a key, value, and associated expiration entry if any, from the DB */
+int dbSyncDelete(redisDb *db, robj *key) {
+    return dbGenericDelete(db, key, 0);
+}
+
+/* Delete a key, value, and associated expiration entry if any, from the DB. If
+ * the value consists of many allocations, it may be freed asynchronously. */
+int dbAsyncDelete(redisDb *db, robj *key) {
+    return dbGenericDelete(db, key, 1);
+}
+
 /* This is a wrapper whose behavior depends on the Redis lazy free
  * configuration. Deletes the key synchronously or asynchronously. */
 int dbDelete(redisDb *db, robj *key) {
-    return server.lazyfree_lazy_server_del ? dbAsyncDelete(db,key) :
-                                             dbSyncDelete(db,key);
+    return dbGenericDelete(db, key, server.lazyfree_lazy_server_del);
 }
 
 /* Prepare the string object stored at 'key' to be modified destructively
diff --git a/src/lazyfree.c b/src/lazyfree.c
index e793be4cf..10f1ab39f 100644
--- a/src/lazyfree.c
+++ b/src/lazyfree.c
@@ -1,7 +1,6 @@
 #include "server.h"
 #include "bio.h"
 #include "atomicvar.h"
-#include "cluster.h"
 
 static redisAtomic size_t lazyfree_objects = 0;
 static redisAtomic size_t lazyfreed_objects = 0;
@@ -127,57 +126,20 @@ size_t lazyfreeGetFreeEffort(robj *key, robj *obj, int dbid) {
     }
 }
 
-/* Delete a key, value, and associated expiration entry if any, from the DB.
- * If there are enough allocations to free the value object may be put into
- * a lazy free list instead of being freed synchronously. The lazy free list
- * will be reclaimed in a different bio.c thread. */
+/* If there are enough allocations to free the value object asynchronously, it
+ * may be put into a lazy free list instead of being freed synchronously. The
+ * lazy free list will be reclaimed in a different bio.c thread. If the value is
+ * composed of a few allocations, to free in a lazy way is actually just
+ * slower... So under a certain limit we just free the object synchronously. */
 #define LAZYFREE_THRESHOLD 64
-int dbAsyncDelete(redisDb *db, robj *key) {
-    /* Deleting an entry from the expires dict will not free the sds of
-     * the key, because it is shared with the main dictionary. */
-    if (dictSize(db->expires) > 0) dictDelete(db->expires,key->ptr);
-
-    /* If the value is composed of a few allocations, to free in a lazy way
-     * is actually just slower... So under a certain limit we just free
-     * the object synchronously. */
-    dictEntry *de = dictUnlink(db->dict,key->ptr);
-    if (de) {
-        robj *val = dictGetVal(de);
-
-        /* Tells the module that the key has been unlinked from the database. */
-        moduleNotifyKeyUnlink(key,val,db->id);
-
-        size_t free_effort = lazyfreeGetFreeEffort(key,val,db->id);
-
-        /* If releasing the object is too much work, do it in the background
-         * by adding the object to the lazy free list.
-         * Note that if the object is shared, to reclaim it now it is not
-         * possible. This rarely happens, however sometimes the implementation
-         * of parts of the Redis core may call incrRefCount() to protect
-         * objects, and then call dbDelete(). In this case we'll fall
-         * through and reach the dictFreeUnlinkedEntry() call, that will be
-         * equivalent to just calling decrRefCount(). */
-        if (free_effort > LAZYFREE_THRESHOLD && val->refcount == 1) {
-            atomicIncr(lazyfree_objects,1);
-            bioCreateLazyFreeJob(lazyfreeFreeObject,1, val);
-            dictSetVal(db->dict,de,NULL);
-        }
-    }
-
-    /* Release the key-val pair, or just the key if we set the val
-     * field to NULL in order to lazy free it later. */
-    if (de) {
-        if (server.cluster_enabled) slotToKeyDelEntry(de);
-        dictFreeUnlinkedEntry(db->dict,de);
-        return 1;
-    } else {
-        return 0;
-    }
-}
 
 /* Free an object, if the object is huge enough, free it in async way. */
 void freeObjAsync(robj *key, robj *obj, int dbid) {
     size_t free_effort = lazyfreeGetFreeEffort(key,obj,dbid);
+    /* Note that if the object is shared, to reclaim it now it is not
+     * possible. This rarely happens, however sometimes the implementation
+     * of parts of the Redis core may call incrRefCount() to protect
+     * objects, and then call dbDelete(). */
     if (free_effort > LAZYFREE_THRESHOLD && obj->refcount == 1) {
         atomicIncr(lazyfree_objects,1);
         bioCreateLazyFreeJob(lazyfreeFreeObject,1,obj);