Merge branch 'valkey-io:unstable' into unstable

.github/actions/generate-package-build-matrix/action.yml

@@ -24,11 +24,11 @@ runs:
 
     - name: Get targets
       run: |
-        x86_arch=$(jq -c '[.linux_targets[] | select(.arch=="x86_64")]' utils/releasetools/build-config.json)
+        x86_arch=$(jq -c '[.linux_targets[] | select(.arch=="x86_64")]' .github/actions/generate-package-build-matrix/build-config.json)
         x86_matrix=$(echo "{ \"distro\" : $x86_arch }" | jq -c .)
         echo "X86_MATRIX=$x86_matrix" >> $GITHUB_ENV
 
-        arm_arch=$(jq -c '[.linux_targets[] | select(.arch=="arm64")]' utils/releasetools/build-config.json)
+        arm_arch=$(jq -c '[.linux_targets[] | select(.arch=="arm64")]' .github/actions/generate-package-build-matrix/build-config.json)
         arm_matrix=$(echo "{ \"distro\" : $arm_arch }" | jq -c .)
         echo "ARM_MATRIX=$arm_matrix" >> $GITHUB_ENV
       shell: bash

.github/workflows/build-release-packages.yml

@@ -8,7 +8,7 @@ on:
       - '.github/workflows/build-release-packages.yml'
       - '.github/workflows/call-build-linux-arm-packages.yml'
       - '.github/workflows/call-build-linux-x86-packages.yml'
-      - 'utils/releasetools/build-config.json'
+      - '.github/actions/generate-package-build-matrix/build-config.json'
   workflow_dispatch:
     inputs:
       version:
@@ -23,7 +23,7 @@ jobs:
   # This job provides the version metadata from the tag for the other jobs to use.
   release-build-get-meta:
     name: Get metadata to build
-    if: github.repository == 'valkey-io/valkey'
+    if: github.event_name == 'workflow_dispatch' || github.repository == 'valkey-io/valkey'
     runs-on: ubuntu-latest
     outputs:
       version: ${{ steps.get_version.outputs.VERSION }}
@@ -59,15 +59,17 @@ jobs:
         id: check-if-testing
         run: |
           if [[ "${{ github.event_name }}" == "push" ]]; then
+            echo "This is a test workflow -> We will upload to the Test S3 Bucket"
             echo "IS_TEST=true" >> $GITHUB_OUTPUT
           else
+            echo "This is a Release workflow -> We will upload to the Release S3 Bucket"
             echo "IS_TEST=false" >> $GITHUB_OUTPUT
           fi
         shell: bash
 
   generate-build-matrix:
     name: Generating build matrix
-    if: github.repository == 'valkey-io/valkey'
+    if: github.event_name == 'workflow_dispatch' || github.repository == 'valkey-io/valkey'
     runs-on: ubuntu-latest
     outputs:
       x86_64-build-matrix: ${{ steps.set-matrix.outputs.x86_64-build-matrix }}
@@ -92,7 +94,7 @@ jobs:
       build_matrix: ${{ needs.generate-build-matrix.outputs.x86_64-build-matrix }}
       region: us-west-2
     secrets:
-      bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
+      bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_S3_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
       role_to_assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
 
   release-build-linux-arm-packages:
@@ -106,5 +108,5 @@ jobs:
       build_matrix: ${{ needs.generate-build-matrix.outputs.arm64-build-matrix }}
       region: us-west-2
     secrets:
-      bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
+      bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_S3_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
       role_to_assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}

.github/workflows/daily.yml

@@ -375,6 +375,44 @@ jobs:
         if: true && !contains(github.event.inputs.skiptests, 'cluster')
         run: ./runtest-cluster --io-threads ${{github.event.inputs.cluster_test_args}}
 
