Commit Graph

12375 Commits

Author SHA1 Message Date
debing.sun f364dcca2d
Make RM_DefragRedisModuleDict API support incremental defragmentation for dict leaf (#13840)
After https://github.com/redis/redis/pull/13816, we make a new API to
defrag RedisModuleDict.
Currently, we only support incremental defragmentation of the dictionary
itself, but the defragmentation of values is still not incremental. If
the values are very large, it could lead to significant blocking.
Therefore, in this PR, we have added incremental defragmentation for the
values.

The main change is to the `RedisModuleDefragDictValueCallback`, we
modified the return value of this callback.
When the callback returns 1, we will save the `seekTo` as the key of the
current unfinished node, and the next time we enter, we will continue
defragmenting this node.
When the return value is 0, we will proceed to the next node.

## Test
Since each dictionary in the global dict originally contained only 10
strings, but now it has been changed to a nested dictionary, each
dictionary now has 10 sub-dictionaries, with each sub-dictionary
containing 10 strings, this has led to a corresponding reduction in the
defragmentation time obtained from other tests.
Therefore, the other tests have been modified to always wait for
defragmentation to be turned off before the test begins, then start it
after creating fragmentation, ensuring that they can always run for a
full defragmentation cycle.

---------

Co-authored-by: ephraimfeldblum <ephraim.feldblum@redis.com>
2025-03-04 17:19:41 +08:00
debing.sun 7939ba031d
Enable the callback to be NULL for RM_DefragRedisModuleDict() and reduce the system calls of RM_DefragShouldStop() (#13830)
1) Enable the callback to be NULL for RM_DefragRedisModuleDict()
    Because the dictionary may store only the key without the value.

2) Reduce the system calls of RM_DefragShouldStop()
The API checks the following thresholds before performing a time check:
over 512 defrag hits, or over 1024 defrag misses, and performs the time
judgment if any of these thresholds are reached.

3) Added defragmentation statistics for dictionary items to cover the
associated code for RM_DefragRedisModuleDict().

4) Removed `module_ctx` from `defragModuleCtx` struct, which can be
replaced by a temporary variable.

---------

