8.4 commits backport

modules/redisearch/Makefile

@@ -3,5 +3,10 @@ MODULE_VERSION = v8.4.10
 MODULE_REPO = https://github.com/redisearch/redisearch
 TARGET_MODULE = $(SRC_DIR)/bin/$(FULL_VARIANT)/search-community/redisearch.so
 
+  # Set INLINE_LSE_ATOMICS=1 for perf improvement on common ARM CPUs (i.e. Graviton2/3/4); no effect on x86 or macOS.
+  # Default 0 keeps the binary runnable on pre-Armv8.1-a cores (Cortex-A72, Graviton1, RPi4) that would otherwise SIGILL at module load.
+INLINE_LSE_ATOMICS ?= 0
+export INLINE_LSE_ATOMICS
+
 include ../common.mk
 

src/config.c

@@ -2401,13 +2401,7 @@ static int isValidAnnouncedNodename(char *val,const char **err) {
     return 1;
 }
 
-static int isValidAnnouncedHostname(char *val, const char **err) {
-    if (strlen(val) >= NET_HOST_STR_LEN) {
-        *err = "Hostnames must be less than "
-            STRINGIFY(NET_HOST_STR_LEN) " characters";
-        return 0;
-    }
-
+static int isValidHostnameChars(char *val, const char **err) {
     int i = 0;
     char c;
     while ((c = val[i])) {
@@ -2425,6 +2419,15 @@ static int isValidAnnouncedHostname(char *val, const char **err) {
     return 1;
 }
 
+static int isValidAnnouncedHostname(char *val, const char **err) {
+    if (strlen(val) >= NET_HOST_STR_LEN) {
+        *err = "Hostnames must be less than "
+            STRINGIFY(NET_HOST_STR_LEN) " characters";
+        return 0;
+    }
+    return isValidHostnameChars(val, err);
+}
+
 /* Validation function for cluster-announce-ip.
  * Ensures the IP address is valid and rejects control characters. */
 static int isValidClusterAnnounceIp(char *val, const char **err) {
@@ -2434,12 +2437,19 @@ static int isValidClusterAnnounceIp(char *val, const char **err) {
         return 1;
     }
 
-    if (inet_pton(AF_INET, val, buf) != 1 &&
-        inet_pton(AF_INET6, val, buf) != 1) {
-        *err = "Cluster announce IP must be a valid IPv4 or IPv6 address";
+    /* Accept valid IPv4 or IPv6 */
+    if (inet_pton(AF_INET, val, buf) == 1 || inet_pton(AF_INET6, val, buf) == 1) {
+        return 1;
+    }
+    /* Also accept valid hostnames, but limited to NET_IP_STR_LEN since
+     * cluster_announce_ip is stored in a NET_IP_STR_LEN buffer */
+    if (strlen(val) >= NET_IP_STR_LEN) {
+        *err = "Hostnames for cluster-announce-ip must be less than "
+               STRINGIFY(NET_IP_STR_LEN) " characters";
         return 0;
     }
-    return 1;
+    /* Also accept valid hostnames */
+    return isValidHostnameChars(val, err);
 }
 
 /* Validate specified string is a valid proc-title-template */

src/db.c

@@ -1503,9 +1503,22 @@ void scanCallback(void *privdata, const dictEntry *de, dictEntryLink plink) {
     /* o and typename can not have values at the same time. */
     serverAssert(!((data->type != LLONG_MAX) && o));
 
+    kvobj *kv = NULL;
     if (!o) { /* If scanning keyspace */
-        kvobj *kv = dictGetKV(de);
-
+        kv = dictGetKV(de);
+        keyStr = kvobjGetKey(kv);
+    } else {
+        keyStr = dictGetKey(de);
+    }
+
+    /* Filter element if it does not match the pattern. */
+    if (data->pattern) {
+        if (!stringmatchlen(data->pattern, sdslen(data->pattern), keyStr, data->strlen(keyStr), 0)) {
+            return;
+        }
+    }
+
+    if (!o) {
         /* Expiration check first - only for database keyspace scanning.
          * Use kv obj to avoid robj creation. */
         if (expireIfNeeded(data->db, NULL, kv, 0) != KEY_VALID)
@@ -1520,17 +1533,6 @@ void scanCallback(void *privdata, const dictEntry *de, dictEntryLink plink) {
             if (!objectTypeCompare(kv, data->type))
                 return;
         }
-
-        keyStr = kvobjGetKey(kv);
-    } else {
-        keyStr = dictGetKey(de);
-    }
-
-    /* Filter element if it does not match the pattern. */
-    if (data->pattern) {
-        if (!stringmatchlen(data->pattern, sdslen(data->pattern), keyStr, data->strlen(keyStr), 0)) {
-            return;
-        }
     }
 
     if (o == NULL) {
@@ -3021,7 +3023,7 @@ int getKeysUsingKeySpecs(struct redisCommand *cmd, robj **argv, int argc, int se
             }
 
             first += spec->fk.keynum.firstkey;
-            last = first + (long)numkeys-1;
+            last = first + ((long)numkeys - 1) * step;
         } else {
             /* unknown spec */
             goto invalid_spec;

src/memory_prefetch.c

@@ -161,6 +161,11 @@ static void initBatchInfo(dict **dicts, GetValueDataFunc func) {
             info->state = PREFETCH_DONE;
             continue;
         }
+
+        /* We skip prefetch during loading, so ht_table[0] should never be NULL
+         * when dictSize() > 0 (which only happens mid-dictEmpty via _dictReset). */
+        serverAssert(batch->current_dicts[i]->ht_table[0]);
+
         info->ht_idx = HT_IDX_INVALID;
         info->current_entry = NULL;
         info->current_kv = NULL;
@@ -327,7 +332,7 @@ int determinePrefetchCount(int len) {
  * 2. Prefetch the keys and values for all commands in the current batch from
  *    the main dictionaries. */
 void prefetchCommands(void) {
-    if (!batch) return;
+    if (!batch || server.loading) return;
 
     /* Prefetch argv's for all clients */
     for (size_t i = 0; i < batch->client_count; i++) {

src/module.c

@@ -13311,7 +13311,8 @@ int setModuleStringConfig(ModuleConfig *config, sds strval, const char **err) {
 
 int setModuleEnumConfig(ModuleConfig *config, int val, const char **err) {
     RedisModuleString *error = NULL;
-    int return_code = config->set_fn.set_enum(config->name, val, config->privdata, &error);
+    char *rname = getRegisteredConfigName(config);
+    int return_code = config->set_fn.set_enum(rname, val, config->privdata, &error);
     propagateErrorString(error, err);
     return return_code == REDISMODULE_OK ? 1 : 0;
 }

src/multi.c

@@ -179,6 +179,9 @@ void execCommand(client *c) {
 
     c->all_argv_len_sum = c->mstate.argv_len_sums;
 
+    /* Skip ACL check for the AOF client while server loading. */
+    int skip_acl_check = server.loading && c->id == CLIENT_ID_AOF;
+
     addReplyArrayLen(c,c->mstate.count);
     for (j = 0; j < c->mstate.count; j++) {
         c->argc = c->mstate.commands[j]->argc;
@@ -189,7 +192,10 @@ void execCommand(client *c) {
         /* ACL permissions are also checked at the time of execution in case
          * they were changed after the commands were queued. */
         int acl_errpos;
-        int acl_retval = ACLCheckAllPerm(c,&acl_errpos);
+        int acl_retval = ACL_OK;
+        if (!skip_acl_check) {
+            acl_retval = ACLCheckAllPerm(c,&acl_errpos);
+        }
         if (acl_retval != ACL_OK) {
             char *reason;
             switch (acl_retval) {
@@ -498,6 +504,7 @@ size_t multiStateMemOverhead(client *c) {
     /* Add watched keys overhead, Note: this doesn't take into account the watched keys themselves, because they aren't managed per-client. */
     mem += listLength(c->watched_keys) * (sizeof(listNode) + sizeof(watchedKey));
     /* Reserved memory for queued multi commands. */
-    mem += c->mstate.alloc_count * sizeof(pendingCommand);
+    mem += c->mstate.alloc_count * sizeof(pendingCommand*);
+    mem += c->mstate.count * sizeof(pendingCommand);
     return mem;
 }

src/server.c

@@ -4826,6 +4826,7 @@ int finishShutdown(void) {
          * The temp rdb file fd may won't be closed when redis exits quickly,
          * but OS will close this fd when process exits. */
         rdbRemoveTempFile(server.child_pid, 0);
+        resetChildState();
     }
 
     /* Kill module child if there is one. */

src/t_stream.c

@@ -702,7 +702,9 @@ typedef struct {
     int trim_strategy_arg_idx; /* Index of the count in MAXLEN/MINID, for rewriting. */
     int delete_strategy; /* DELETE_STRATEGY_* */
     int approx_trim; /* If 1 only delete whole radix tree nodes, so
-                      * the trim argument is not applied verbatim. */
+                      * the trim argument is not applied verbatim.
+                      * Note: This flag is ignored when delete_strategy is non-KEEPREF.
+                      * Individual entries may still be processed for consumer groups. */
     long long limit; /* Maximum amount of entries to trim. If 0, no limitation
                       * on the amount of trimming work is enforced. */
     /* TRIM_STRATEGY_MAXLEN options */
@@ -810,8 +812,11 @@ int64_t streamTrim(stream *s, streamAddTrimArgs *args) {
         }
 
         /* If we cannot remove a whole element, and approx is true,
-         * stop here. */
-        if (approx) break;
+         * stop here. However, for non-KEEPREF strategies, if the node was
+         * eligible for removal but we couldn't remove it (because we need
+         * to check consumer group references), we should continue to process
+         * entries within this node. */
+        if (approx && delete_strategy == DELETE_STRATEGY_KEEPREF) break;
 
         /* Now we have to trim entries from within 'lp' */
         size_t oldsize = lpBytes(lp);
@@ -883,12 +888,12 @@ int64_t streamTrim(stream *s, streamAddTrimArgs *args) {
 
                 if (can_delete) {
                     /* Mark the entry as deleted. */
-                    intptr_t delta = p - lp;
+                    intptr_t delta = p ? (p - lp) : 0; /* p may be NULL if this was the last entry */
                     flags |= STREAM_ITEM_FLAG_DELETED;
                     lp = lpReplaceInteger(lp, &pcopy, flags);
                     deleted_from_lp++;
                     s->length--;
-                    p = lp + delta;
+                    if (p) p = lp + delta;
                 }
             }
         }
@@ -3130,6 +3135,7 @@ static void streamFreeCG(stream *s, streamCG *cg) {
 
 /* Destroy a consumer group and clean up all associated references. */
 void streamDestroyCG(stream *s, streamCG *cg) {
+    /* Remove all references from the cgroups_ref. */
     raxIterator it;
     raxStart(&it, cg->pel);
     raxSeek(&it, "^", NULL, 0);
@@ -3139,6 +3145,12 @@ void streamDestroyCG(stream *s, streamCG *cg) {
     }
     raxStop(&it);
 
+    /* If we're destroying the group with the minimum last_id, the cached
+     * minimum is no longer valid and needs to be recalculated from the
+     * remaining groups. */
+    if (s->min_cgroup_last_id_valid && streamCompareID(&s->min_cgroup_last_id, &cg->last_id) == 0)
+        s->min_cgroup_last_id_valid = 0;
+
     streamFreeCG(s, cg);
 }
 

tests/integration/aof.tcl

@@ -374,6 +374,35 @@ tags {"aof external:skip"} {
         }
     }
 
+    test {skip EXEC ACL check during AOF load} {
+        set user_acl "default on nopass ~* &* +@read -@write +multi +exec +select +ping"
+
+        create_aof_manifest $aof_dirpath $aof_manifest_file {
+            append_to_manifest "file appendonly.aof.1.incr.aof seq 1 type i\n"
+        }
+
+        create_aof $aof_dirpath $aof_file {
+            append_to_aof [formatCommand set beforetx beforetx]
+            append_to_aof [formatCommand multi]
+            append_to_aof [formatCommand set tx1 tx1]
+            append_to_aof [formatCommand set tx2 tx2]
+            append_to_aof [formatCommand exec]
+            append_to_aof [formatCommand set aftertx aftertx]
+        }
+
+        start_server_aof [list dir $server_path user $user_acl] {
+            set c [redis [srv host] [srv port] 0 $::tls]
+            wait_done_loading $c
+            assert_equal {beforetx} [$c get beforetx]
+            assert_equal {aftertx}  [$c get aftertx]
+            assert_equal {tx1} [$c get tx1]
+            assert_equal {tx2} [$c get tx2]
+
+            catch {$c set newkey value} e
+            assert_match {*NOPERM*set*} $e
+        }
+    }
+
     # redis could load AOF which has timestamp annotations inside
     create_aof $aof_dirpath $aof_file {
         append_to_aof "#TS:1628217470\r\n"