+  test-ubuntu-tls-io-threads:
+    runs-on: ubuntu-latest
+    if: |
+      (github.event_name == 'workflow_dispatch' ||
+        (github.event_name == 'schedule' && github.repository == 'valkey-io/valkey') ||
+        (github.event_name == 'pull_request' && (contains(github.event.pull_request.labels.*.name, 'run-extra-tests') || github.event.pull_request.base.ref != 'unstable'))) &&
+      !contains(github.event.inputs.skipjobs, 'tls') && !contains(github.event.inputs.skipjobs, 'iothreads')
+    timeout-minutes: 14400
+    steps:
+      - name: prep
+        if: github.event_name == 'workflow_dispatch'
+        run: |
+          echo "GITHUB_REPOSITORY=${{github.event.inputs.use_repo}}" >> $GITHUB_ENV
+          echo "GITHUB_HEAD_REF=${{github.event.inputs.use_git_ref}}" >> $GITHUB_ENV
+          echo "skipjobs: ${{github.event.inputs.skipjobs}}"
+          echo "skiptests: ${{github.event.inputs.skiptests}}"
+          echo "test_args: ${{github.event.inputs.test_args}}"
+          echo "cluster_test_args: ${{github.event.inputs.cluster_test_args}}"
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        with:
+          repository: ${{ env.GITHUB_REPOSITORY }}
+          ref: ${{ env.GITHUB_HEAD_REF }}
+      - name: make
+        run: |
+          make BUILD_TLS=yes SERVER_CFLAGS='-Werror'
+      - name: testprep
+        run: |
+          sudo apt-get install tcl8.6 tclx tcl-tls
+          ./utils/gen-test-certs.sh
+      - name: test
+        if: true && !contains(github.event.inputs.skiptests, 'valkey')
+        run: |
+          ./runtest --io-threads --tls --accurate --verbose --tags network --dump-logs ${{github.event.inputs.test_args}}
+      - name: cluster tests
+        if: true && !contains(github.event.inputs.skiptests, 'cluster')
+        run: |
+          ./runtest-cluster --io-threads --tls ${{github.event.inputs.cluster_test_args}}
+
   test-ubuntu-reclaim-cache:
     runs-on: ubuntu-latest
     if: |
@@ -689,6 +727,52 @@ jobs:
         if: true && !contains(github.event.inputs.skiptests, 'unittest')
         run: ./src/valkey-unit-tests --accurate
 