Co-authored-by: oranagra <oran@redislabs.com>
2025-02-26 20:04:29 +08:00
Yuan Wang f1d6542b1a
Stabilize tcl test cases (#13829)
Recently encountered some errors as bellow,

HGETEX/HSETEX with PXAT/EXAT options, after getting ttl, we calculate
current time by `[clock seconds]` that may have a delay that causes
results greater than expected.

Dismiss memory test error, now we introduced rdb-channel replication,
the full synchronization might finish before the child process exits. So
we may fail if calling `bgsave` immediately after full sync.
2025-02-25 16:31:53 +08:00
Denis Nevmerzhitskii 33f03f6fc8
Fix wrong behavior of XREAD + after last entry of stream have been removed (#13632)
Close #13628

This PR changes behavior of special `+` id of XREAD command. Now it uses
`streamLastValidID` to find last entry instead of `last_id` field of
stream object.
This PR adds test for the issue.

**Notes**

Initial idea to update `last_id` while executing XDEL seems to be wrong.
`last_id` is used to strore last generated id and not id of last entry.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
Co-authored-by: guybe7 <guy.benoish@redislabs.com>
2025-02-25 13:40:24 +08:00
Filipe Oliveira (Redis) 985bf68f34
Reduce redundant key slot calculations on expiration checks (#13796)
On high-pipeline/fast commands use-cases, expireIfNeeded can take up to
3% cpu cycles. 

This PR introduces an optimization where key expiration checks leverage
key slots to improve efficiency.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
Co-authored-by: ShooterIT <wangyuancode@163.com>
2025-02-25 11:55:30 +08:00
Moti Cohen 0200e8ada6
Fix multiple issues with "INFO KEYSIZES" (#13825)
This commit addresses several issues related to the `INFO KEYSIZES` feature:
- HyperLogLog commands: `KEYSIZES` hooks were not properly set or tested.
- HFE lazy expiration: `KEYSIZES` hooks were not properly set or tested.
- Empty DB & SYNC flow: On `blocking_async=0` flow, global `keysizes`
  histogram were not reset (can reproduced using `DEBUG RELOAD`).
- Empty string handling: Fix histogram for strings of size 0. Not 
  relevant to other data-types.
2025-02-25 00:38:44 +02:00
Filipe Oliveira (Redis) 1848809f66
Optimize dictFind by leveraging key length functions to avoid redundant computations. (#13792)
This PR enhances dictFind by introducing support for key length
functions, allowing the use of keyCompareWithLen when available. This
avoids redundant key length computations, improving efficiency,
especially when the dictionary is rehashing or there are a significant
number of hash collisions.

Additionally, it maintains backward compatibility and optimizes key
lookups without altering existing behavior.

Performance improvement on 100% GETs use-case

benchmark command used
```
taskset -c 1-11 memtier_benchmark --ratio 0:1 --key-maximum 1000000 --key-minimum 1 -c 1 -t 5 --pipeline 100 --key-pattern P:P --test-time 30 --hide-histogram -d 1024 -S /tmp/1.socket  -x 3
```

In unstable dictFindByHash takes 29% (and sdslen within it takes 8.9%)
of CPU cycles for a high-pipeline 100% gets use-case.
After this change dictFindByHash takes 27.8% (and sdslen within it takes
7.7%)

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
Co-authored-by: Yuan Wang <wangyuancode@163.com>
2025-02-24 22:27:06 +08:00
Filipe Oliveira (Redis) d7a448f9ae
Avoid redundant calls to sdslen(c->querybuf) in processMultibulkBuffer (#13787)
Optimize processMultibulkBuffer by avoiding redundant calls to
sdslen(c->querybuf).
The cached length is updated only when querybuf is modified.
2025-02-24 20:06:14 +08:00
debing.sun 658424fc83
Revert "Update history for ban-list propagation (#13749)" (#13827)
As discussed in
https://github.com/redis/redis/pull/13749#issuecomment-2673612941.
After #10398 we should record only the arguments and output changes in
the command history, while placing all others in the redis-doc, so
revert #13749.
2025-02-24 17:40:25 +08:00
Filipe Oliveira (Redis) 3f06ddfb7b
Reuse lookupCommand data on consecutive same command calls on main thread (#13764)
We can see that on fast commands and fast pipeline use-cases,
lookupCommand() takes 1.9% to 3.4% of total cpu cyles (depending on
pipeline). In cases in which consecutives commands are the same we can
avoid the call to lookupCommand() completely without changing or adding
new fields to the client struct (we simply reuse the info already
avaiable in lastcmd). This change can represent an improvement of around
4.4% in QPS on the high pipeline use-cases.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
2025-02-24 12:33:14 +08:00
debing.sun ee933d9e2b
Fixed passing incorrect endtime value for module context (#13822)
1) Fix a bug that passing an incorrect endtime to module.
   This bug was found by @ShooterIT.
After #13814, all endtime will be monotonic time, and we should no
longer convert it to ustime relative.
Add assertions to prevent endtime from being much larger thatn the
current time.

2) Fix a race in test `Reduce defrag CPU usage when module data can't be
defragged`

---------

Co-authored-by: ShooterIT <wangyuancode@163.com>
2025-02-23 12:58:48 +08:00
debing.sun 032357ec0f
Add RM_DefragRedisModuleDict module API (#13816)
After #13815, we introduced incremental defragmentation for global data
for module.
Now we added a new module API `RM_DefragRedisModuleDict` to incremental
defrag `RedisModuleDict`.

This PR adds a new APIs and a new defrag callback:
```c
RedisModuleDict *RM_DefragRedisModuleDict(RedisModuleDefragCtx *ctx, RedisModuleDict *dict, RedisModuleDefragDictValueCallback valueCB, RedisModuleString **seekTo);

typedef void *(*RedisModuleDefragDictValueCallback)(RedisModuleDefragCtx *ctx, void *data, unsigned char *key, size_t keylen);
```

Usage:
```c
RedisModuleString *seekTo = NULL;
RedisModuleDict *dict = = RedisModule_CreateDict(ctx);
... populate the dict code ...
/* Defragment a dictionary completely */
do {
    RedisModuleDict *new = RedisModule_DefragRedisModuleDict(ctx, dict, defragGlobalDictValueCB, &seekTo);
    if (new != NULL) {
        dict = new;
    }
} while (seekTo);
```

---------

Co-authored-by: ShooterIT <wangyuancode@163.com>
Co-authored-by: oranagra <oran@redislabs.com>
2025-02-20 21:09:29 +08:00
debing.sun 695126ccce
Add support for incremental defragmentation of global module data (#13815)
## Description

Currently, when performing defragmentation on non-key data within the
module, we cannot process the defragmentation incrementally. This
limitation affects the efficiency and flexibility of defragmentation in
certain scenarios.
The primary goal of this PR is to introduce support for incremental
defragmentation of global module data.

## Interface Change
New module API `RegisterDefragFunc2`

This is a more advanced version of `RM_RegisterDefragFunc`, in that it
takes a new callbacks(`RegisterDefragFunc2`) that has a return value,
and can use RM_DefragShouldStop in and indicate that it should be called
again later, or is it done (returned 0).

## Note
The `RegisterDefragFunc` API remains available.

---------

Co-authored-by: ShooterIT <wangyuancode@163.com>
Co-authored-by: oranagra <oran@redislabs.com>
2025-02-20 00:28:16 +08:00
debing.sun 725cd268e6
Refactor of ActiveDefrag to reduce latencies (#13814)
This PR is based on: https://github.com/valkey-io/valkey/pull/1462

## Issue/Problems

Duty Cycle: Active Defrag has configuration values which determine the
intended percentage of CPU to be used based on a gradient of the
fragmentation percentage. However, Active Defrag performs its work on
the 100ms serverCron timer. It then computes a duty cycle and performs a
single long cycle. For example, if the intended CPU is computed to be
10%, Active Defrag will perform 10ms of work on this 100ms timer cron.

* This type of cycle introduces large latencies on the client (up to
25ms with default configurations)
* This mechanism is subject to starvation when slow commands delay the
serverCron

Maintainability: The current Active Defrag code is difficult to read &
maintain. Refactoring of the high level control mechanisms and functions
will allow us to more seamlessly adapt to new defragmentation needs.
Specific examples include:

* A single function (activeDefragCycle) includes the logic to
start/stop/modify the defragmentation as well as performing one "step"
of the defragmentation. This should be separated out, so that the actual
defrag activity can be performed on an independent timer (see duty cycle
above).
* The code is focused on kvstores, with other actions just thrown in at
the end (defragOtherGlobals). There's no mechanism to break this up to
reduce latencies.
* For the main dictionary (only), there is a mechanism to set aside
large keys to be processed in a later step. However this code creates a
separate list in each kvstore (main dict or not), bleeding/exposing
internal defrag logic. We only need 1 list - inside defrag. This logic
should be more contained for the main key store.
* The structure is not well suited towards other non-main-dictionary
items. For example, pub-sub and pub-sub-shard was added, but it's added
in such a way that in CMD mode, with multiple DBs, we will defrag
pub-sub repeatedly after each DB.

## Description of the feature

Primarily, this feature will split activeDefragCycle into 2 functions.

1. One function will be called from serverCron to determine if a defrag
cycle (a complete scan) needs to be started. It will also determine if
the CPU expenditure needs to be adjusted.
2. The 2nd function will be a timer proc dedicated to performing defrag.
This will be invoked independently from serverCron.

Once the functions are split, there is more control over the latency
created by the defrag process. A new configuration will be used to
determine the running time for the defrag timer proc. The default for
this will be 500us (one-half of the current minimum time). Then the
timer will be adjusted to achieve the desired CPU. As an example, 5% of
CPU will run the defrag process for 500us every 10ms. This is much
better than running for 5ms every 100ms.

The timer function will also adjust to compensate for starvation. If a
slow command delays the timer, the process will run proportionately
longer to ensure that the configured CPU is achieved. Given the presence
of slow commands, the proportional extra time is insignificant to
latency. This also addresses the overload case. At 100% CPU, if the
event loop slows, defrag will run proportionately longer to achieve the
configured CPU utilization.

Optionally, in low CPU situations, there would be little impact in
utilizing more than the configured CPU. We could optionally allow the
timer to pop more often (even with a 0ms delay) and the (tail) latency
impact would not change.

And we add a time limit for the defrag duty cycle to prevent excessive
latency. When latency is already high (indicated by a long time between
calls), we don't want to make it worse by running defrag for too long.

Addressing maintainability:

* The basic code structure can more clearly be organized around a
"cycle".
* Have clear begin/end functions and a set of "stages" to be executed.
* Rather than stages being limited to "kvstore" type data, a cycle
should be more flexible, incorporating the ability to incrementally
perform arbitrary work. This will likely be necessary in the future for
certain module types. It can be used today to address oddballs like
defragOtherGlobals.
* We reduced some of the globals, and reduce some of the coupling.
defrag_later should be removed from serverDb.
* Each stage should begin on a fresh cycle. So if there are
non-time-bounded operations like kvstoreDictLUTDefrag, these would be
less likely to introduce additional latency.


Signed-off-by: Jim Brunner
[brunnerj@amazon.com](mailto:brunnerj@amazon.com)
Signed-off-by: Madelyn Olson
[madelyneolson@gmail.com](mailto:madelyneolson@gmail.com)
Co-authored-by: Madelyn Olson
[madelyneolson@gmail.com](mailto:madelyneolson@gmail.com)

---------

Signed-off-by: Jim Brunner brunnerj@amazon.com
Signed-off-by: Madelyn Olson madelyneolson@gmail.com
Co-authored-by: Madelyn Olson madelyneolson@gmail.com
Co-authored-by: ShooterIT <wangyuancode@163.com>
2025-02-20 00:05:24 +08:00
guybe7 66df58f961
Do not send NL if replica client is already closed (#13813)
In case a replica connection was closed mid-RDB, we should not send a \n
to that replica, otherwise, it may reach the replica BEFORE it realizes
that the RDB transfer failed, causing it to treat the \n as if it was
read from the RDB stream
2025-02-19 15:04:28 +07:00
luozongle01 b045fe4e17
Fix overflow on 32-bit systems when calculating idle time for eviction (#13804)
the `dictGetSignedIntegerVal` function should be used here,
because in some cases (especially on 32-bit systems) long may
be 4 bytes, and the ttl time saved in expires is a unix timestamp
(millisecond value), which is more than 4 bytes. In this case, we may
not be able to get the correct idle time, which may cause eviction
disorder, in other words, keys that should be evicted later may be
evicted earlier.
2025-02-19 11:01:15 +08:00
Yunxiao Du c5f91abaf7
Fix syntax issue in comments of src/module.c (#13802)
closes https://github.com/redis/redis/issues/13797, just fix syntax
issue in comments instead of real code.
2025-02-19 10:58:14 +08:00
Ozan Tezcan 6c202f495c
Remove DENYOOM flag from hexpire command (#13800)
Remove DENYOOM flag from hexpire / hexpireat / hpexpire / hpexpireat
commands.

h(p)expire(at) commands may allocate some memory but it is not that big.
Similary, we don't have DENYOOM flag for EXPIRE command. This change
will align EXPIRE and HEXPIRE commands in this manner.
2025-02-16 20:07:29 +03:00
Ozan Tezcan e2608478b6
Add HGETDEL, HGETEX and HSETEX hash commands (#13798)
This PR adds three new hash commands: HGETDEL, HGETEX and HSETEX. These
commands enable user to do multiple operations in one step atomically
e.g. set a hash field and update its TTL with a single command.
Previously, it was only possible to do it by calling hset and hexpire
commands subsequently.

- **HGETDEL command**

  ```
  HGETDEL <key> FIELDS <numfields> field [field ...]
  ```
  
  **Description**  
  Get and delete the value of one or more fields of a given hash key
  
  **Reply**  
Array reply: list of the value associated with each field or nil if the
field doesn’t exist.

- **HGETEX command**

  ```
   HGETEX <key>  
[EX seconds | PX milliseconds | EXAT unix-time-seconds | PXAT
unix-time-milliseconds | PERSIST]
     FIELDS <numfields> field [field ...]
  ```

  **Description**
Get the value of one or more fields of a given hash key, and optionally
set their expiration

  **Options:**
  EX seconds: Set the specified expiration time, in seconds.
  PX milliseconds: Set the specified expiration time, in milliseconds.
EXAT timestamp-seconds: Set the specified Unix time at which the field
will expire, in seconds.
PXAT timestamp-milliseconds: Set the specified Unix time at which the
field will expire, in milliseconds.
  PERSIST: Remove the time to live associated with the field.

  **Reply** 
Array reply: list of the value associated with each field or nil if the
field doesn’t exist.

- **HSETEX command**

  ```
  HSETEX <key>
     [FNX | FXX]
[EX seconds | PX milliseconds | EXAT unix-time-seconds | PXAT
unix-time-milliseconds | KEEPTTL]
     FIELDS <numfields> field value [field value...]
  ```
  **Description**
Set the value of one or more fields of a given hash key, and optionally
set their expiration

  **Options:**
  FNX: Only set the fields if all do not already exist.
  FXX: Only set the fields if all already exist.

  EX seconds: Set the specified expiration time, in seconds.
  PX milliseconds: Set the specified expiration time, in milliseconds.
EXAT timestamp-seconds: Set the specified Unix time at which the field
will expire, in seconds.
PXAT timestamp-milliseconds: Set the specified Unix time at which the
field will expire, in milliseconds.
  KEEPTTL: Retain the time to live associated with the field.

  
Note: If no option is provided, any associated expiration time will be
discarded similar to how SET command behaves.

  **Reply**
  Integer reply: 0 if no fields were set
  Integer reply: 1 if all the fields were set
2025-02-14 17:13:35 +03:00
Ofir Luzon 57807cd338
Memory Usage command LIST accuracy fix (#13783)
MEMORY USAGE on a List samples quicklist entries, but does not account
to how many elements are in each sampled node. This can skew the
calculation when the sampled nodes are not balanced.
The fix calculate the average element size in the sampled nodes instead
of the average node size.
2025-02-14 09:18:47 +08:00
Yuan Wang 7f5f588232
AOF offset info (#13773)
### Background 
AOF is often used as an effective data recovery method, but now if we
have two AOFs from different nodes, it is hard to learn which one has
latest data. Generally, we determine whose data is more up-to-date by
reading the latest modification time of the AOF file, but because of
replication delay, even if both master and replica write to the AOF at
the same time, the data in the master is more up-to-date (there are
commands that didn't arrive at the replica yet, or a large number of
commands have accumulated on replica side ), so we may make wrong
decision.

### Solution
The replication offset always increments when AOF is enabled even if
there is no replica, we think replication offset is better method to
determine which one has more up-to-date data, whoever has a larger
offset will have newer data, so we add the start replication offset info
for AOF, as bellow.
```
file appendonly.aof.2.base.rdb seq 2 type b
file appendonly.aof.2.incr.aof seq 2 type i startoffset 224
```
And if we close gracefully the AOF file, not a crash, such as
`shutdown`, `kill signal 15` or `config set appendonly no`, we will add
the end replication offset, as bellow.
```
file appendonly.aof.2.base.rdb seq 2 type b
file appendonly.aof.2.incr.aof seq 2 type i startoffset 224 endoffset 532
```

#### Things to pay attention to
- For BASE AOF, we do not add `startoffset` and `endoffset` info, since
we could not know the start replication replication of data, and it is
useless to help us to determine which one has more up-to-date data.
- For AOFs from old version, we also don't add `startoffset` and
`endoffset` info, since we also don't know start replication replication
of them. If we add the start offset from 0, we might make the judgment
even less accurate. For example, if the master has just rewritten the
AOF, its INCR AOF will inevitably be very small. However, if the replica
has not rewritten AOF for a long time, its INCR AOF might be much
larger. By applying the following method, we might make incorrect
decisions, so we still just check timestamp instead of adding offset
info
- If the last INCR AOF has `startoffset` or `endoffset`, we need to
restore `server.master_repl_offset` according to them to avoid the
rollback of the `startoffset` of next INCR AOF. If it has `endoffset`,
we just use this value as `server.master_repl_offset`, and a very
important thing is to remove this information from the manifest file to
avoid the next time we load the manifest file with wrong `endoffset`. If
it only has `startoffset`, we calculate `server.master_repl_offset` by
the `startoffset` plus the file size.

### How to determine which one has more up-to-date data
If AOF has a larger replication offset, it will have more up-to-date
data. The following is how to get AOF offset:

Read the AOF manifest file to obtain information about **the last INCR
AOF**
1. If the last INCR AOF has `endoffset` field, we can directly use the
`endoffset` to present the replication offset of AOF
2. If there is no `endoffset`(such as redis crashes abnormally), but
there is `startoffset` filed of the last INCR AOF, we can get the
replication offset of AOF by `startoffset` plus the file size
3. Finally, if the AOF doesn’t have both `startoffset` and `endoffset`,
maybe from old version, and new version redis has not rewritten AOF yet,
we still need to check the modification timestamp of the last INCR AOF

### TODO
Fix ping causing inconsistency between AOF size and replication
offset in the future PR. Because we increment the replication offset
when sending PING/REPLCONF to the replica but do not write data to the
AOF file, this might cause the starting offset of the AOF file plus its
size to be inconsistent with the actual replication offset.
2025-02-13 17:31:40 +08:00
Yuan Wang 662cb2fe75
Don't send unnecessary PING to replicas (#13790)
The reason why master sends PING is to keep the connection with replica
active, so master need not send PING to replicas if already sent
replication stream in the past `repl_ping_slave_period` time.

Now master only sends PINGs and increases `master_repl_offset` if there
is no traffic, so this PR also can reduce the impact of issue in
https://github.com/redis/redis/pull/13773, of course, does not resolve
it completely.
> Fix ping causing inconsistency between AOF size and replication offset
in the future PR. Because we increment the replication offset when
sending PING/REPLCONF to the replica but do not write data to the AOF
file, this might cause the starting offset of the AOF file plus its size
to be inconsistent with the actual replication offset.
2025-02-13 10:52:19 +08:00
Yuan Wang 87124a38b6
Fix wrongly updating fsynced_reploff_pending when appendfsync=everysecond (#13793)
```
if (server.aof_fsync == AOF_FSYNC_EVERYSEC &&
    server.aof_last_incr_fsync_offset != server.aof_last_incr_size &&
    server.mstime - server.aof_last_fsync >= 1000 &&
    !(sync_in_progress = aofFsyncInProgress())) {
    goto try_fsync;
```
In https://github.com/redis/redis/pull/12622, when when
appendfsync=everysecond, if redis has written some data to AOF but not
`fsync`, and less than 1 second has passed since the last `fsync `,
redis will won't fsync AOF, but we will update `
fsynced_reploff_pending`, so it cause the `WAITAOF` to return
prematurely.

this bug is introduced in https://github.com/redis/redis/pull/12622,
from 7.4

The bug fix
1bd6688bca
is just as follows:
```diff
diff --git a/src/aof.c b/src/aof.c
index 8ccd8d8f8..521b30449 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1096,8 +1096,11 @@ void flushAppendOnlyFile(int force) {
              * in which case master_repl_offset will increase but fsynced_reploff_pending won't be updated
              * (because there's no reason, from the AOF POV, to call fsync) and then WAITAOF may wait on
              * the higher offset (which contains data that was only propagated to replicas, and not to AOF) */
-            if (!sync_in_progress && server.aof_fsync != AOF_FSYNC_NO)
+            if (server.aof_last_incr_fsync_offset == server.aof_last_incr_size &&
+                !(sync_in_progress = aofFsyncInProgress()))
+            {
                 atomicSet(server.fsynced_reploff_pending, server.master_repl_offset);
+            }
             return;
```
Additionally, we slightly refactored fsync AOF to make it simpler, as
584f008d1c
2025-02-13 10:48:29 +08:00
Yves LeBras 1583d60cd6
Missing --memkeys and --keystats for some options in redis-cli help text (#13794)
Help text modified for -i, --count, --pattern.
2025-02-13 08:42:38 +08:00
YaacovHazan 1cd622bdca
Add an API to load default configuration values (#13788)
Currently we have RedisModule_LoadConfigs which the module is expected
to call during OnLoad which sets the configuration values from the
config queue or it sets the default value.

The problem is that the module might still want to support loading
values from the command line. If we want to give precedence to the
config file values then it means the module needs to set the values
before calling the Load Config function.

The problem is that then the API overrides the variables which were set
from the module command line with default values.

The new API should solve that in the following way.

1.Module registers its configuration parameters with redis 2.Module
calls RedisModule_LoadDefaultConfigs which loads the default values for
all the registered configuration parameters of the module 3.Module sets
the variables internally using the values it got from the command line
4.Module calls RedisModule_LoadConfigs which will set the values based
on the redis configuration file.

This allows for the default values to be set, for the module to override
them and for redis to override what the module wrote. In short it
determines a logical flow and ordering of where the values for the
parameters should come from.

The change done by all these previous commits:
d9134f8f9
7a40fd630
b9361ad5f
83c034855
f164012c1
98be450f1
0f6e3a827
de4e92ac3
49455c43a
c2694fb69
c88f9fe26
855ec46a6
f7353db7e
294492dbf
192799539
a8850a8d3
f35ad8231
a03477349
fd5c32588

Co-authored-by: YaacovHazan <yaacov.hazan@redislabs.com>
2025-02-06 13:38:07 +02:00
kei-nan d9134f8f95 Update tests/modules/moduleconfigs.c
missing else clause

Co-authored-by: debing.sun <debing.sun@redis.com>
2025-02-06 13:16:33 +02:00
jonathan keinan 7a40fd630d * fix comments 2025-02-06 13:16:33 +02:00
jonathan keinan b9361ad5fe * only use new api if override-default was provided as an argument 2025-02-06 13:16:33 +02:00
kei-nan 83c0348553 Apply suggestions from code review
* apply comment suggestions

Co-authored-by: Oran Agra <oran@redislabs.com>
2025-02-06 13:16:33 +02:00
kei-nan f164012c19 Update tests/unit/moduleapi/moduleconfigs.tcl
Co-authored-by: nafraf <nafraf@users.noreply.github.com>
2025-02-06 13:16:33 +02:00
jonathan keinan 98be450f1d * fix typo 2025-02-06 13:16:33 +02:00
jonathan keinan 0f6e3a8273 * improve function documentation 2025-02-06 13:16:33 +02:00
jonathan keinan de4e92ac39 * addressing code review comments 2025-02-06 13:16:33 +02:00
jonathan keinan 49455c43ae * change foo to goo so test will be correct and pass 2025-02-06 13:16:33 +02:00
jonathan keinan c2694fb696 * change config value in test to be different than overwritten value 2025-02-06 13:16:33 +02:00
jonathan keinan c88f9fe26f * update comment 2025-02-06 13:16:33 +02:00
jonathan keinan 855ec46a6a * rename MODULE_ONLOAD_CONFIG to MODULE_NON_DEFAULT_CONFIG 2025-02-06 13:16:33 +02:00
jonathan keinan f7353db7eb * fix test
* cleanup strval2 on if an error during the OnLoad was encountered.
2025-02-06 13:16:33 +02:00
jonathan keinan 294492dbf2 * fix tests
* add some logging to test module
2025-02-06 13:16:33 +02:00
jonathan keinan 192799539f * register `LoadDefaultConfigs` 2025-02-06 13:16:33 +02:00
jonathan keinan a8850a8d30 * remove unused variable 2025-02-06 13:16:33 +02:00
jonathan keinan f35ad82314 * add missing newline 2025-02-06 13:16:33 +02:00
jonathan keinan a034773497 * remove redundant module config variable 2025-02-06 13:16:33 +02:00
jonathan keinan fd5c325886 * initial commit 2025-02-06 13:16:33 +02:00
Alexander Gorbulya 17eb33e0c3
Fix typo in repl-ping-replica-period comment in redis.conf (#13782)
The comment for the `repl-ping-replica-period` option in `redis.conf`
mistakenly refers to `repl_ping_replica_period` (with underscores).
This PR corrects it to use the proper format with dashes, as per the
actual configuration option.
2025-02-06 10:14:56 +08:00
YaacovHazan 0aeb86d78d Revert "Improve GETRANGE command behavior (#12272)"
Although the commit #6ceadfb58 improves GETRANGE command behavior,
we can't accept it as we should avoid breaking changes for non-critical bug fixes.

This reverts commit 6ceadfb580.
2025-02-05 20:49:42 +02:00
YaacovHazan 8afb72a326 Revert "improve performance for scan command when matching data type (#12395)"
Although the commit #7f0a7f0a6 improves the performance of the SCAN command,
we can't accept it as we should avoid breaking changes for non-critical bug fixes.

This reverts commit 7f0a7f0a69.
2025-02-05 20:49:42 +02:00
Raz Monsonego 04589f90d7
Add internal connection and command mechanism (#13740)
# PR: Add Mechanism for Internal Commands and Connections in Redis

This PR introduces a mechanism to handle **internal commands and
connections** in Redis. It includes enhancements for command
registration, internal authentication, and observability.

## Key Features

1. **Internal Command Flag**:
   - Introduced a new **module command registration flag**: `internal`.
- Commands marked with `internal` can only be executed by **internal
connections**, AOF loading flows, and master-replica connections.
- For any other connection, these commands will appear as non-existent.

2. **Support for internal authentication added to `AUTH`**:
- Used by depicting the special username `internal connection` with the
right internal password, i.e.,: `AUTH "internal connection"
<internal_secret>`.
- No user-defined ACL username can have this name, since spaces are not
aloud in the ACL parser.
   - Allows connections to authenticate as **internal connections**.
- Authenticated internal connections can execute internal commands
successfully.

4. **Module API for Internal Secret**:
- Added the `RedisModule_GetInternalSecret()` API, that exposes the
internal secret that should be used as the password for the new `AUTH
"internal connection" <password>` command.
- This API enables the modules to authenticate against other shards as
local connections.

## Notes on Behavior

- **ACL validation**:
- Commands dispatched by internal connections bypass ACL validation, to
give the caller full access regardless of the user with which it is
connected.

- **Command Visibility**:
- Internal commands **do not appear** in `COMMAND <subcommand>` and
`MONITOR` for non-internal connections.
- Internal commands **are logged** in the slow log, latency report and
commands' statistics to maintain observability.

- **`RM_Call()` Updates**:
  - **Non-internal connections**:
- Cannot execute internal commands when the command is sent with the `C`
flag (otherwise can).
- Internal connections bypass ACL validations (i.e., run as the
unrestricted user).

- **Internal commands' success**:
- Internal commands succeed upon being sent from either an internal
connection (i.e., authenticated via the new `AUTH "internal connection"
<internal_secret>` API), an AOF loading process, or from a master via
the replication link.
Any other connections that attempt to execute an internal command fail
with the `unknown command` error message raised.

- **`CLIENT LIST` flags**:
  - Added the `I` flag, to indicate that the connection is internal.

- **Lua Scripts**:
   - Prevented internal commands from being executed via Lua scripts.

---------

Co-authored-by: Meir Shpilraien <meir@redis.com>
2025-02-05 11:48:08 +02:00
Ozan Tezcan 09f8a2f374
Start AOFRW before streaming repl buffer during fullsync (#13758)
During fullsync, before loading RDB on the replica, we stop aof child to
prevent copy-on-write disaster.
Once rdb is loaded, aof is started again and it will trigger aof
rewrite. With https://github.com/redis/redis/pull/13732 , for rdbchannel
replication, this behavior was changed. Currently, we start aof after
replication buffer is streamed to db. This PR changes it back to start
aof just after rdb is loaded (before repl buffer is streamed)

Both approaches may have pros and cons. If we start aof before streaming
repl buffers, we may still face with copy-on-write issues as repl
buffers potentially include large amount of changes. If we wait until
replication buffer drained, it means we are delaying starting aof
persistence.

Additional changes are introduced as part of this PR:

- Interface change:
Added `mem_replica_full_sync_buffer` field to the `INFO MEMORY` command
reply. During full sync, it shows total memory consumed by accumulated
replication stream buffer on replica. Added same metric to `MEMORY
STATS` command reply as `replica.fullsync.buffer` field.
  
  
- Fixes: 
- Count repl stream buffer size of replica as part of 'memory overhead'
calculation for fields in "INFO MEMORY" and "MEMORY STATS" outputs.
Before this PR, repl buffer was not counted as part of memory overhead
calculation, causing misreports for fields like `used_memory_overhead`
and `used_memory_dataset` in "INFO STATS" and for `overhead.total` field
in "MEMORY STATS" command reply.
- Dismiss replication stream buffers memory of replica in the fork to
reduce COW impact during a fork.
- Fixed a few time sensitive flaky tests, deleted a noop statement,
fixed some comments and fail messages in rdbchannel tests.
2025-02-04 21:40:18 +03:00
Meir Shpilraien (Spielrein) 870b6bd487
Added a shared secret over Redis cluster. (#13763)
The PR introduces a new shared secret that is shared over all the nodes
on the Redis cluster. The main idea is to leverage the cluster bus to
share a secret between all the nodes such that later the nodes will be
able to authenticate using this secret and send internal commands to
each other (see #13740 for more information about internal commands).

The way the shared secret is chosen is the following:
1. Each node, when start, randomly generate its own internal secret.
2. Each node share its internal secret over the cluster ping messages.
3. If a node gets a ping message with secret smaller then his current
secret, it embrace it.
4. Eventually all nodes should embrace the minimal secret

The converges of the secret is as good as the topology converges.

To extend the ping messages to contain the secret, we leverage the
extension mechanism. Nodes that runs an older Redis version will just
ignore those extensions.

Specific tests were added to verify that eventually all nodes see the
secrets. In addition, a verification was added to the test infra to
verify the secret on `cluster_config_consistent` and to
`assert_cluster_state`.
2025-02-03 09:54:37 +02:00