- 09 Nov, 2022 1 commit
-
-
Viktor Söderqvist authored
Small sets with not only integer elements are listpack encoded, by default up to 128 elements, max 64 bytes per element, new config `set-max-listpack-entries` and `set-max-listpack-value`. This saves memory for small sets compared to using a hashtable. Sets with only integers, even very small sets, are still intset encoded (up to 1G limit, etc.). Larger sets are hashtable encoded. This PR increments the RDB version, and has an effect on OBJECT ENCODING Possible conversions when elements are added: intset -> listpack listpack -> hashtable intset -> hashtable Note: No conversion happens when elements are deleted. If all elements are deleted and then added again, the set is deleted and recreated, thus implicitly converted to a smaller encoding.
-
- 02 Nov, 2022 1 commit
-
-
Brennan authored
Re-design cluster link send queue to improve memory management
-
- 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>
-
- 18 Oct, 2022 2 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.
-
- 16 Oct, 2022 1 commit
-
-
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>`
-
- 09 Oct, 2022 1 commit
-
-
Binbin authored
Freeze time during execution of scripts and all other commands. This means that a key is either expired or not, and doesn't change state during a script execution. resolves #10182 This PR try to add a new `commandTimeSnapshot` function. The function logic is extracted from `keyIsExpired`, but the related calls to `fixed_time_expire` and `mstime()` are removed, see below. In commands, we will avoid calling `mstime()` multiple times and just use the one that sampled in call. The background is, e.g. using `PEXPIRE 1` with valgrind sometimes result in the key being deleted rather than expired. The reason is that both `PEXPIRE` command and `checkAlreadyExpired` call `mstime()` separately. There are other more important changes in this PR: 1. Eliminate `fixed_time_expire`, it is no longer needed. When we want to sample time we should always use a time snapshot. We will use `in_nested_call` instead to update the cached time in `call`. 2. Move the call for `updateCachedTime` from `serverCron` to `afterSleep`. Now `commandTimeSnapshot` will always return the sample time, the `lookupKeyReadWithFlags` call in `getNodeByQuery` will get a outdated cached time (because `processCommand` is out of the `call` context). We put the call to `updateCachedTime` in `aftersleep`. 3. Cache the time each time the module lock Redis. Call `updateCachedTime` in `moduleGILAfterLock`, affecting `RM_ThreadSafeContextLock` and `RM_ThreadSafeContextTryLock` Currently the commandTimeSnapshot change affects the following TTL commands: - SET EX / SET PX - EXPIRE / PEXPIRE - SETEX / PSETEX - GETEX EX / GETEX PX - TTL / PTTL - EXPIRETIME / PEXPIRETIME - RESTORE key TTL And other commands just use the cached mstime (including TIME). This is considered to be a breaking change since it can break a script that uses a loop to wait for a key to expire.
-
- 07 Oct, 2022 1 commit
-
-
aradz44 authored
Added authentication failure and access denied metrics
-
- 03 Oct, 2022 1 commit
-
-
Madelyn Olson authored
This PR introduces a couple of changes to improve cluster test stability: 1. Increase the cluster node timeout to 3 seconds, which is similar to the normal cluster tests, but introduce a new mechanism to increase the ping period so that the tests are still fast. This new config is a debug config. 2. Set `cluster-replica-no-failover yes` on a wider array of tests which are sensitive to failovers. This was occurring on the ARM CI.
-
- 02 Oct, 2022 1 commit
-
-
Binbin authored
- fix `the the` typo - `LPOPRPUSH` does not exist, should be `RPOPLPUSH` - `CLUSTER GETKEYINSLOT` 's time complexity should be O(N) - `there bytes` should be `three bytes`, this closes #11266 - `slave` word to `replica` in log, modified the front and missed the back - remove useless aofReadDiffFromParent in server.h - `trackingHandlePendingKeyInvalidations` method adds a void parameter
-
- 28 Sep, 2022 1 commit
-
-
sundb authored
Fix the missing server.dirty increment and redundant signalModifiedKey in serveClientBlockedOnList (#11326) Mainly fix two minor bug 1. When handle BL*POP/BLMOVE commands with blocked clients, we should increment server.dirty. 2. `listPopRangeAndReplyWithKey()` in `serveClientBlockedOnList()` should not repeat calling `signalModifiedKey()` which has been called when an element was pushed on the list. (was skipped in all bpop commands, other than blmpop) Other optimization add `signal` param for `listElementsRemoved` to skip `signalModifiedKey()` to unify all pop operation. Unifying all pop operations also prepares us for #11303, so that we can avoid having to deal with the conversion from quicklist to listpack() in various places when the list shrinks.
-
- 22 Sep, 2022 2 commits
-
-
Shaya Potter authored
Adds a number of user management/ACL validaiton/command execution functions to improve a Redis module's ability to enforce ACLs correctly and easily. * RM_SetContextUser - sets a RedisModuleUser on the context, which RM_Call will use to both validate ACLs (if requested and set) as well as assign to the client so that scripts executed via RM_Call will have proper ACL validation. * RM_SetModuleUserACLString - Enables one to pass an entire ACL string, not just a single OP and have it applied to the user * RM_GetModuleUserACLString - returns a stringified version of the user's ACL (same format as dump and list). Contains an optimization to cache the stringified version until the underlying ACL is modified. * Slightly re-purpose the "C" flag to RM_Call from just being about ACL check before calling the command, to actually running the command with the right user, so that it also affects commands inside EVAL scripts. see #11231
-
Valentino Geron authored
The bug is that the the server keeps on sending newlines to the client. As a result, the receiver might not find the EOF marker since it searches for it only on the end of each payload it reads from the socket. The but only affects `CLIENT_REPL_RDBONLY`. This affects `redis-cli --rdb` (depending on timing) The fixed consist of two steps: 1. The `CLIENT_REPL_RDBONLY` should be closed ASAP (we cannot always call to `freeClient` so we use `freeClientAsync`) 2. Add new replication state `SLAVE_STATE_RDB_TRANSMITTED`
-
- 15 Sep, 2022 1 commit
-
-
Adi Pinsky authored
-
- 28 Aug, 2022 1 commit
-
-
Shaya Potter authored
When RM_Call was used with `M` (reject OOM), `W` (reject writes), as well as `S` (rejecting stale or write commands in "Script mode"), it would have only checked the command flags, but not the declared script flag in case it's a command that runs a script. Refactoring: extracts out similar code in server.c's processCommand to be usable in RM_Call as well.
-
- 24 Aug, 2022 2 commits
-
-
Oran Agra authored
Redis 7.0 has #9890 which added an assertion when the propagation queue was not flushed and we got to beforeSleep. But it turns out that when processCommands calls getNodeByQuery and decides to reject the command, it can lead to a key that was lazy expired and is deleted without later flushing the propagation queue. This change prevents lazy expiry from deleting the key at this stage (not as part of a command being processed in `call`)
-
Meir Shpilraien (Spielrein) authored
The PR reverts the changes made on #10969. The reason for revert was trigger because of occasional test failure that started after the PR was merged. The issue is that if there is a lazy expire during the command invocation, the `del` command is added to the replication stream after the command placeholder. So the logical order on the primary is: * Delete the key (lazy expiration) * Command invocation But the replication stream gets it the other way around: * Command invocation (because the command is written into the placeholder) * Delete the key (lazy expiration) So if the command write to the key that was just lazy expired we will get inconsistency between primary and replica. One solution we considered is to add another lazy expire replication stream and write all the lazy expire there. Then when replicating, we will replicate the lazy expire replication stream first. This will solve this specific test failure but we realize that the issues does not ends here and the more we dig the more problems we find.One of the example we thought about (that can actually crashes Redis) is as follow: * User perform SINTERSTORE * When Redis tries to fetch the second input key it triggers lazy expire * The lazy expire trigger a module logic that deletes the first input key * Now Redis hold the robj of the first input key that was actually freed We believe we took the wrong approach and we will come up with another PR that solve the problem differently, for now we revert the changes so we will not have the tests failure. Notice that not the entire code was revert, some parts of the PR are changes that we would like to keep. The changes that **was** reverted are: * Saving a placeholder for replication at the beginning of the command (`call` function) * Order of the replication stream on active expire and eviction (we will decide how to handle it correctly on follow up PR) * `Spop` changes are no longer needed (because we reverted the placeholder code) Changes that **was not** reverted: * On expire/eviction, wrap the `del` and the notification effect in a multi exec. * `PropagateNow` function can still accept a special dbid, -1, indicating not to replicate select. * Keep optimisation for reusing the `alsoPropagate` array instead of allocating it each time. Tests: * All tests was kept and only few tests was modify to work correctly with the changes * Test was added to verify that the revert fixes the issues.
-
- 23 Aug, 2022 1 commit
-
-
Oran Agra authored
* Support BUILD_TLS=module to be loaded as a module via config file or command line. e.g. redis-server --loadmodule redis-tls.so * Updates to redismodule.h to allow it to be used side by side with server.h by defining REDISMODULE_CORE_MODULE * Changes to server.h, redismodule.h and module.c to avoid repeated type declarations (gcc 4.8 doesn't like these) * Add a mechanism for non-ABI neutral modules (ones who include server.h) to refuse loading if they detect not being built together with redis (release.c) * Fix wrong signature of RedisModuleDefragFunc, this could break compilation of a module, but not the ABI * Move initialization of listeners in server.c to be after loading the modules * Config TLS after initialization of listeners * Init cluster after initialization of listeners * Add TLS module to CI * Fix a test suite race conditions: Now that the listeners are initialized later, it's not sufficient to wait for the PID message in the log, we need to wait for the "Server Initialized" message. * Fix issues with moduleconfigs test as a result from start_server waiting for "Server Initialized" * Fix issues with modules/infra test as a result of an additional module present Notes about Sentinel: Sentinel can't really rely on the tls module, since it uses hiredis to initiate connections and depends on OpenSSL (won't be able to use any other connection modules for that), so it was decided that when TLS is built as a module, sentinel does not support TLS at all. This means that it keeps using redis_tls_ctx and redis_tls_client_ctx directly. Example code of config in redis-tls.so(may be use in the future): RedisModuleString *tls_cfg = NULL; void tlsInfo(RedisModuleInfoCtx *ctx, int for_crash_report) { UNUSED(for_crash_report); RedisModule_InfoAddSection(ctx, ""); RedisModule_InfoAddFieldLongLong(ctx, "var", 42); } int tlsCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { if (argc != 2) return RedisModule_WrongArity(ctx); return RedisModule_ReplyWithString(ctx, argv[1]); } RedisModuleString *getStringConfigCommand(const char *name, void *privdata) { REDISMODULE_NOT_USED(name); REDISMODULE_NOT_USED(privdata); return tls_cfg; } int setStringConfigCommand(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) { REDISMODULE_NOT_USED(name); REDISMODULE_NOT_USED(err); REDISMODULE_NOT_USED(privdata); if (tls_cfg) RedisModule_FreeString(NULL, tls_cfg); RedisModule_RetainString(NULL, new); tls_cfg = new; return REDISMODULE_OK; } int RedisModule_OnLoad(void *ctx, RedisModuleString **argv, int argc) { .... if (RedisModule_CreateCommand(ctx,"tls",tlsCommand,"",0,0,0) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_RegisterStringConfig(ctx, "cfg", "", REDISMODULE_CONFIG_DEFAULT, getStringConfigCommand, setStringConfigCommand, NULL, NULL) == REDISMODULE_ERR) return REDISMODULE_ERR; if (RedisModule_LoadConfigs(ctx) == REDISMODULE_ERR) { if (tls_cfg) { RedisModule_FreeString(ctx, tls_cfg); tls_cfg = NULL; } return REDISMODULE_ERR; } ... } Co-authored-by:
zhenwei pi <pizhenwei@bytedance.com> Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
- 22 Aug, 2022 5 commits
-
-
zhenwei pi authored
Introduce listen method into connection type, this allows no hard code of listen logic. Originally, we initialize server during startup like this: if (server.port) listenToPort(server.port,&server.ipfd); if (server.tls_port) listenToPort(server.port,&server.tlsfd); if (server.unixsocket) anetUnixServer(...server.unixsocket...); ... if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK) if (createSocketAcceptHandler(&server.tlsfd, acceptTcpHandler) != C_OK) if (createSocketAcceptHandler(&server.sofd, acceptTcpHandler) != C_OK) ... If a new connection type gets supported, we have to add more hard code to setup listener. Introduce .listen and refactor listener, and Unix socket supports this. this allows to setup listener arguments and create listener in a loop. What's more, '.listen' is defined in connection.h, so we should include server.h to import 'struct socketFds', but server.h has already include 'connection.h'. To avoid including loop(also to make code reasonable), define 'struct connListener' in connection.h instead of 'struct socketFds' in server.h. This leads this commit to get more changes. There are more fields in 'struct connListener', hence it's possible to simplify changeBindAddr & applyTLSPort() & updatePort() into a single logic: update the listener config from the server.xxx, and re-create the listener. Because of the new field 'priv' in struct connListener, we expect to pass this to the accept handler(even it's not used currently), this may be used in the future. Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
zhenwei pi authored
Unix socket uses different accept handler/create listener from TCP, to hide these difference to avoid hard code, use a new unix socket connection type. Also move 'acceptUnixHandler' into unix.c. Currently, the connection framework becomes like following: uplayer | connection layer / | \ TCP Unix TLS It's possible to build Unix socket support as a shared library, and load it dynamically. Because TCP and Unix socket don't require any heavy dependencies or overheads, we build them into Redis statically. Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
zhenwei pi authored
Abstract accept handler for socket&TLS, and add helper function 'connAcceptHandler' to get accept handler by specified type. Also move acceptTcpHandler into socket.c, and move acceptTLSHandler into tls.c. Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
zhenwei pi authored
socketFds is also suitable for Unix socket, then we can use 'createSocketAcceptHandler' to create accept handler. And then, we can abstract accept handler in the future. Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
zhenwei pi authored
Use connTypeRegister() to register a connection type into redis, and query connection by connectionByType() via type. With this change, we can hide TLS specified methods into connection type: - void tlsInit(void); - void tlsCleanup(void); - int tlsConfigure(redisTLSContextConfig *ctx_config); - int isTlsConfigured(void); Merge isTlsConfigured & tlsConfigure, use an argument *reconfigure* to distinguish: tlsConfigure(&server.tls_ctx_config) -> onnTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 1) isTlsConfigured() && tlsConfigure(&server.tls_ctx_config) -> connTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 0) Finally, we can remove USE_OPENSSL from config.c. If redis is built without TLS, and still run redis with TLS, then redis reports: # Missing implement of connection type 1 # Failed to configure TLS. Check logs for more info. The log can be optimised, let's leave it in the future. Maybe we can use connection type as a string. Although uninitialized fields of a static struct are zero, we still set them as NULL explicitly in socket.c, let them clear to read & maintain: .init = NULL, .cleanup = NULL, .configure = NULL, Signed-off-by:
zhenwei pi <pizhenwei@bytedance.com>
-
- 21 Aug, 2022 1 commit
-
-
yourtree authored
Till now Redis officially supported tuning it via environment variable see #1074. But we had other requests to allow changing it at runtime, see #799, and #11041. Note that `strcoll()` is used as Lua comparison function and also for comparison of certain string objects in Redis, which leads to a problem that, in different regions, for some characters, the result may be different. Below is an example. ``` 127.0.0.1:6333> SORT test alpha 1) "<" 2) ">" 3) "," 4) "*" 127.0.0.1:6333> CONFIG GET locale-collate 1) "locale-collate" 2) "" 127.0.0.1:6333> CONFIG SET locale-collate 1 (error) ERR CONFIG SET failed (possibly related to argument 'locale') 127.0.0.1:6333> CONFIG SET locale-collate C OK 127.0.0.1:6333> SORT test alpha 1) "*" 2) "," 3) "<" 4) ">" ``` That will cause accidental code compatibility issues for Lua scripts and some Redis commands. This commit creates a new config parameter to control the local environment which only affects `Collate` category. Above shows how it affects `SORT` command, and below shows the influence on Lua scripts. ``` 127.0.0.1:6333> CONFIG GET locale-collate 1) " locale-collate" 2) "C" 127.0.0.1:6333> EVAL "return ',' < '*'" 0 (nil) 127.0.0.1:6333> CONFIG SET locale-collate "" OK 127.0.0.1:6333> EVAL "return ',' < '*'" 0 (integer) 1 ``` Co-authored-by:
calvincjli <calvincjli@tencent.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 18 Aug, 2022 2 commits
-
-
guybe7 authored
This PR makes sure that "name" is unique for all arguments in the same level (i.e. all args of a command and all args within a block/oneof). This means several argument with identical meaning can be referred to together, but also if someone needs to refer to a specific one, they can use its full path. In addition, the "display_text" field has been added, to be used by redis.io in order to render the syntax of the command (for the vast majority it is identical to "name" but sometimes we want to use a different string that is not "name") The "display" field is exposed via COMMAND DOCS and will be present for every argument, except "oneof" and "block" (which are container arguments) Other changes: 1. Make sure we do not have any container arguments ("oneof" or "block") that contain less than two sub-args (otherwise it doesn't make sense) 2. migrate.json: both AUTH and AUTH2 should not be "optional" 3. arg names cannot contain un...
-
Meir Shpilraien (Spielrein) authored
Fix replication inconsistency on modules that uses key space notifications. ### The Problem In general, key space notifications are invoked after the command logic was executed (this is not always the case, we will discuss later about specific command that do not follow this rules). For example, the `set x 1` will trigger a `set` notification that will be invoked after the `set` logic was performed, so if the notification logic will try to fetch `x`, it will see the new data that was written. Consider the scenario on which the notification logic performs some write commands. for example, the notification logic increase some counter, `incr x{counter}`, indicating how many times `x` was changed. The logical order by which the logic was executed is has follow: ``` set x 1 incr x{counter} ``` The issue is that the `set x 1` command is added to the replication buffer at the end of the command invocation (specifically after the key space notification logic was invoked and performed the `incr` command). The replication/aof sees the commands in the wrong order: ``` incr x{counter} set x 1 ``` In this specific example the order is less important. But if, for example, the notification would have deleted `x` then we would end up with primary-replica inconsistency. ### The Solution Put the command that cause the notification in its rightful place. In the above example, the `set x 1` command logic was executed before the notification logic, so it should be added to the replication buffer before the commands that is invoked by the notification logic. To achieve this, without a major code refactoring, we save a placeholder in the replication buffer, when finishing invoking the command logic we check if the command need to be replicated, and if it does, we use the placeholder to add it to the replication buffer instead of appending it to the end. To be efficient and not allocating memory on each command to save the placeholder, the replication buffer array was modified to reuse memory (instead of allocating it each time we want to replicate commands). Also, to avoid saving a placeholder when not needed, we do it only for WRITE or MAY_REPLICATE commands. #### Additional Fixes * Expire and Eviction notifications: * Expire/Eviction logical order was to first perform the Expire/Eviction and then the notification logic. The replication buffer got this in the other way around (first notification effect and then the `del` command). The PR fixes this issue. * The notification effect and the `del` command was not wrap with `multi-exec` (if needed). The PR also fix this issue. * SPOP command: * On spop, the `spop` notification was fired before the command logic was executed. The change in this PR would have cause the replication order to be change (first `spop` command and then notification `logic`) although the logical order is first the notification logic and then the `spop` logic. The right fix would have been to move the notification to be fired after the command was executed (like all the other commands), but this can be considered a breaking change. To overcome this, the PR keeps the current behavior and changes the `spop` code to keep the right logical order when pushing commands to the replication buffer. Another PR will follow to fix the SPOP properly and match it to the other command (we split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if we chose to). #### Unhanded Known Limitations * key miss event: * On key miss event, if a module performed some write command on the event (using `RM_Call`), the `dirty` counter would increase and the read command that cause the key miss event would be replicated to the replication and aof. This problem can also happened on a write command that open some keys but eventually decides not to perform any action. We decided not to handle this problem on this PR because the solution is complex and will cause additional risks in case we will want to cherry-pick this PR. We should decide if we want to handle it in future PR's. For now, modules writers is advice not to perform any write commands on key miss event. #### Testing * We already have tests to cover cases where a notification is invoking write commands that are also added to the replication buffer, the tests was modified to verify that the replica gets the command in the correct logical order. * Test was added to verify that `spop` behavior was kept unchanged. * Test was added to verify key miss event behave as expected. * Test was added to verify the changes do not break lazy expiration. #### Additional Changes * `propagateNow` function can accept a special dbid, -1, indicating not to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands` function. The side effect of this change is that now the `select` command will appear inside the `multi/exec` block on the replication stream (instead of outside of the `multi/exec` block). Tests was modified to match this new behavior.
-
- 15 Aug, 2022 1 commit
-
-
Oran Agra authored
The initial module format introduced in 4.0 RC1 and was changed in RC2 The initial function format introduced in 7.0 RC1 and changed in RC3
-
- 04 Aug, 2022 1 commit
-
-
filipe oliveira authored
Avoid the sdslen() on shared.crlf given we know its size beforehand. Improve ~3-4% of cpu cycles to lrange logic (#10987) * Avoid the sdslen() on shared.crlf given we know its size beforehand * Removed shared.crlf from sharedObjects
-
- 03 Aug, 2022 1 commit
-
-
Moti Cohen authored
Fixing few macros that doesn't follows most basic safety conventions which is wrapping any usage of passed variable with parentheses and if written more than one command, then wrap it with do-while(0) (or parentheses).
-
- 26 Jul, 2022 1 commit
-
-
Binbin authored
The reason we do this is because in #11036, we added error log message when failing to open RDB file for reading. In loadDdataFromDisk we call rdbLoad and also check errno, now the logging corrupts errno (reported in alpine daily). It is not safe to rely on errno as we do today, so we change the return value of rdbLoad function to enums, like we have when loading an AOF.
-
- 19 Jul, 2022 1 commit
-
-
Binbin authored
Following #10996, it forgot to modify RM_StringCompare in module.c Modified RM_StringCompare, compareStringObjectsWithFlags, compareStringObjects and collateStringObjects.
-
- 07 Jul, 2022 1 commit
-
-
Binbin authored
* Fix some outdated comments and some typo
-
- 06 Jul, 2022 1 commit
-
-
Binbin authored
We already have `pubsub_channels` and `pubsub_patterns` in INFO stats, now add `pubsubshard_channels` (symmetry). Sharded pubsub was added in #8621
-
- 04 Jul, 2022 2 commits
-
-
Harkrishn Patro authored
## Issue During the MULTI/EXEC flow, each command gets queued until the `EXEC` command is received and during this phase on every command queue, a `realloc` is being invoked. This could be expensive based on the realloc behavior (if copy to a new memory location). ## Solution In order to reduce the no. of syscall, couple of optimization I've used. 1. By default, reserve memory for atleast two commands. `MULTI/EXEC` for a single command doesn't have any significance. Hence, I believe customer wouldn't use it. 2. For further reservation, increase the memory allocation in exponent growth (power of 2). This reduces the no. of `realloc` call from `N` to `log(N)` times. ## Other changes: * Include multi exec queued command array in client memory consumption calculation (affects client eviction too)
-
Harkrishn Patro authored
Account sharded pubsub channels memory consumption in client memory usage computation to accurately evict client based on the set threshold for `maxmemory-clients`.
-
- 30 Jun, 2022 1 commit
-
-
Binbin authored
This was harmless because we marked the parent command with SENTINEL flag. So the populateCommandTable was ok. And we also don't show the flag (SENTINEL and ONLY-SENTNEL) in COMMAND INFO. In this PR, we also add the same CMD_SENTINEL and CMD_ONLY_SENTINEL flags check when populating the sub-commands. so that in the future it'll be possible to add some sub-commands to sentinel or sentinel-only but not others.
-
- 26 Jun, 2022 1 commit
-
-
Viktor Söderqvist authored
Adding Module APIs to let the module read and set the client name of an arbitrary connection.
-
- 12 Jun, 2022 1 commit
-
-
Binbin authored
The SET and BITFIELD command were added `get_keys_function` in #10148, causing them to be wrongly marked movablekeys in `populateCommandMovableKeys`. This was an unintended side effect introduced in #10148 (7.0 RC1) which could cause some clients an extra round trip for these commands in cluster mode. Since we define movablekeys as a way to determine if the legacy range [first, last, step] doesn't find all keys, then we need a completely different approach. The right approach should be to check if the legacy range covers all key-specs, and if none of the key-specs have the INCOMPLETE flag. This way, we don't need to look at getkeys_proc of VARIABLE_FLAG at all. Probably with the exception of modules, who may still not be using key-specs. In this PR, we removed `populateCommandMovableKeys` and put its logic in `populateCommandLegacyRangeSpec`. In order to properly serve both old and new modules, we must probably keep relying CMD_MODULE_GETKEYS, but do that only for modules that don't declare key-specs. For ones that do, we need to take the same approach we take with native redis commands. This approach was proposed by Oran. Fixes #10833 Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 06 Jun, 2022 1 commit
-
-
DarrenJiang13 authored
Split instantaneous_repl_total_kbps to instantaneous_input_repl_kbps and instantaneous_output_repl_kbps. (#10810) A supplement to https://github.com/redis/redis/pull/10062 Split `instantaneous_repl_total_kbps` to `instantaneous_input_repl_kbps` and `instantaneous_output_repl_kbps`. ## Work: This PR: - delete 1 info field: - `instantaneous_repl_total_kbps` - add 2 info fields: - `instantaneous_input_repl_kbps / instantaneous_output_repl_kbps` ## Result: - master ``` total_net_input_bytes:26633673 total_net_output_bytes:21716596 total_net_repl_input_bytes:0 total_net_repl_output_bytes:18433052 instantaneous_input_kbps:0.02 instantaneous_output_kbps:0.00 instantaneous_input_repl_kbps:0.00 instantaneous_output_repl_kbps:0.00 ``` - slave ``` total_net_input_bytes:18433212 total_net_output_bytes:94790 total_net_repl_input_bytes:18433052 total_net_repl_output_bytes:0 instantaneous_input_kbps:0.00 instantaneous_output_kbps:0.05 instantaneous_input_repl_kbps:0.00 instantaneous_output_repl_kbps:0.00 ```
-