Merge branch 'unstable' of https://github.com/PingXie/valkey into cla…

…ng-format
PingXie · May 19, 2024 · 4042432 · 4042432
2 parents 168f10d + efa8ba5
commit 4042432
Show file tree

Hide file tree

Showing 20 changed files with 179 additions and 150 deletions.
diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml
@@ -1073,3 +1073,35 @@ jobs:
     - name: validator
       run: ./utils/req-res-log-validator.py --verbose --fail-missing-reply-schemas ${{ (!contains(github.event.inputs.skiptests, 'valkey') && !contains(github.event.inputs.skiptests, 'module') && !contains(github.event.inputs.sentinel, 'valkey') && !contains(github.event.inputs.skiptests, 'cluster')) && github.event.inputs.test_args == '' && github.event.inputs.cluster_test_args == '' && '--fail-commands-not-all-hit' || '' }}
 
+  notify-about-job-results:
+    runs-on: ubuntu-latest
+    if: always() && github.event_name != 'workflow_dispatch' && github.repository == 'valkey-io/valkey'
+    needs: [test-ubuntu-jemalloc, test-ubuntu-jemalloc-fortify, test-ubuntu-libc-malloc, test-ubuntu-no-malloc-usable-size, test-ubuntu-32bit, test-ubuntu-tls, test-ubuntu-tls-no-tls, test-ubuntu-io-threads, test-ubuntu-reclaim-cache, test-valgrind-test, test-valgrind-misc, test-valgrind-no-malloc-usable-size-test, test-valgrind-no-malloc-usable-size-misc, test-sanitizer-address, test-sanitizer-undefined, test-centos7-jemalloc, test-centos7-tls-module, test-centos7-tls-module-no-tls, test-macos-latest, test-macos-latest-sentinel, test-macos-latest-cluster, build-macos, test-freebsd, test-alpine-jemalloc, test-alpine-libc-malloc, reply-schemas-validator]
+    steps:
+      - name: Collect job status
+        run: |
+          FAILED_JOBS=()
+          NEEDS_JSON='${{ toJSON(needs) }}'
+          JOBS=($(echo "$NEEDS_JSON" | jq 'keys' | tr -d '[] ,'))
+          for JOB in ${JOBS[@]}; do
+            JOB_RESULT=$(echo "$NEEDS_JSON" | jq ".[$JOB][\"result\"]" | tr -d '"')
+            if [ $JOB_RESULT = "failure" ]; then
+              FAILED_JOBS+=($JOB)
+            fi
+          done
+          
+          if [[ ${#FAILED_JOBS[@]} -ne 0 ]]; then
+            echo "FAILED_JOBS=${FAILED_JOBS[@]}" >> $GITHUB_ENV
+            echo "STATUS=failure" >> $GITHUB_ENV
+          else
+            echo "STATUS=success" >> $GITHUB_ENV
+          fi
+      - name: Notify about results
+        uses: ravsamhq/notify-slack-action@v2
+        with:
+          status: ${{ env.STATUS }}
+          notify_when: "failure"
+          notification_title: "Daily test run <${{github.server_url}}/${{github.repository}}/actions/runs/${{github.run_id}}|Failure>"
+          message_format: ":fire: Tests failed: ${{ env.FAILED_JOBS }}"
+        env:
+          SLACK_WEBHOOK_URL: ${{ secrets.SLACK_NOTIFICATIONS_WEBHOOK_URL }}
diff --git a/src/cluster.c b/src/cluster.c
@@ -789,8 +789,8 @@ void clusterCommandMyId(client *c) {
     }
 }
 
-char* getMyClusterId(void) {
-    return clusterNodeGetName(getMyClusterNode());
+int clusterNodeIsMyself(clusterNode *n) {
+    return n == getMyClusterNode();
 }
 
 void clusterCommandMyShardId(client *c) {
@@ -1193,7 +1193,7 @@ clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, i
     if (((c->flags & CLIENT_READONLY) || pubsubshard_included) &&
         !is_write_command &&
         clusterNodeIsSlave(myself) &&
-        clusterNodeGetSlaveof(myself) == n)
+        clusterNodeGetMaster(myself) == n)
     {
         return myself;
     }
@@ -1286,7 +1286,7 @@ int clusterRedirectBlockedClientIfNeeded(client *c) {
              * replica can handle, allow it. */
             if ((c->flags & CLIENT_READONLY) &&
                 !(c->lastcmd->flags & CMD_WRITE) &&
-                clusterNodeIsSlave(myself) && clusterNodeGetSlaveof(myself) == node)
+                clusterNodeIsSlave(myself) && clusterNodeGetMaster(myself) == node)
             {
                 node = myself;
             }

diff --git a/src/cluster.h b/src/cluster.h
@@ -60,33 +60,29 @@ int handleDebugClusterCommand(client *c);
 const char **clusterDebugCommandExtendedHelp(void);
 /* handle implementation specific cluster commands. Return 1 if handled, 0 otherwise. */
 int clusterCommandSpecial(client *c);
-const char** clusterCommandExtendedHelp(void);
+const char **clusterCommandExtendedHelp(void);
 
 int clusterAllowFailoverCmd(client *c);
 void clusterPromoteSelfToMaster(void);
 int clusterManualFailoverTimeLimit(void);
 
-void clusterCommandSlots(client * c);
+void clusterCommandSlots(client *c);
 void clusterCommandMyId(client *c);
 void clusterCommandMyShardId(client *c);
 void clusterCommandShards(client *c);
 sds clusterGenNodeDescription(client *c, clusterNode *node, int tls_primary);
 
 int clusterNodeCoversSlot(clusterNode *n, int slot);
 int getNodeDefaultClientPort(clusterNode *n);
-int clusterNodeIsMyself(clusterNode *n);
 clusterNode *getMyClusterNode(void);
-char *getMyClusterId(void);
 int getClusterSize(void);
 int getMyShardSlotCount(void);
 int handleDebugClusterCommand(client *c);
-int clusterNodePending(clusterNode  *node);
+int clusterNodePending(clusterNode *node);
 int clusterNodeIsMaster(clusterNode *n);
 char **getClusterNodesList(size_t *numnodes);
-int clusterNodeIsMaster(clusterNode *n);
 char *clusterNodeIp(clusterNode *node);
 int clusterNodeIsSlave(clusterNode *node);
-clusterNode *clusterNodeGetSlaveof(clusterNode *node);
 clusterNode *clusterNodeGetMaster(clusterNode *node);
 char *clusterNodeGetName(clusterNode *node);
 int clusterNodeTimedOut(clusterNode *node);
@@ -106,6 +102,7 @@ clusterNode *clusterLookupNode(const char *name, int length);
 void clusterReplicateOpenSlots(void);
 
 /* functions with shared implementations */
+int clusterNodeIsMyself(clusterNode *n);
 clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, int argc, int *hashslot, int *ask);
 int clusterRedirectBlockedClientIfNeeded(client *c);
 void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code);

diff --git a/src/cluster_legacy.c b/src/cluster_legacy.c
@@ -6090,10 +6090,6 @@ unsigned int countChannelsInSlot(unsigned int hashslot) {
     return kvstoreDictSize(server.pubsubshard_channels, hashslot);
 }
 
-int clusterNodeIsMyself(clusterNode *n) {
-    return n == server.cluster->myself;
-}
-
 clusterNode *getMyClusterNode(void) {
     return server.cluster->myself;
 }
@@ -6175,7 +6171,7 @@ int handleDebugClusterCommand(client *c) {
     return 1;
 }
 
-int clusterNodePending(clusterNode  *node) {
+int clusterNodePending(clusterNode *node) {
     return node->flags & (CLUSTER_NODE_NOADDR|CLUSTER_NODE_HANDSHAKE);
 }
 
@@ -6187,10 +6183,6 @@ int clusterNodeIsSlave(clusterNode *node) {
     return node->flags & CLUSTER_NODE_SLAVE;
 }
 
-clusterNode *clusterNodeGetSlaveof(clusterNode *node) {
-    return node->slaveof;
-}
-
 clusterNode *clusterNodeGetMaster(clusterNode *node) {
     while (node->slaveof != NULL) node = node->slaveof;
     return node;

diff --git a/src/db.c b/src/db.c
@@ -885,11 +885,10 @@ void scanCallback(void *privdata, const dictEntry *de) {
     serverAssert(!((data->type != LLONG_MAX) && o));
 
     /* Filter an element if it isn't the type we want. */
-    /* TODO: uncomment in version 8.0
     if (!o && data->type != LLONG_MAX) {
         robj *rval = dictGetVal(de);
         if (!objectTypeCompare(rval, data->type)) return;
-    }*/
+    }
 
     /* Filter element if it does not match the pattern. */
     sds keysds = dictGetKey(de);
@@ -1034,9 +1033,8 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
             typename = c->argv[i+1]->ptr;
             type = getObjectTypeByName(typename);
             if (type == LLONG_MAX) {
-                /* TODO: uncomment in version 8.0
                 addReplyErrorFormat(c, "unknown type name '%s'", typename);
-                return; */
+                return;
             }
             i+= 2;
         } else if (!strcasecmp(c->argv[i]->ptr, "novalues")) {
@@ -1195,15 +1193,6 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
         while ((ln = listNext(&li))) {
             sds key = listNodeValue(ln);
             initStaticStringObject(kobj, key);
-            /* Filter an element if it isn't the type we want. */
-            /* TODO: remove this in version 8.0 */
-            if (typename) {
-                robj* typecheck = lookupKeyReadWithFlags(c->db, &kobj, LOOKUP_NOTOUCH|LOOKUP_NONOTIFY);
-                if (!typecheck || !objectTypeCompare(typecheck, type)) {
-                    listDelNode(keys, ln);
-                }
-                continue;
-            }
             if (expireIfNeeded(c->db, &kobj, 0) != KEY_VALID) {
                 listDelNode(keys, ln);
             }

diff --git a/src/module.c b/src/module.c
@@ -9017,7 +9017,7 @@ void VM_FreeClusterNodesList(char **ids) {
  * is disabled. */
 const char *VM_GetMyClusterID(void) {
     if (!server.cluster_enabled) return NULL;
-    return getMyClusterId();
+    return clusterNodeGetName(getMyClusterNode());
 }
 
 /* Return the number of nodes in the cluster, regardless of their state
@@ -9064,8 +9064,8 @@ int VM_GetClusterNodeInfo(ValkeyModuleCtx *ctx, const char *id, char *ip, char *
         /* If the information is not available, the function will set the
          * field to zero bytes, so that when the field can't be populated the
          * function kinda remains predictable. */
-        if (clusterNodeIsSlave(node) && clusterNodeGetSlaveof(node))
-            memcpy(master_id, clusterNodeGetName(clusterNodeGetSlaveof(node)) ,VALKEYMODULE_NODE_ID_LEN);
+        if (clusterNodeIsSlave(node) && clusterNodeGetMaster(node))
+            memcpy(master_id, clusterNodeGetName(clusterNodeGetMaster(node)) ,VALKEYMODULE_NODE_ID_LEN);
         else
             memset(master_id,0,VALKEYMODULE_NODE_ID_LEN);
     }

diff --git a/src/sds.c b/src/sds.c
@@ -349,6 +349,7 @@ sds sdsResize(sds s, size_t size, int would_regrow) {
      * type. */
     int use_realloc = (oldtype==type || (type < oldtype && type > SDS_TYPE_8));
     size_t newlen = use_realloc ? oldhdrlen+size+1 : hdrlen+size+1;
+    size_t newsize = 0;
 
     if (use_realloc) {
         int alloc_already_optimal = 0;
@@ -357,24 +358,27 @@ sds sdsResize(sds s, size_t size, int would_regrow) {
              * We aim to avoid calling realloc() when using Jemalloc if there is no
              * change in the allocation size, as it incurs a cost even if the
              * allocation size stays the same. */
-            alloc_already_optimal = (je_nallocx(newlen, 0) == zmalloc_size(sh));
+            newsize = zmalloc_size(sh);
+            alloc_already_optimal = (je_nallocx(newlen, 0) == newsize);
         #endif
         if (!alloc_already_optimal) {
-            newsh = s_realloc(sh, newlen);
+            newsh = s_realloc_usable(sh, newlen, &newsize);
             if (newsh == NULL) return NULL;
             s = (char*)newsh+oldhdrlen;
+            newsize -= (oldhdrlen + 1);
         }
     } else {
-        newsh = s_malloc(newlen);
+        newsh = s_malloc_usable(newlen, &newsize);
         if (newsh == NULL) return NULL;
         memcpy((char*)newsh+hdrlen, s, len);
         s_free(sh);
         s = (char*)newsh+hdrlen;
         s[-1] = type;
+        newsize -= (hdrlen + 1);
     }
-    s[len] = 0;
+    s[len] = '\0';
     sdssetlen(s, len);
-    sdssetalloc(s, size);
+    sdssetalloc(s, newsize);
     return s;
 }
 

diff --git a/src/server.c b/src/server.c
@@ -6932,7 +6932,6 @@ struct serverTest {
     {"ziplist", ziplistTest},
     {"quicklist", quicklistTest},
     {"zipmap", zipmapTest},
-    {"zmalloc", zmalloc_test},
     {"dict", dictTest},
     {"listpack", listpackTest},
 };

diff --git a/src/unit/test_files.h b/src/unit/test_files.h
@@ -30,6 +30,9 @@ int test_ll2string(int argc, char **argv, int flags);
 int test_ld2string(int argc, char **argv, int flags);
 int test_fixedpoint_d2string(int argc, char **argv, int flags);
 int test_reclaimFilePageCache(int argc, char **argv, int flags);
+int test_zmallocInitialUsedMemory(int argc, char **argv, int flags);
+int test_zmallocAllocReallocCallocAndFree(int argc, char **argv, int flags);
+int test_zmallocAllocZeroByteAndFree(int argc, char **argv, int flags);
 
 unitTest __test_crc64_c[] = {{"test_crc64", test_crc64}, {NULL, NULL}};
 unitTest __test_crc64combine_c[] = {{"test_crc64combine", test_crc64combine}, {NULL, NULL}};
@@ -39,6 +42,7 @@ unitTest __test_kvstore_c[] = {{"test_kvstoreAdd16Keys", test_kvstoreAdd16Keys},
 unitTest __test_sds_c[] = {{"test_sds", test_sds}, {NULL, NULL}};
 unitTest __test_sha1_c[] = {{"test_sha1", test_sha1}, {NULL, NULL}};
 unitTest __test_util_c[] = {{"test_string2ll", test_string2ll}, {"test_string2l", test_string2l}, {"test_ll2string", test_ll2string}, {"test_ld2string", test_ld2string}, {"test_fixedpoint_d2string", test_fixedpoint_d2string}, {"test_reclaimFilePageCache", test_reclaimFilePageCache}, {NULL, NULL}};
+unitTest __test_zmalloc_c[] = {{"test_zmallocInitialUsedMemory", test_zmallocInitialUsedMemory}, {"test_zmallocAllocReallocCallocAndFree", test_zmallocAllocReallocCallocAndFree}, {"test_zmallocAllocZeroByteAndFree", test_zmallocAllocZeroByteAndFree}, {NULL, NULL}};
 
 struct unitTestSuite {
     char *filename;
@@ -52,4 +56,5 @@ struct unitTestSuite {
     {"test_sds.c", __test_sds_c},
     {"test_sha1.c", __test_sha1_c},
     {"test_util.c", __test_util_c},
+    {"test_zmalloc.c", __test_zmalloc_c},
 };
diff --git a/src/unit/test_sds.c b/src/unit/test_sds.c
@@ -227,32 +227,40 @@ int test_sds(int argc, char **argv, int flags) {
                 memcmp(x, "v1={value1} {} v2=value2", 24) == 0);
     sdsfree(x);
 
-    /* Test sdsresize - extend */
+    /* Test sdsResize - extend */
     x = sdsnew("1234567890123456789012345678901234567890");
     x = sdsResize(x, 200, 1);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand len", sdslen(x) == 40);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand strlen", strlen(x) == 40);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand alloc", sdsalloc(x) == 200);
-    /* Test sdsresize - trim free space */
+    TEST_ASSERT_MESSAGE("sdsReszie() expand type", x[-1] == SDS_TYPE_8);
+    TEST_ASSERT_MESSAGE("sdsReszie() expand len", sdslen(x) == 40);
+    TEST_ASSERT_MESSAGE("sdsReszie() expand strlen", strlen(x) == 40);
+    /* Different allocator allocates at least as large as requested size,
+     * to confirm the allocator won't waste too much,
+     * we add a largest size checker here. */
+    TEST_ASSERT_MESSAGE("sdsReszie() expand alloc", sdsalloc(x) >= 200 && sdsalloc(x) < 400);
+    /* Test sdsResize - trim free space */
     x = sdsResize(x, 80, 1);
-    TEST_ASSERT_MESSAGE("sdsrezie() shrink len", sdslen(x) == 40);
-    TEST_ASSERT_MESSAGE("sdsrezie() shrink strlen", strlen(x) == 40);
-    TEST_ASSERT_MESSAGE("sdsrezie() shrink alloc", sdsalloc(x) == 80);
-    /* Test sdsresize - crop used space */
+    TEST_ASSERT_MESSAGE("sdsReszie() shrink type", x[-1] == SDS_TYPE_8);
+    TEST_ASSERT_MESSAGE("sdsReszie() shrink len", sdslen(x) == 40);
+    TEST_ASSERT_MESSAGE("sdsReszie() shrink strlen", strlen(x) == 40);
+    TEST_ASSERT_MESSAGE("sdsReszie() shrink alloc", sdsalloc(x) >= 80);
+    /* Test sdsResize - crop used space */
     x = sdsResize(x, 30, 1);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop len", sdslen(x) == 30);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop strlen", strlen(x) == 30);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop alloc", sdsalloc(x) == 30);
-    /* Test sdsresize - extend to different class */
+    TEST_ASSERT_MESSAGE("sdsReszie() crop type", x[-1] == SDS_TYPE_8);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop len", sdslen(x) == 30);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop strlen", strlen(x) == 30);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop alloc", sdsalloc(x) >= 30);
+    /* Test sdsResize - extend to different class */
     x = sdsResize(x, 400, 1);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand len", sdslen(x) == 30);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand strlen", strlen(x) == 30);
-    TEST_ASSERT_MESSAGE("sdsrezie() expand alloc", sdsalloc(x) == 400);
-    /* Test sdsresize - shrink to different class */
+    TEST_ASSERT_MESSAGE("sdsReszie() expand type", x[-1] == SDS_TYPE_16);
+    TEST_ASSERT_MESSAGE("sdsReszie() expand len", sdslen(x) == 30);
+    TEST_ASSERT_MESSAGE("sdsReszie() expand strlen", strlen(x) == 30);
+    TEST_ASSERT_MESSAGE("sdsReszie() expand alloc", sdsalloc(x) >= 400);
+    /* Test sdsResize - shrink to different class */
     x = sdsResize(x, 4, 1);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop len", sdslen(x) == 4);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop strlen", strlen(x) == 4);
-    TEST_ASSERT_MESSAGE("sdsrezie() crop alloc", sdsalloc(x) == 4);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop type", x[-1] == SDS_TYPE_8);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop len", sdslen(x) == 4);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop strlen", strlen(x) == 4);
+    TEST_ASSERT_MESSAGE("sdsReszie() crop alloc", sdsalloc(x) >= 4);
     sdsfree(x);
     return 0;
 }
diff --git a/src/unit/test_util.c b/src/unit/test_util.c
@@ -266,12 +266,14 @@ static int cache_exist(int fd) {
      * page is currently resident in memory */
     return flag&1;
 }
+#endif
 
 int test_reclaimFilePageCache(int argc, char **argv, int flags) {
     UNUSED(argc);
     UNUSED(argv);
     UNUSED(flags);
 
+#if defined(__linux__)
     char *tmpfile = "/tmp/redis-reclaim-cache-test";
     int fd = open(tmpfile, O_RDWR|O_CREAT, 0644);
     TEST_ASSERT(fd >= 0);
@@ -291,8 +293,6 @@ int test_reclaimFilePageCache(int argc, char **argv, int flags) {
     TEST_ASSERT(!cache_exist(fd));
 
     unlink(tmpfile);
-    printf("reclaimFilePageCache test is ok\n");
-
+#endif
     return 0;
 }
-#endif