Merge remote-tracking branch 'valkey/unstable' into hashset

valkey-io · Nov 18, 2024 · 2a60fe5 · 2a60fe5
2 parents e43a34a + d07674f
commit 2a60fe5
Show file tree

Hide file tree

Showing 29 changed files with 2,649 additions and 1,661 deletions.
diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml
@@ -60,7 +60,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make all-with-unit-tests SERVER_CFLAGS='-Werror -DSERVER_TEST'
+        run: make all-with-unit-tests SERVER_CFLAGS='-Werror'
       - name: testprep
         run: sudo apt-get install tcl8.6 tclx
       - name: test
@@ -75,10 +75,7 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: legacy unit tests
-        if: true && !contains(github.event.inputs.skiptests, 'unittest')
-        run: ./src/valkey-server test all --accurate
-      - name: new unit tests
+      - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests --accurate
 
@@ -109,7 +106,7 @@ jobs:
         run: |
           apt-get update && apt-get install -y make gcc-13
           update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-13 100
-          make all-with-unit-tests CC=gcc OPT=-O3 SERVER_CFLAGS='-Werror -DSERVER_TEST -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3'
+          make all-with-unit-tests CC=gcc OPT=-O3 SERVER_CFLAGS='-Werror -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3'
       - name: testprep
         run: apt-get install -y tcl8.6 tclx procps
       - name: test
@@ -124,10 +121,7 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: legacy unit tests
-        if: true && !contains(github.event.inputs.skiptests, 'unittest')
-        run: ./src/valkey-server test all --accurate
-      - name: new unit tests
+      - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests --accurate
 
@@ -234,7 +228,7 @@ jobs:
       - name: make
         run: |
           sudo apt-get update && sudo apt-get install libc6-dev-i386
-          make 32bit SERVER_CFLAGS='-Werror -DSERVER_TEST'
+          make 32bit SERVER_CFLAGS='-Werror'
       - name: testprep
         run: sudo apt-get install tcl8.6 tclx
       - name: test
@@ -251,10 +245,7 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: legacy unit tests
-        if: true && !contains(github.event.inputs.skiptests, 'unittest')
-        run: ./src/valkey-server test all --accurate
-      - name: new unit tests
+      - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests --accurate
 
@@ -483,7 +474,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make valgrind SERVER_CFLAGS='-Werror -DSERVER_TEST'
+        run: make valgrind SERVER_CFLAGS='-Werror'
       - name: testprep
         run: |
           sudo apt-get update
@@ -515,7 +506,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make valgrind SERVER_CFLAGS='-Werror -DSERVER_TEST'
+        run: make valgrind all-with-unit-tests SERVER_CFLAGS='-Werror'
       - name: testprep
         run: |
           sudo apt-get update
@@ -526,7 +517,7 @@ jobs:
       - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: |
-          valgrind --track-origins=yes --suppressions=./src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full --log-file=err.txt ./src/valkey-server test all --valgrind
+          valgrind --track-origins=yes --suppressions=./src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full --log-file=err.txt ./src/valkey-unit-tests --valgrind
           if grep -q 0x err.txt; then cat err.txt; exit 1; fi
 
   test-valgrind-no-malloc-usable-size-test:
@@ -552,7 +543,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make valgrind CFLAGS="-DNO_MALLOC_USABLE_SIZE -DSERVER_TEST" SERVER_CFLAGS='-Werror'
+        run: make valgrind CFLAGS="-DNO_MALLOC_USABLE_SIZE" SERVER_CFLAGS='-Werror'
       - name: testprep
         run: |
           sudo apt-get update
@@ -584,7 +575,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make valgrind CFLAGS="-DNO_MALLOC_USABLE_SIZE -DSERVER_TEST" SERVER_CFLAGS='-Werror'
+        run: make valgrind all-with-unit-tests CFLAGS="-DNO_MALLOC_USABLE_SIZE" SERVER_CFLAGS='-Werror'
       - name: testprep
         run: |
           sudo apt-get update
@@ -595,7 +586,7 @@ jobs:
       - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: |