+  test-sanitizer-force-defrag:
+    runs-on: ubuntu-latest
+    if: |
+      (github.event_name == 'workflow_dispatch' ||
+        (github.event_name == 'schedule' && github.repository == 'valkey-io/valkey') ||
+        (github.event_name == 'pull_request' && github.event.pull_request.base.ref != 'unstable')) &&
+      !contains(github.event.inputs.skipjobs, 'sanitizer')
+    timeout-minutes: 14400
+    strategy:
+      fail-fast: false
+    steps:
+      - name: prep
+        if: github.event_name == 'workflow_dispatch'
+        run: |
+          echo "GITHUB_REPOSITORY=${{github.event.inputs.use_repo}}" >> $GITHUB_ENV
+          echo "GITHUB_HEAD_REF=${{github.event.inputs.use_git_ref}}" >> $GITHUB_ENV
+          echo "skipjobs: ${{github.event.inputs.skipjobs}}"
+          echo "skiptests: ${{github.event.inputs.skiptests}}"
+          echo "test_args: ${{github.event.inputs.test_args}}"
+          echo "cluster_test_args: ${{github.event.inputs.cluster_test_args}}"
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+        with:
+          repository: ${{ env.GITHUB_REPOSITORY }}
+          ref: ${{ env.GITHUB_HEAD_REF }}
+      - name: make
+        run: make all-with-unit-tests OPT=-O3 SANITIZER=address DEBUG_FORCE_DEFRAG=yes USE_JEMALLOC=no SERVER_CFLAGS='-Werror'
+      - name: testprep
+        run: |
+          sudo apt-get update
+          sudo apt-get install tcl8.6 tclx -y
+      - name: test
+        if: true && !contains(github.event.inputs.skiptests, 'valkey')
+        run: ./runtest --accurate --verbose --dump-logs ${{github.event.inputs.test_args}}
+      - name: module api test
+        if: true && !contains(github.event.inputs.skiptests, 'modules')
+        run: CFLAGS='-Werror' ./runtest-moduleapi --verbose --dump-logs ${{github.event.inputs.test_args}}
+      - name: sentinel tests
+        if: true && !contains(github.event.inputs.skiptests, 'sentinel')
+        run: ./runtest-sentinel ${{github.event.inputs.cluster_test_args}}
+      - name: cluster tests
+        if: true && !contains(github.event.inputs.skiptests, 'cluster')
+        run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}}
+      - name: unittest
+        if: true && !contains(github.event.inputs.skiptests, 'unittest')
+        run: ./src/valkey-unit-tests
+
   test-rpm-distros-jemalloc:
     if: |
       (github.event_name == 'workflow_dispatch' ||

.github/workflows/spell-check.yml

@@ -26,7 +26,7 @@ jobs:
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
 
       - name: Install typos
-        uses: taiki-e/install-action@cd5df4de2e75f3b819ba55f780f7bb8cd4a05a41 # v2.32.2
+        uses: taiki-e/install-action@fe9759bf4432218c779595708e80a1aadc85cedc # v2.46.10
         with:
           tool: typos
 

CMakeLists.txt

@@ -41,3 +41,4 @@ unset(BUILD_UNIT_TESTS CACHE)
 unset(BUILD_TEST_MODULES CACHE)
 unset(BUILD_EXAMPLE_MODULES CACHE)
 unset(USE_TLS CACHE)
+unset(DEBUG_FORCE_DEFRAG CACHE)

deps/CMakeLists.txt

@@ -1,4 +1,6 @@
-add_subdirectory(jemalloc)
+if (USE_JEMALLOC)
+    add_subdirectory(jemalloc)
+endif ()
 add_subdirectory(lua)
 
 # Set hiredis options. We need to disable the defaults set in the OPTION(..) we do this by setting them in the CACHE

src/CMakeLists.txt

@@ -22,6 +22,12 @@ if (VALKEY_RELEASE_BUILD)
     set_property(TARGET valkey-server PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
 endif ()
 
+if (DEBUG_FORCE_DEFRAG)
+    message(STATUS "Forcing Active Defrag run on valkey-server")
+    target_compile_definitions(valkey-server PRIVATE DEBUG_FORCE_DEFRAG)
+    target_compile_definitions(valkey-server PRIVATE HAVE_DEFRAG)
+endif ()
+
 if (BUILD_SANITIZER)
     # 'BUILD_SANITIZER' is defined in ValkeySetup module (based on user input)
     # If defined, the variables 'VALKEY_SANITAIZER_CFLAGS' and 'VALKEY_SANITAIZER_LDFLAGS'

src/Makefile

@@ -130,6 +130,11 @@ ifdef REDIS_LDFLAGS
     SERVER_LDFLAGS := $(REDIS_LDFLAGS)
 endif
 
+# Special case of forcing defrag to run even though we have no Jemlloc support
+ifeq ($(DEBUG_FORCE_DEFRAG), yes)
+	SERVER_CFLAGS +=-DHAVE_DEFRAG -DDEBUG_FORCE_DEFRAG
+endif
+
 FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(SERVER_CFLAGS)
 FINAL_LDFLAGS=$(LDFLAGS) $(OPT) $(SERVER_LDFLAGS) $(DEBUG)
 FINAL_LIBS=-lm

src/acl.c

@@ -297,11 +297,6 @@ int ACLListMatchSds(void *a, void *b) {
     return sdscmp(a, b) == 0;
 }
 
-/* Method to free list elements from ACL users password/patterns lists. */
-void ACLListFreeSds(void *item) {
-    sdsfree(item);
-}
-
 /* Method to duplicate list elements from ACL users password/patterns lists. */
 void *ACLListDupSds(void *item) {
     return sdsdup(item);
@@ -374,7 +369,7 @@ aclSelector *ACLCreateSelector(int flags) {
     listSetFreeMethod(selector->patterns, ACLListFreeKeyPattern);
     listSetDupMethod(selector->patterns, ACLListDupKeyPattern);
     listSetMatchMethod(selector->channels, ACLListMatchSds);
-    listSetFreeMethod(selector->channels, ACLListFreeSds);
+    listSetFreeMethod(selector->channels, sdsfreeVoid);
     listSetDupMethod(selector->channels, ACLListDupSds);
     memset(selector->allowed_commands, 0, sizeof(selector->allowed_commands));
 
@@ -445,7 +440,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
     u->passwords = listCreate();
     u->acl_string = NULL;
     listSetMatchMethod(u->passwords, ACLListMatchSds);
-    listSetFreeMethod(u->passwords, ACLListFreeSds);
+    listSetFreeMethod(u->passwords, sdsfreeVoid);
     listSetDupMethod(u->passwords, ACLListDupSds);
 
     u->selectors = listCreate();
@@ -489,6 +484,11 @@ void ACLFreeUser(user *u) {
     zfree(u);
 }
 
+/* Used for generic free functions. */
+static void ACLFreeUserVoid(void *u) {
+    ACLFreeUser(u);
+}
+
 /* When a user is deleted we need to cycle the active
  * connections in order to kill all the pending ones that
  * are authenticated with such user. */
@@ -2445,12 +2445,12 @@ sds ACLLoadFromFile(const char *filename) {
             c->user = new_user;
         }
 
-        if (user_channels) raxFreeWithCallback(user_channels, (void (*)(void *))listRelease);
-        raxFreeWithCallback(old_users, (void (*)(void *))ACLFreeUser);
+        if (user_channels) raxFreeWithCallback(user_channels, listReleaseVoid);
+        raxFreeWithCallback(old_users, ACLFreeUserVoid);
         sdsfree(errors);
         return NULL;
     } else {
-        raxFreeWithCallback(Users, (void (*)(void *))ACLFreeUser);
+        raxFreeWithCallback(Users, ACLFreeUserVoid);
         Users = old_users;
         errors =
             sdscat(errors, "WARNING: ACL errors detected, no change to the previously active ACL rules was performed");

src/adlist.c

@@ -77,6 +77,12 @@ void listRelease(list *list) {
     zfree(list);
 }
 
+/* Just like listRelease, but takes the list as a (void *).
+ * Useful as generic free callback. */
+void listReleaseVoid(void *l) {
+    listRelease((list *)l);
+}
+
 /* Add a new node to the list, to head, containing the specified 'value'
  * pointer as value.
  *

src/adlist.h

@@ -72,6 +72,7 @@ typedef struct list {
 /* Prototypes */
 list *listCreate(void);
 void listRelease(list *list);
+void listReleaseVoid(void *list);
 void listEmpty(list *list);
 list *listAddNodeHead(list *list, void *value);
 list *listAddNodeTail(list *list, void *value);

src/allocator_defrag.c

@@ -43,12 +43,10 @@
  * the other component to ensure both are using the same allocator configuration.
  */
 
-#include <stdio.h>
+#include "server.h"
 #include "serverassert.h"
 #include "allocator_defrag.h"
 
-#define UNUSED(x) (void)(x)
-
 #if defined(HAVE_DEFRAG) && defined(USE_JEMALLOC)
 
 #define STRINGIFY_(x) #x
@@ -402,8 +400,56 @@ int allocatorShouldDefrag(void *ptr) {
                               je_cb.bin_info[binind].nregs - SLAB_NFREE(out, 0));
 }
 
-#else
+/* Utility function to get the fragmentation ratio from jemalloc.
+ * It is critical to do that by comparing only heap maps that belong to
+ * jemalloc, and skip ones the jemalloc keeps as spare. Since we use this
+ * fragmentation ratio in order to decide if a defrag action should be taken
+ * or not, a false detection can cause the defragmenter to waste a lot of CPU
+ * without the possibility of getting any results. */
+float getAllocatorFragmentation(size_t *out_frag_bytes) {
+    size_t resident, active, allocated, frag_smallbins_bytes;
+    zmalloc_get_allocator_info(&allocated, &active, &resident, NULL, NULL);
+    frag_smallbins_bytes = allocatorDefragGetFragSmallbins();
+    /* Calculate the fragmentation ratio as the proportion of wasted memory in small
+     * bins (which are defraggable) relative to the total allocated memory (including large bins).
+     * This is because otherwise, if most of the memory usage is large bins, we may show high percentage,
+     * despite the fact it's not a lot of memory for the user. */
+    float frag_pct = (float)frag_smallbins_bytes / allocated * 100;
+    float rss_pct = ((float)resident / allocated) * 100 - 100;
+    size_t rss_bytes = resident - allocated;
+    if (out_frag_bytes) *out_frag_bytes = frag_smallbins_bytes;
+    serverLog(LL_DEBUG, "allocated=%zu, active=%zu, resident=%zu, frag=%.2f%% (%.2f%% rss), frag_bytes=%zu (%zu rss)",
+              allocated, active, resident, frag_pct, rss_pct, frag_smallbins_bytes, rss_bytes);
+    return frag_pct;
+}
 
+#elif defined(DEBUG_FORCE_DEFRAG)
+int allocatorDefragInit(void) {
+    return 0;
+}
+void allocatorDefragFree(void *ptr, size_t size) {
+    UNUSED(size);
+    zfree(ptr);
+}
+__attribute__((malloc)) void *allocatorDefragAlloc(size_t size) {
+    return zmalloc(size);
+    return NULL;
+}
+unsigned long allocatorDefragGetFragSmallbins(void) {
+    return 0;
+}
+
+int allocatorShouldDefrag(void *ptr) {
+    UNUSED(ptr);
+    return 1;
+}
+
+float getAllocatorFragmentation(size_t *out_frag_bytes) {
+    *out_frag_bytes = server.active_defrag_ignore_bytes + 1;
+    return server.active_defrag_threshold_upper;
+}
+
+#else
 int allocatorDefragInit(void) {
     return -1;
 }
@@ -423,4 +469,9 @@ int allocatorShouldDefrag(void *ptr) {
     UNUSED(ptr);
     return 0;
 }
+
+float getAllocatorFragmentation(size_t *out_frag_bytes) {
+    UNUSED(out_frag_bytes);
+    return 0;
+}
 #endif

src/allocator_defrag.h

@@ -5,10 +5,11 @@
 #include <jemalloc/jemalloc.h>
 /* We can enable the server defrag capabilities only if we are using Jemalloc
  * and the version that has the experimental.utilization namespace in mallctl . */
-#if defined(JEMALLOC_VERSION_MAJOR) &&                              \
-    (JEMALLOC_VERSION_MAJOR > 5 ||                                  \
-     (JEMALLOC_VERSION_MAJOR == 5 && JEMALLOC_VERSION_MINOR > 2) || \
-     (JEMALLOC_VERSION_MAJOR == 5 && JEMALLOC_VERSION_MINOR == 2 && JEMALLOC_VERSION_BUGFIX >= 1))
+#if (defined(JEMALLOC_VERSION_MAJOR) &&                                                                 \
+     (JEMALLOC_VERSION_MAJOR > 5 ||                                                                     \
+      (JEMALLOC_VERSION_MAJOR == 5 && JEMALLOC_VERSION_MINOR > 2) ||                                    \
+      (JEMALLOC_VERSION_MAJOR == 5 && JEMALLOC_VERSION_MINOR == 2 && JEMALLOC_VERSION_BUGFIX >= 1))) || \
+    defined(DEBUG_FORCE_DEFRAG)
 #define HAVE_DEFRAG
 #endif
 #endif
@@ -18,5 +19,6 @@ void allocatorDefragFree(void *ptr, size_t size);
 __attribute__((malloc)) void *allocatorDefragAlloc(size_t size);
 unsigned long allocatorDefragGetFragSmallbins(void);
 int allocatorShouldDefrag(void *ptr);
+float getAllocatorFragmentation(size_t *out_frag_bytes);
 
 #endif /* __ALLOCATOR_DEFRAG_H */