- 08 Nov, 2022 1 commit
-
-
Ozan Tezcan authored
Add needs:save tag for the test introduced by #11376
-
- 07 Nov, 2022 1 commit
-
-
Oran Agra authored
Till now Redis attempted to avoid using jemalloc on ARM, but didn't do that properly (missing armv8l and aarch64), so in fact we did you jemalloc on these without a problem. Side notes: Some ARM platforms, which share instruction set and can share binaries (docker images), may have different page size, and apparently jemalloc uses the page size of the build machine as the maximum page size to be supported by the build. see https://github.com/redis-stack/redis-stack/issues/187 To work around that, when building for ARM, one can change the maximum page size to 64k (or greater if present on the build machine) In recent versions of jemalloc, this should not have any severe side effects (like VM map fragmentation), see: https://github.com/jemalloc/jemalloc/issues/467 https://github.com/redis/redis/pull/11170#issuecomment-1236265230 To do that, one can use: ``` JEMALLOC_CONFIGURE_OPTS="--with-lg-page=16" make ``` Besides that, this PR fixes a messy makefile condition that was created here: f30b18f4
-
- 06 Nov, 2022 2 commits
-
-
yancz2000 authored
-
Hanif Ariffin authored
Since they're created with makeObjectShared, then incrRefCount on them is a NOP. Signed-off-by:
Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
-
- 04 Nov, 2022 2 commits
-
-
Binbin authored
Our FreeBSD daily has been failing recently: ``` Config file: freebsd-13.1.conf cd: /Users/runner/work/redis/redis: No such file or directory gmake: *** No targets specified and no makefile found. Stop. ``` Upgrade vmactions/freebsd-vm to the latest version (0.3.0) can work. I've tested it, but don't know why, but first let's fix it.
-
Binbin authored
Introduce socket `shutdown()` into connection type, and use it on normal socket if a fork is active. This allows us to close client connections when there are child processes sharing the file descriptors. Fixes #10077. The reason is that since the `fork()` child is holding the file descriptors, the `close` in `unlinkClient -> connClose` isn't sufficient. The client will not realize that the connection is disconnected until the child process ends. Let's try to be conservative and only use shutdown when the fork is active.
-
- 03 Nov, 2022 2 commits
-
-
Madelyn Olson authored
Retain ACL categories used to generate ACL for displaying them later
-
Binbin authored
Today we don't place any specific restrictions on module command names. This can cause ambiguous scenarios. For example, someone might name a command like "module|feature" which would be incorrectly parsed by the ACL system as a subcommand. In this PR, we will block some chars that we know can mess things up. Specifically ones that can appear ok at first and cause problems in some cases (we rather surface the issue right away). There are these characters: * ` ` (space) - issues with old inline protocol. * `\r`, `\n` (newline) - can mess up the protocol on acl error replies. * `|` - sub-commands. * `@` - ACL categories * `=`, `,` - info and client list fields. note that we decided to leave `:` out as it's handled by `getSafeInfoString` and is more likely to already been used by existing modules.
-
- 02 Nov, 2022 4 commits
-
-
Wen Hui authored
Resolve an edge case where the ID of a stream is updated retroactively to an ID lower than the already set max_deleted_entry_id. Currently, if we have command as below: **xsetid mystream 1-1 MAXDELETEDID 1-2** Then we will get the following error: **(error) ERR The ID specified in XSETID is smaller than the provided max_deleted_entry_id** Becuase the provided MAXDELETEDID 1-2 is greated than input last-id: 1-1 Then we could assume there is a similar situation: step 1: we add three items in the mystream **127.0.0.1:6381> xadd mystream 1-1 a 1 "1-1" 127.0.0.1:6381> xadd mystream 1-2 b 2 "1-2" 127.0.0.1:6381> xadd mystream 1-3 c 3 "1-3"** step 2: we could check the mystream infomation as below: **127.0.0.1:6381> xinfo stream mystream 1) "length" 2) (integer) 3 7) "last-generated-id" 8) "1-3" 9) "max-deleted-entry-id" 10) "0-0" step 3: we delete the item id 1-2 and 1-3 as below: **127.0.0.1:6381> xdel mystream 1-2 (integer) 1 127.0.0.1:6381> xdel mystream 1-3 (integer) 1** step 4: we check the mystream information: 127.0.0.1:6381> xinfo stream mystream 1) "length" 2) (integer) 1 7) "last-generated-id" 8) "1-3" 9) "max-deleted-entry-id" 10) "1-3" we could notice that the **max-deleted-entry-id update to 1-3**, so right now, if we just run: **xsetid mystream 1-2** the above command has the same effect with **xsetid mystream 1-2 MAXDELETEDID 1-3** So we should return an error to the client that **(error) ERR The ID specified in XSETID is smaller than current max_deleted_entry_id**
-
Wen Hui authored
According to the source code, the commands can be executed with only key name, and no GET/SET/INCR operation arguments. change the docs to reflect that by marking these arguments as optional. also add tests.
-
Binbin authored
* Print IP and port on cluster bus message sanity check Add a print statement to indicate which IP/port is sending the error messages. That way we can at least check to see if it is a node in the cluster or some other nefarious nodes. It is proposed in #11339. Unrelated changes: the return check for connAddrPeerName should be -1 instead of C_ERR, although the value of C_ERR is also -1. Co-authored-by:
Madelyn Olson <34459052+madolson@users.noreply.github.com>
-
Brennan authored
Re-design cluster link send queue to improve memory management
-
- 01 Nov, 2022 1 commit
-
-
Wen Hui authored
This PR add test case for PR #11417, with only key as argument for GEOHASH and GEOPOS
-
- 29 Oct, 2022 1 commit
-
-
xbasel authored
All current usage of the function are correct, but in the future it might change.
-
- 27 Oct, 2022 2 commits
-
-
Moti Cohen authored
Renamed from "Pause Clients" to "Pause Actions" since the mechanism can pause several actions in redis, not just clients (e.g. eviction, expiration). Previously each pause purpose (which has a timeout that's tracked separately from others purposes), also implicitly dictated what it pauses (reads, writes, eviction, etc). Now it is explicit, and the actions that are paused (bit flags) are defined separately from the purpose. - Previously, when using feature pause-client it also implicitly means to make the server static: - Pause replica traffic - Pauses eviction processing - Pauses expire processing Making the server static is used also for failover and shutdown. This PR internally rebrand pause-client API to become pause-action API. It also Simplifies pauseClients structure by replacing pointers array with static array. The context of this PR is to add another trigger to pause-client which will activated in case of OOM as throttling mechanism ([see here](https://github.com/redis/redis/issues/10907)). In this case we want only to pause client, and eviction actions.
-
Shaya Potter authored
RM_Call is designed to let modules call redis commands disregarding the OOM state (the module is responsible to declare its command flags to redis, or perform the necessary checks). The other (new) alternative is to pass the "M" flag to RM_Call so that redis can OOM reject commands implicitly. However, Currently, RM_Call enforces OOM on scripts (excluding scripts that declared `allow-oom`) in all cases, regardless of the RM_Call "M" flag being present. This PR fixes scripts to be consistent with other commands being executed by RM_Call. It modifies the flow in effect treats scripts as if they if they have the ALLOW_OOM script flag, if the "M" flag is not passed (i.e. no OOM checking is being performed by RM_Call, so no OOM checking should be done on script). Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 26 Oct, 2022 1 commit
-
-
AntiTopQuark authored
Reducing duplicate header file definitions <sys/resource.h>
-
- 25 Oct, 2022 2 commits
-
-
xbasel authored
-
Wen Hui authored
These commands take a list of members, which can be empty (i.e. running the command with just a key name). this always results in an empty array reply, so it doesn't make much sense, but changing it is a breaking change. This PR fixes the documentation, making the member field as optional, just makes sure the command format documentation is consistent with the command behavior. The command format will be: 127.0.0.1:6381> GEOPOS key [member [member ...]] 127.0.0.1:6381> GEOHASH key [member [member ...]]
-
- 24 Oct, 2022 2 commits
-
-
guybe7 authored
We do not need to return the length of argv because it is equal to argc, which we return anyway. This change makes the code cleaner and adds a comment to explain something that might not be immediately clear.
-
guybe7 authored
This is a rare failure mode of a new feature of redis 7 introduced in #9217 (when the incremental part of the ID overflows). Till now, the outcome of that error was undetermined (could easily result in `Elements are too large to be stored` wrongly, due to unset `errno`).
-
- 23 Oct, 2022 2 commits
-
-
Moti Cohen authored
Funcion sentinelAddrEqualsHostname() of sentinel makes DNS resolve and based on it determines if two IP addresses are equal. Now, If the DNS resolve command fails, the function simply returns 0, even if the hostnames are identical. This might become an issue in case of failover such that sentinel might receives from Redis instance, response to regular INFO query it sent, and wrongly decide that the instance is pointing to is different leader than the one recorded because of this function, yet hostnames are identical. In turn sentinel disconnects the connection between sentinel and valid slave which leads to -failover-abort-no-good-slave. See issue #11241. I managed to reproduce only part of the flow in which the function return wrong result and trigger +fix-slave-config. The fix is even if the function failed to resolve then compare based on hostnames. That is our best effort as long as the server is unavailable for some reason. It is fine since Redis instance cannot have multiple hostnames for a given setup
-
qetu3790 authored
redis.set_repl() needs one arg, but the tips says two.
-
- 22 Oct, 2022 2 commits
-
-
Binbin authored
Make PFMERGE source key optional in docs, add tests with one input key, add tests on missing source keys (#11205) The following two cases will create an empty destkey HLL: 1. called with no source keys, like `pfmerge destkey` 2. called with non-existing source keys, like `pfmerge destkey non-existing-source-key` In the first case, in `PFMERGE`, the dest key is actually one of the source keys too. So `PFMERGE k1 k2` is equivalent to `SUNIONSTORE k1 k1 k2`, and `PFMERGE k1` is equivalent to `SUNIONSTORE k1 k1`. So the first case is reasonable, the source key is actually optional. And the second case, `PFMERGE` on missing keys should succeed and create an empty dest. This is consistent with `PFCOUNT`, and also with `SUNIONSTORE`, no need to change.
-
sundb authored
In the module, we will reuse the list iterator entry for RM_ListDelete, but `listTypeDelete` will only update `quicklistEntry->zi` but not `quicklistEntry->node`, which will result in `quicklistEntry->node` pointing to a freed memory address if the quicklist node is deleted. This PR sync `key->u.list.index` and `key->u.list.entry` to list iterator after `RM_ListDelete`. This PR also optimizes the release code of the original list iterator. Co-authored-by:
Viktor Söderqvist <viktor@zuiderkwast.se>
-
- 19 Oct, 2022 1 commit
-
-
FutabaRio authored
11 was the size of header/trailer in the old structure Ziplist, but now the size of header/trailer in the new structure Listpack should be 7.
-
- 18 Oct, 2022 5 commits
-
-
guybe7 authored
The use case is a module that wants to implement a blocking command on a key that necessarily exists and wants to unblock the client in case the key is deleted (much like what we implemented for XREADGROUP in #10306) New module API: * RedisModule_BlockClientOnKeysWithFlags Flags: * REDISMODULE_BLOCK_UNBLOCK_NONE * REDISMODULE_BLOCK_UNBLOCK_DELETED ### Detailed description of code changes blocked.c: 1. Both module and stream functions are called whether the key exists or not, regardless of its type. We do that in order to allow modules/stream to unblock the client in case the key is no longer present or has changed type (the behavior for streams didn't change, just code that moved into serveClientsBlockedOnStreamKey) 2. Make sure afterCommand is called in serveClientsBlockedOnKeyByModule, in order to propagate actions from moduleTryServeClientBlockedOnKey. 3. handleClientsBlockedOnKeys: call propagatePendingCommands directly after lookupKeyReadWithFlags to prevent a possible lazy-expire DEL from being mixed with any command propagated by the preceding functions. 4. blockForKeys: Caller can specifiy that it wants to be awakened if key is deleted. Minor optimizations (use dictAddRaw). 5. signalKeyAsReady became signalKeyAsReadyLogic which can take a boolean in case the key is deleted. It will only signal if there's at least one client that awaits key deletion (to save calls to handleClientsBlockedOnKeys). Minor optimizations (use dictAddRaw) db.c: 1. scanDatabaseForDeletedStreams is now scanDatabaseForDeletedKeys and will signalKeyAsReady for any key that was removed from the database or changed type. It is the responsibility of the code in blocked.c to ignore or act on deleted/type-changed keys. 2. Use the new signalDeletedKeyAsReady where needed blockedonkey.c + tcl: 1. Added test of new capabilities (FSL.BPOPGT now requires the key to exist in order to work)
-
Meir Shpilraien (Spielrein) authored
### Background The issue is that when saving an RDB with module AUX data, the module AUX metadata (moduleid, when, ...) is saved to the RDB even though the module did not saved any actual data. This prevent loading the RDB in the absence of the module (although there is no actual data in the RDB that requires the module to be loaded). ### Solution The solution suggested in this PR is that module AUX will be saved on the RDB only if the module actually saved something during `aux_save` function. To support backward compatibility, we introduce `aux_save2` callback that acts the same as `aux_save` with the tiny change of avoid saving the aux field if no data was actually saved by the module. Modules can use the new API to make sure that if they have no data to save, then it will be possible to load the created RDB even without the module. ### Concerns A module may register for the aux load and save hooks just in order to be notified when saving or loading starts or completed (there are better ways to do that, but it still possible that someone used it). However, if a module didn't save a single field in the save callback, it means it's not allowed to read in the read callback, since it has no way to distinguish between empty and non-empty payloads. furthermore, it means that if the module did that, it must never change it, since it'll break compatibility with it's old RDB files, so this is really not a valid use case. Since some modules (ones who currently save one field indicating an empty payload), need to know if saving an empty payload is valid, and if Redis is gonna ignore an empty payload or store it, we opted to add a new API (rather than change behavior of an existing API and expect modules to check the redis version) ### Technical Details To avoid saving AUX data on RDB, we change the code to first save the AUX metadata (moduleid, when, ...) into a temporary buffer. The buffer is then flushed to the rio at the first time the module makes a write operation inside the `aux_save` function. If the module saves nothing (and `aux_save2` was used), the entire temporary buffer is simply dropped and no data about this AUX field is saved to the RDB. This make it possible to load the RDB even in the absence of the module. Test was added to verify the fix.
-
Shuning authored
Seems excessive to call getExpire if we don't need it. This can maybe have some speedup on AOF file loading (saving a dictFind call) Co-authored-by:
lvshuning <lvshuning@meituan.com>
-
DarrenJiang13 authored
fix some malloc macros in `listpack.c`. listpack has it's own malloc aliases, but in some places normal redis malloc calls have slipped in.
-
Binbin authored
And fix a few newly detected typo. Closes #11394
-
- 16 Oct, 2022 3 commits
-
-
Shaya Potter authored
Motivation: for applications that use RM ACL verification functions, they would want to return errors back to the user, in ways that are consistent with Redis. While investigating how we should return ACL errors to the user, we realized that Redis isn't consistent, and currently returns ACL error strings in 3 primary ways. [For the actual implications of this change, see the "Impact" section at the bottom] 1. how it returns an error when calling a command normally ACL_DENIED_CMD -> "this user has no permissions to run the '%s' command" ACL_DENIED_KEY -> "this user has no permissions to access one of the keys used as arguments" ACL_DENIED_CHANNEL -> "this user has no permissions to access one of the channels used as arguments" 2. how it returns an error when calling via 'acl dryrun' command ACL_DENIED_CMD -> "This user has no permissions to run the '%s' command" ACL_DENIED_KEY -> "This user has no permissions to access the '%s' key" ACL_DENIED_CHANNEL -> "This user has no permissions to access the '%s' channel" 3. how it returns an error via RM_Call (and scripting is similar). ACL_DENIED_CMD -> "can't run this command or subcommand"; ACL_DENIED_KEY -> "can't access at least one of the keys mentioned in the command arguments"; ACL_DENIED_CHANNEL -> "can't publish to the channel mentioned in the command"; In addition, if one wants to use RM_Call's "dry run" capability instead of the RM ACL functions directly, one also sees a different problem than it returns ACL errors with a -ERR, not a -PERM, so it can't be returned directly to the caller. This PR modifies the code to generate a base message in a common manner with the ability to set verbose flag for acl dry run errors, and keep it unset for normal/rm_call/script cases ```c sds getAclErrorMessage(int acl_res, user *user, struct redisCommand *cmd, sds errored_val, int verbose) { switch (acl_res) { case ACL_DENIED_CMD: return sdscatfmt(sdsempty(), "User %S has no permissions to run " "the '%S' command", user->name, cmd->fullname); case ACL_DENIED_KEY: if (verbose) { return sdscatfmt(sdsempty(), "User %S has no permissions to access " "the '%S' key", user->name, errored_val); } else { return sdsnew("No permissions to access a key"); } case ACL_DENIED_CHANNEL: if (verbose) { return sdscatfmt(sdsempty(), "User %S has no permissions to access " "the '%S' channel", user->name, errored_val); } else { return sdsnew("No permissions to access a channel"); } } ``` The caller can append/prepend the message (adding NOPERM for normal/RM_Call or indicating it's within a script). Impact: - Plain commands, as well as scripts and RM_Call now include the user name. - ACL DRYRUN remains the only one that's verbose (mentions the offending channel or key name) - Changes RM_Call ACL errors from being a `-ERR` to being `-NOPERM` (besides for textual changes) **This somewhat a breaking change, but it only affects the RM_Call with both `C` and `E`, or `D`** - Changes ACL errors in scripts textually from being `The user executing the script <old non unified text>` to `ACL failure in script: <new unified text>`
-
Meir Shpilraien (Spielrein) authored
As discussed on #11084, `propagatePendingCommands` should happened after the del notification is fired so that the notification effect and the `del` will be replicated inside MULTI EXEC. Test was added to verify the fix.
-
David CARLIER authored
* Fixes build warning when CACHE_LINE_SIZE is already defined * Fixes wrong CACHE_LINE_SIZE on some FreeBSD systems where it could be set to 128 (e.g. on MIPS) * Fixes wrong CACHE_LINE_SIZE on Apple M1 (use 128 instead of 64) Wrong cache line size in that case can some false sharing of array elements between threads, see #10892
-
- 15 Oct, 2022 1 commit
-
-
filipe oliveira authored
optimizing d2string() and addReplyDouble() with grisu2: double to string conversion based on Florian Loitsch's Grisu-algorithm (#10587) All commands / use cases that heavily rely on double to a string representation conversion, (e.g. meaning take a double-precision floating-point number like 1.5 and return a string like "1.5" ), could benefit from a performance boost by swapping snprintf(buf,len,"%.17g",value) by the equivalent [fpconv_dtoa](https://github.com/night-shift/fpconv) or any other algorithm that ensures 100% coverage of conversion. This is a well-studied topic and Projects like MongoDB. RedPanda, PyTorch leverage libraries ( fmtlib ) that use the optimized double to string conversion underneath. The positive impact can be substantial. This PR uses the grisu2 approach ( grisu explained on https://www.cs.tufts.edu/~nr/cs257/archive/florian-loitsch/printf.pdf section 5 ). test suite changes: Despite being compatible, in some cases it produces a different result from printf, and some tests had to be adjusted. one case is that `%.17g` (which means %e or %f which ever is shorter), chose to use `5000000000` instead of 5e+9, which sounds like a bug? In other cases, we changed TCL to compare numbers instead of strings to ignore minor rounding issues (`expr 0.8 == 0.79999999999999999`)
-
- 13 Oct, 2022 2 commits
-
-
C Charles authored
MIGTATE with AUTH that contains "keys" is getting wrong key names in migrateGetKeys, leads to ACL errors (#11253) When using the MIGRATE, with a destination Redis that has the user name or password set to the string "keys", Redis would have determine the wrong set of key names the command is gonna access. This lead to ACL returning wrong authentication result. Destination instance: ``` 127.0.0.1:6380> acl setuser default >keys OK 127.0.0.1:6380> acl setuser keys on nopass ~* &* +@all OK ``` Source instance: ``` 127.0.0.1:6379> set a 123 OK 127.0.0.1:6379> acl setuser cc on nopass ~a* +@all OK 127.0.0.1:6379> auth cc 1 OK 127.0.0.1:6379> migrate 127.0.0.1 6380 "" 0 1000 auth keys keys a (error) NOPERM this user has no permissions to access one of the keys used as arguments 127.0.0.1:6379> migrate 127.0.0.1 6380 "" 0 1000 auth2 keys pswd keys a (error) NOPERM this user has no permissions to access one of the keys used as arguments ``` Using `acl dryrun` we know that the parameters of `auth` and `auth2` are mistaken for the `keys` option. ``` 127.0.0.1:6379> acl dryrun cc migrate whatever whatever "" 0 1000 auth keys keys a "This user has no permissions to access the 'keys' key" 127.0.0.1:6379> acl dryrun cc migrate whatever whatever "" 0 1000 auth2 keys pswd keys a "This user has no permissions to access the 'pswd' key" ``` Fix the bug by editing db.c/migrateGetKeys function, which finds the `keys` option and all the keys following.
-
Oran Agra authored
1. show the overcommit warning when overcommit is disabled (2), not just when it is set to heuristic (0). 2. improve warning text to mention the issue with jemalloc causing VM mapping fragmentation when set to 2.
-
- 12 Oct, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
PR #9320 introduces initialization order changes. Now cluster is initialized after modules. This changes causes a crash if the module uses RM_Call inside the load function on cluster mode (the code will try to access `server.cluster` which at this point is NULL). To solve it, separate cluster initialization into 2 phases: 1. Structure initialization that happened before the modules initialization 2. Listener initialization that happened after. Test was added to verify the fix.
-
- 11 Oct, 2022 2 commits
-
-
Binbin authored
Redis commands has been significantly refactored in 7.0. This PR updates the outdated README.md to reflect it. Based on #10864, doing some additional cleanups. Co-authored-by:
theoboldalex <theoboldalex@gmail.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
Nikita Tolkachev authored
remove double period at the end of sentence.
-