-          valgrind --track-origins=yes --suppressions=./src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full --log-file=err.txt ./src/valkey-server test all --valgrind
+          valgrind --track-origins=yes --suppressions=./src/valgrind.sup --show-reachable=no --show-possibly-lost=no --leak-check=full --log-file=err.txt ./src/valkey-unit-tests --valgrind
           if grep -q 0x err.txt; then cat err.txt; exit 1; fi
 
   test-sanitizer-address:
@@ -627,7 +618,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make all-with-unit-tests OPT=-O3 SANITIZER=address SERVER_CFLAGS='-DSERVER_TEST -Werror'
+        run: make all-with-unit-tests OPT=-O3 SANITIZER=address SERVER_CFLAGS='-Werror'
       - name: testprep
         run: |
           sudo apt-get update
@@ -644,10 +635,7 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: legacy unit tests
-        if: true && !contains(github.event.inputs.skiptests, 'unittest')
-        run: ./src/valkey-server test all
-      - name: new unit tests
+      - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests
 
@@ -680,7 +668,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make all-with-unit-tests OPT=-O3 SANITIZER=undefined SERVER_CFLAGS='-DSERVER_TEST -Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
+        run: make all-with-unit-tests OPT=-O3 SANITIZER=undefined SERVER_CFLAGS='-Werror' LUA_DEBUG=yes # we (ab)use this flow to also check Lua C API violations
       - name: testprep
         run: |
           sudo apt-get update
@@ -697,10 +685,7 @@ jobs:
       - name: cluster tests
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
-      - name: legacy unit tests
-        if: true && !contains(github.event.inputs.skiptests, 'unittest')
-        run: ./src/valkey-server test all --accurate
-      - name: new unit tests
+      - name: unittest
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests --accurate
 
@@ -1031,7 +1016,7 @@ jobs:
           repository: ${{ env.GITHUB_REPOSITORY }}
           ref: ${{ env.GITHUB_HEAD_REF }}
       - name: make
-        run: make SERVER_CFLAGS='-Werror -DSERVER_TEST'
+        run: make SERVER_CFLAGS='-Werror'
 
   test-freebsd:
     runs-on: macos-12

diff --git a/src/Makefile b/src/Makefile
@@ -25,7 +25,7 @@ ifeq ($(OPTIMIZATION),-O3)
 	ifeq (clang,$(CLANG))
 		OPTIMIZATION+=-flto
 	else
-		OPTIMIZATION+=-flto=auto
+		OPTIMIZATION+=-flto=auto -ffat-lto-objects
 	endif
 endif
 ifneq ($(OPTIMIZATION),-O0)
@@ -131,9 +131,6 @@ ifdef REDIS_LDFLAGS
 endif
 
 FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(SERVER_CFLAGS)
-ifeq ($(SERVER_TEST),yes)
-	FINAL_CFLAGS +=-DSERVER_TEST=1
-endif
 FINAL_LDFLAGS=$(LDFLAGS) $(OPT) $(SERVER_LDFLAGS) $(DEBUG)
 FINAL_LIBS=-lm
 DEBUG=-g -ggdb

diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c
@@ -4909,6 +4909,8 @@ static int clusterNodeCronHandleReconnect(clusterNode *node, mstime_t handshake_
     /* A Node in HANDSHAKE state has a limited lifespan equal to the
      * configured node timeout. */
     if (nodeInHandshake(node) && now - node->ctime > handshake_timeout) {
+        serverLog(LL_WARNING, "Clusterbus handshake timeout %s:%d after %lldms", node->ip,
+                  node->cport, handshake_timeout);
         clusterDelNode(node);
         return 1;
     }
@@ -6082,6 +6084,9 @@ void removeChannelsInSlot(unsigned int slot) {
 unsigned int delKeysInSlot(unsigned int hashslot) {
     if (!countKeysInSlot(hashslot)) return 0;
 
+    /* We may lose a slot during the pause. We need to track this
+     * state so that we don't assert in propagateNow(). */
+    server.server_del_keys_in_slot = 1;
     unsigned int j = 0;
 
     kvstoreDictIterator *kvs_di = NULL;
@@ -6106,6 +6111,8 @@ unsigned int delKeysInSlot(unsigned int hashslot) {
     }
     kvstoreReleaseDictIterator(kvs_di);
 
+    server.server_del_keys_in_slot = 0;
+    serverAssert(server.execution_nesting == 0);
     return j;
 }
 

diff --git a/src/config.c b/src/config.c
@@ -1009,15 +1009,14 @@ void configGetCommand(client *c) {
 
 #define CONFIG_REWRITE_SIGNATURE "# Generated by CONFIG REWRITE"
 
-/* We use the following dictionary type to store where a configuration
- * option is mentioned in the old configuration file, so it's
- * like "maxmemory" -> list of line numbers (first line is zero). */
-void dictListDestructor(dict *d, void *val);
-
 /* Sentinel config rewriting is implemented inside sentinel.c by
  * rewriteConfigSentinelOption(). */
 void rewriteConfigSentinelOption(struct rewriteConfigState *state);
 
+/* We use the following dictionary type to store where a configuration
+ * option is mentioned in the old configuration file, so it's
+ * like "maxmemory" -> list of line numbers (first line is zero).
+ */
 dictType optionToLineDictType = {
     dictSdsCaseHash,       /* hash function */
     NULL,                  /* key dup */

diff --git a/src/dict.c b/src/dict.c
@@ -576,7 +576,7 @@ dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing) {
     if (!position) return NULL;
 
     /* Dup the key if necessary. */
-    if (d->type->keyDup) key = d->type->keyDup(d, key);
+    if (d->type->keyDup) key = d->type->keyDup(key);
 
     return dictInsertAtPosition(d, key, position);
 }
@@ -640,7 +640,7 @@ int dictReplace(dict *d, void *key, void *val) {
      * reverse. */
     void *oldval = dictGetVal(existing);
     dictSetVal(d, existing, val);
-    if (d->type->valDestructor) d->type->valDestructor(d, oldval);
+    if (d->type->valDestructor) d->type->valDestructor(oldval);
     return 0;
 }
 
@@ -742,6 +742,18 @@ dictEntry *dictUnlink(dict *d, const void *key) {
     return dictGenericDelete(d, key, 1);
 }
 
+inline static void dictFreeKey(dict *d, dictEntry *entry) {
+    if (d->type->keyDestructor) {
+        d->type->keyDestructor(dictGetKey(entry));
+    }
+}
+
+inline static void dictFreeVal(dict *d, dictEntry *entry) {
+    if (d->type->valDestructor) {
+        d->type->valDestructor(dictGetVal(entry));
+    }
+}
+
 /* You need to call this function to really free the entry after a call
  * to dictUnlink(). It's safe to call this function with 'he' = NULL. */
 void dictFreeUnlinkedEntry(dict *d, dictEntry *he) {
@@ -919,7 +931,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table
          : (entryIsEmbedded(de) ? &decodeEntryEmbedded(de)->field : (panic("Entry type not supported"), NULL)))
 
 void dictSetKey(dict *d, dictEntry *de, void *key) {
-    void *k = d->type->keyDup ? d->type->keyDup(d, key) : key;
+    void *k = d->type->keyDup ? d->type->keyDup(key) : key;
     if (entryIsNormal(de)) {
         dictEntryNormal *_de = decodeEntryNormal(de);
         _de->key = k;

diff --git a/src/dict.h b/src/dict.h
@@ -53,10 +53,10 @@ typedef struct dict dict;
 typedef struct dictType {
     /* Callbacks */
     uint64_t (*hashFunction)(const void *key);
-    void *(*keyDup)(dict *d, const void *key);
-    int (*keyCompare)(dict *d, const void *key1, const void *key2);
-    void (*keyDestructor)(dict *d, void *key);
-    void (*valDestructor)(dict *d, void *obj);
+    void *(*keyDup)(const void *key);
+    int (*keyCompare)(const void *key1, const void *key2);
+    void (*keyDestructor)(void *key);
+    void (*valDestructor)(void *obj);
     int (*resizeAllowed)(size_t moreMem, double usedRatio);
     /* Invoked at the start of dict initialization/rehashing (old and new ht are already created) */
     void (*rehashingStarted)(dict *d);
@@ -144,16 +144,13 @@ typedef struct {
 #define DICT_HT_INITIAL_SIZE (1 << (DICT_HT_INITIAL_EXP))
 
 /* ------------------------------- Macros ------------------------------------*/
-#define dictFreeVal(d, entry)                                                           \
-    do {                                                                                \
-        if ((d)->type->valDestructor) (d)->type->valDestructor((d), dictGetVal(entry)); \
-    } while (0)
-
-#define dictFreeKey(d, entry) \
-    if ((d)->type->keyDestructor) (d)->type->keyDestructor((d), dictGetKey(entry))
-
-#define dictCompareKeys(d, key1, key2) \
-    (((d)->type->keyCompare) ? (d)->type->keyCompare((d), key1, key2) : (key1) == (key2))
+static inline int dictCompareKeys(dict *d, const void *key1, const void *key2) {
+    if (d->type->keyCompare) {
+        return d->type->keyCompare(key1, key2);
+    } else {
+        return (key1 == key2);
+    }
+}
 
 #define dictMetadata(d) (&(d)->metadata)
 #define dictMetadataSize(d) ((d)->type->dictMetadataBytes ? (d)->type->dictMetadataBytes(d) : 0)

diff --git a/src/eval.c b/src/eval.c
@@ -57,8 +57,7 @@ void evalGenericCommandWithDebugging(client *c, int evalsha);
 sds ldbCatStackValue(sds s, lua_State *lua, int idx);
 listNode *luaScriptsLRUAdd(client *c, sds sha, int evalsha);
 
-static void dictLuaScriptDestructor(dict *d, void *val) {
-    UNUSED(d);
+static void dictLuaScriptDestructor(void *val) {
     if (val == NULL) return; /* Lazy freeing will set value to NULL. */
     decrRefCount(((luaScript *)val)->body);
     zfree(val);

diff --git a/src/functions.c b/src/functions.c
@@ -43,9 +43,9 @@ typedef enum {
 static size_t engine_cache_memory = 0;
 
 /* Forward declaration */
-static void engineFunctionDispose(dict *d, void *obj);
-static void engineStatsDispose(dict *d, void *obj);
-static void engineLibraryDispose(dict *d, void *obj);
+static void engineFunctionDispose(void *obj);
+static void engineStatsDispose(void *obj);
+static void engineLibraryDispose(void *obj);
 static int functionsVerifyName(sds name);
 
 typedef struct functionsLibEngineStats {
@@ -126,15 +126,13 @@ static size_t libraryMallocSize(functionLibInfo *li) {
     return zmalloc_size(li) + sdsAllocSize(li->name) + sdsAllocSize(li->code);
 }
 
-static void engineStatsDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineStatsDispose(void *obj) {
     functionsLibEngineStats *stats = obj;
     zfree(stats);
 }
 
 /* Dispose function memory */
-static void engineFunctionDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineFunctionDispose(void *obj) {
     if (!obj) {
         return;
     }
@@ -158,8 +156,7 @@ static void engineLibraryFree(functionLibInfo *li) {
     zfree(li);
 }
 
-static void engineLibraryDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineLibraryDispose(void *obj) {
     engineLibraryFree(obj);
 }
 

diff --git a/src/latency.c b/src/latency.c
@@ -37,8 +37,7 @@
 #include "hdr_histogram.h"
 
 /* Dictionary type for latency events. */
-int dictStringKeyCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictStringKeyCompare(const void *key1, const void *key2) {
     return strcmp(key1, key2) == 0;
 }
 

diff --git a/src/module.c b/src/module.c
@@ -11814,8 +11814,7 @@ uint64_t dictCStringKeyHash(const void *key) {
     return dictGenHashFunction((unsigned char *)key, strlen((char *)key));
 }
 
-int dictCStringKeyCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictCStringKeyCompare(const void *key1, const void *key2) {
     return strcmp(key1, key2) == 0;
 }