- 05 Apr, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
## Move library meta data to be part of the library payload. Following the discussion on https://github.com/redis/redis/issues/10429 and the intention to add (in the future) library versioning support, we believe that the entire library metadata (like name and engine) should be part of the library payload and not provided by the `FUNCTION LOAD` command. The reasoning behind this is that the programmer who developed the library should be the one who set those values (name, engine, and in the future also version). **It is not the responsibility of the admin who load the library into the database.** The PR moves all the library metadata (engine and function name) to be part of the library payload. The metadata needs to be provided on the first line of the payload using the shebang format (`#!<engine> name=<name>`), example: ```lua #!lua name=test redis.register_function('foo', function() return 1 end) ``` The above script will run on the Lua engine and will create a library called `test`. ## API Changes (compare to 7.0 rc2) * `FUNCTION LOAD` command was change and now it simply gets the library payload and extract the engine and name from the payload. In addition, the command will now return the function name which can later be used on `FUNCTION DELETE` and `FUNCTION LIST`. * The description field was completely removed from`FUNCTION LOAD`, and `FUNCTION LIST` ## Breaking Changes (compare to 7.0 rc2) * Library description was removed (we can re-add it in the future either as part of the shebang line or an additional line). * Loading an AOF file that was generated by either 7.0 rc1 or 7.0 rc2 will fail because the old command syntax is invalid. ## Notes * Loading an RDB file that was generated by rc1 / rc2 **is** supported, Redis will automatically add the shebang to the libraries payloads (we can probably delete that code after 7.0.3 or so since there's no need to keep supporting upgrades from an RC build).
-
- 04 Apr, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
If, for some reason, Redis decides not to execute the script, we need to pop the function and error handler from Lua stack. Otherwise, eventually the Lua stack will explode. Relevant only for 7.0-rc1 and 7.0-rc2.
-
- 02 Apr, 2022 1 commit
-
-
Viktor Söderqvist authored
* Fix race condition where node loses its last slot and turns into replica When a node has lost its last slot and finds out from the SETSLOT command before the cluster bus PONG from the new owner arrives. In this case, the node didn't turn itself into a replica of the new slot owner. This commit adds the same logic to the SETSLOT command as already exists for the cluster bus PONG processing. * Revert "Fix new / failing cluster slot migration test (#10482)" This reverts commit 0b21ef8d. In this test, the old slot owner finds out that it has lost its last slot in a nondeterministic way. Either the cluster bus PONG from the new slot owner and sometimes in a SETSLOT command from redis-cli. In both cases, the result should be the same and the old owner should turn itself into a replica of the new slot owner.
-
- 31 Mar, 2022 2 commits
-
-
sundb authored
Fix global `strval` not reset to NULL after being freed, causing a crash on alpine (most likely because the dynamic library loader doesn't init globals on reload) By the way, fix the memory leak of using `RedisModule_Free` to free `RedisModuleString`, and add a corresponding test.
-
Binbin authored
Fix three timing issues in the test
-
- 30 Mar, 2022 1 commit
-
-
Nick Chun authored
This feature adds the ability to add four different types (Bool, Numeric, String, Enum) of configurations to a module to be accessed via the redis config file, and the CONFIG command. **Configuration Names**: We impose a restriction that a module configuration always starts with the module name and contains a '.' followed by the config name. If a module passes "config1" as the name to a register function, it will be registered as MODULENAME.config1. **Configuration Persistence**: Module Configurations exist only as long as a module is loaded. If a module is unloaded, the configurations are removed. There is now also a minimal core API for removal of standardConfig objects from configs by name. **Get and Set Callbacks**: Storage of config values is owned by the module that registers them, and provides callbacks for Redis to access and manipulate the values. This is exposed through a GET and SET callback. The get callback returns a typed value of the config to redis. The callback takes the name of the configuration, and also a privdata pointer. Note that these only take the CONFIGNAME portion of the config, not the entire MODULENAME.CONFIGNAME. ``` typedef RedisModuleString * (*RedisModuleConfigGetStringFunc)(const char *name, void *privdata); typedef long long (*RedisModuleConfigGetNumericFunc)(const char *name, void *privdata); typedef int (*RedisModuleConfigGetBoolFunc)(const char *name, void *privdata); typedef int (*RedisModuleConfigGetEnumFunc)(const char *name, void *privdata); ``` Configs must also must specify a set callback, i.e. what to do on a CONFIG SET XYZ 123 or when loading configurations from cli/.conf file matching these typedefs. *name* is again just the CONFIGNAME portion, *val* is the parsed value from the core, *privdata* is the registration time privdata pointer, and *err* is for providing errors to a client. ``` typedef int (*RedisModuleConfigSetStringFunc)(const char *name, RedisModuleString *val, void *privdata, RedisModuleString **err); typedef int (*RedisModuleConfigSetNumericFunc)(const char *name, long long val, void *privdata, RedisModuleString **err); typedef int (*RedisModuleConfigSetBoolFunc)(const char *name, int val, void *privdata, RedisModuleString **err); typedef int (*RedisModuleConfigSetEnumFunc)(const char *name, int val, void *privdata, RedisModuleString **err); ``` Modules can also specify an optional apply callback that will be called after value(s) have been set via CONFIG SET: ``` typedef int (*RedisModuleConfigApplyFunc)(RedisModuleCtx *ctx, void *privdata, RedisModuleString **err); ``` **Flags:** We expose 7 new flags to the module, which are used as part of the config registration. ``` #define REDISMODULE_CONFIG_MODIFIABLE 0 /* This is the default for a module config. */ #define REDISMODULE_CONFIG_IMMUTABLE (1ULL<<0) /* Can this value only be set at startup? */ #define REDISMODULE_CONFIG_SENSITIVE (1ULL<<1) /* Does this value contain sensitive information */ #define REDISMODULE_CONFIG_HIDDEN (1ULL<<4) /* This config is hidden in `config get <pattern>` (used for tests/debugging) */ #define REDISMODULE_CONFIG_PROTECTED (1ULL<<5) /* Becomes immutable if enable-protected-configs is enabled. */ #define REDISMODULE_CONFIG_DENY_LOADING (1ULL<<6) /* This config is forbidden during loading. */ /* Numeric Specific Configs */ #define REDISMODULE_CONFIG_MEMORY (1ULL<<7) /* Indicates if this value can be set as a memory value */ ``` **Module Registration APIs**: ``` int (*RedisModule_RegisterBoolConfig)(RedisModuleCtx *ctx, char *name, int default_val, unsigned int flags, RedisModuleConfigGetBoolFunc getfn, RedisModuleConfigSetBoolFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata); int (*RedisModule_RegisterNumericConfig)(RedisModuleCtx *ctx, const char *name, long long default_val, unsigned int flags, long long min, long long max, RedisModuleConfigGetNumericFunc getfn, RedisModuleConfigSetNumericFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata); int (*RedisModule_RegisterStringConfig)(RedisModuleCtx *ctx, const char *name, const char *default_val, unsigned int flags, RedisModuleConfigGetStringFunc getfn, RedisModuleConfigSetStringFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata); int (*RedisModule_RegisterEnumConfig)(RedisModuleCtx *ctx, const char *name, int default_val, unsigned int flags, const char **enum_values, const int *int_values, int num_enum_vals, RedisModuleConfigGetEnumFunc getfn, RedisModuleConfigSetEnumFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata); int (*RedisModule_LoadConfigs)(RedisModuleCtx *ctx); ``` The module name will be auto appended along with a "." to the front of the name of the config. **What RM_Register[...]Config does**: A RedisModule struct now keeps a list of ModuleConfig objects which look like: ``` typedef struct ModuleConfig { sds name; /* Name of config without the module name appended to the front */ void *privdata; /* Optional data passed into the module config callbacks */ union get_fn { /* The get callback specificed by the module */ RedisModuleConfigGetStringFunc get_string; RedisModuleConfigGetNumericFunc get_numeric; RedisModuleConfigGetBoolFunc get_bool; RedisModuleConfigGetEnumFunc get_enum; } get_fn; union set_fn { /* The set callback specified by the module */ RedisModuleConfigSetStringFunc set_string; RedisModuleConfigSetNumericFunc set_numeric; RedisModuleConfigSetBoolFunc set_bool; RedisModuleConfigSetEnumFunc set_enum; } set_fn; RedisModuleConfigApplyFunc apply_fn; RedisModule *module; } ModuleConfig; ``` It also registers a standardConfig in the configs array, with a pointer to the ModuleConfig object associated with it. **What happens on a CONFIG GET/SET MODULENAME.MODULECONFIG:** For CONFIG SET, we do the same parsing as is done in config.c and pass that as the argument to the module set callback. For CONFIG GET, we call the module get callback and return that value to config.c to return to a client. **CONFIG REWRITE**: Starting up a server with module configurations in a .conf file but no module load directive will fail. The flip side is also true, specifying a module load and a bunch of module configurations will load those configurations in using the module defined set callbacks on a RM_LoadConfigs call. Configs being rewritten works the same way as it does for standard configs, as the module has the ability to specify a default value. If a module is unloaded with configurations specified in the .conf file those configurations will be commented out from the .conf file on the next config rewrite. **RM_LoadConfigs:** `RedisModule_LoadConfigs(RedisModuleCtx *ctx);` This last API is used to make configs available within the onLoad() after they have been registered. The expected usage is that a module will register all of its configs, then call LoadConfigs to trigger all of the set callbacks, and then can error out if any of them were malformed. LoadConfigs will attempt to set all configs registered to either a .conf file argument/loadex argument or their default value if an argument is not specified. **LoadConfigs is a required function if configs are registered. ** Also note that LoadConfigs **does not** call the apply callbacks, but a module can do that directly after the LoadConfigs call. **New Command: MODULE LOADEX [CONFIG NAME VALUE] [ARGS ...]:** This command provides the ability to provide startup context information to a module. LOADEX stands for "load extended" similar to GETEX. Note that provided config names need the full MODULENAME.MODULECONFIG name. Any additional arguments a module might want are intended to be specified after ARGS. Everything after ARGS is passed to onLoad as RedisModuleString **argv. Co-authored-by:
Madelyn Olson <madelyneolson@gmail.com> Co-authored-by:
Madelyn Olson <matolson@amazon.com> Co-authored-by:
sundb <sundbcn@gmail.com> Co-authored-by:
Madelyn Olson <34459052+madolson@users.noreply.github.com> Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
Yossi Gottlieb <yossigo@gmail.com>
-
- 28 Mar, 2022 1 commit
-
-
Oran Agra authored
There are a few places that use a hard coded const of 128 to allocate a buffer for d2string. Replace these with a clear macro. Note that In theory, converting double into string could take as much as nearly 400 chars, but since d2string uses `%g` and not `%f`, it won't pass some 40 chars. unrelated: restore some changes to auto generated commands.c that got accidentally reverted in #10293
-
- 27 Mar, 2022 1 commit
-
-
Oran Agra authored
#10381 fixed an issue in `redis-cli --cluster reshard` that used to fail it (redis-cli) because of a race condition. the race condition is / was that when moving the last slot from a node, sometimes the PONG messages delivering the configuration change arrive to that node before the SETSLOT arrives to it, and it becomes a replica. other times the the SETSLOT arrive first, and then PONG **doesn't** demote it. **however**, the PR also added a new test that suffers from exactly the same race condition, and the tests started failing a lot. The fact is (if i understand it correctly), that this test (the one being deleted here), isn't related to the fix that PR fixed (which was to fix redis-cli). The race condition in the cluster code still happens, and as long as we don't solve it, there's no reason to test it. For now, even if my understandings are wrong, i'm gonna delete that failing test, since as far as i understand, #10381 didn't introduce any new risks for that matter (which are gonna be compromised by removing this check), this race existed since forever, and still exists, and the fact that redis-cli is now immune to it is still being tested. Additional work should be carried to fix it, and i live it for other PRs to handle.
-
- 22 Mar, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
The PR extends RM_Call with 3 new capabilities using new flags that are given to RM_Call as part of the `fmt` argument. It aims to assist modules that are getting a list of commands to be executed from the user (not hard coded as part of the module logic), think of a module that implements a new scripting language... * `S` - Run the command in a script mode, this means that it will raise an error if a command which are not allowed inside a script (flaged with the `deny-script` flag) is invoked (like SHUTDOWN). In addition, on script mode, write commands are not allowed if there is not enough good replicas (as configured with `min-replicas-to-write`) and/or a disk error happened. * `W` - no writes mode, Redis will reject any command that is marked with `write` flag. Again can be useful to modules that implement a new scripting language and wants to prevent any write commands. * `E` - Return errors as RedisModuleCallReply. Today the errors that happened before the command was invoked (like unknown commands or acl error) return a NULL reply and set errno. This might be missing important information about the failure and it is also impossible to just pass the error to the user using RM_ReplyWithCallReply. This new flag allows you to get a RedisModuleCallReply object with the relevant error message and treat it as if it was an error that was raised by the command invocation. Tests were added to verify the new code paths. In addition small refactoring was done to share some code between modules, scripts, and `processCommand` function: 1. `getAclErrorMessage` was added to `acl.c` to unified to log message extraction from the acl result 2. `checkGoodReplicasStatus` was added to `replication.c` to check the status of good replicas. It is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and `processCommand`. 3. `writeCommandsGetDiskErrorMessage` was added to `server.c` to get the error message on persistence failure. Again it is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and `processCommand`.
-
- 21 Mar, 2022 2 commits
-
-
guybe7 authored
Bug introduced in #9403, caused inconsistency between master and replica in case just the length (i.e. set a high-index bit to 0) changed.
-
Meir Shpilraien (Spielrein) authored
Increase function tests timeout to avoid false failures on slow systems.
-
- 20 Mar, 2022 1 commit
-
-
郭伟光 authored
fix #10439. see https://github.com/redis/redis/pull/9872 When executing SHUTDOWN we pause the client so we can un-pause it if the shutdown fails. this could happen during the timeout, if the shutdown is aborted, but could also happen from withing the initial `call()` to shutdown, if the rdb save fails. in that case when we return to `call()`, we'll crash if `c->cmd` has been set to NULL. The call stack is: ``` unblockClient(c) replyToClientsBlockedOnShutdown() cancelShutdown() finishShutdown() prepareForShutdown() shutdownCommand() ``` what's special about SHUTDOWN in that respect is that it can be paused, and then un-paused before the original `call()` returns. tests where added for both failed shutdown, and a followup successful one.
-
- 18 Mar, 2022 1 commit
-
-
sundb authored
When ::singledb is 0, we will use db 9 for the test db. Since ::singledb is set to 1 in the cluster-related tests, but not restored, some subsequent tests associated with db 9 will fail.
-
- 16 Mar, 2022 3 commits
-
-
Viktor Söderqvist authored
After migrating a slot, send CLUSTER SETSLOT NODE to the destination node first to make sure the slot isn't left without an owner in case the destination node crashes before it is set as new owner. When informing the source node, it can happen that the destination node has already informed it and if the source node has lost its last slot, it has already turned itself into a replica. Redis-cli should ignore this error in this case.
-
Binbin authored
The new module redact test will fail with valgrind: ``` [err]: modules can redact arguments in tests/unit/moduleapi/auth.tcl Expected 'slowlog reset' to be equal to 'auth.redact 1 (redacted) 3 (redacted)' (context: type eval line 12 cmd {assert_equal {slowlog reset} [lindex [lindex [r slowlog get] 2] 3]} proc ::test) ``` The reason is that with `slowlog-log-slower-than 10000`, `slowlog get` will have a chance to exceed 10ms. Made two changes to avoid failure: 1. change `slowlog-log-slower-than` from 10000 to -1, distable it. 2. assert to use the previous execution result. In theory, the second one can actually be left unchanged, but i think it will be better if it is changed.
-
Madelyn Olson authored
Add module API for redacting client commands
-
- 15 Mar, 2022 1 commit
-
-
ranshid authored
Currently the sort and sort_ro can access external keys via `GET` and `BY` in order to make sure the user cannot violate the authorization ACL rules, the decision is to reject external keys access patterns unless ACL allows SORT full access to all keys. I.e. for backwards compatibility, SORT with GET/BY keeps working, but if ACL has restrictions to certain keys, these features get permission denied. ### Implemented solution We have discussed several potential solutions and decided to only allow the GET and BY arguments when the user has all key permissions with the SORT command. The reasons being that SORT with GET or BY is problematic anyway, for instance it is not supported in cluster mode since it doesn't declare keys, and we're not sure the combination of that feature with ACL key restriction is really required. **HOWEVER** If in the fullness of time we will identify a real need for fine grain access support ...
-
- 10 Mar, 2022 2 commits
- 09 Mar, 2022 1 commit
-
-
蔡相跃 authored
-
- 08 Mar, 2022 2 commits
-
-
guybe7 authored
Deleting a stream while a client is blocked XREADGROUP should unblock the client. The idea is that if a client is blocked via XREADGROUP is different from any other blocking type in the sense that it depends on the existence of both the key and the group. Even if the key is deleted and then revived with XADD it won't help any clients blocked on XREADGROUP because the group no longer exist, so they would fail with -NOGROUP anyway. The conclusion is that it's better to unblock these clients (with error) upon the deletion of the key, rather than waiting for the first XADD. Other changes: 1. Slightly optimize all `serveClientsBlockedOn*` functions by checking `server.blocked_clients_by_type` 2. All `serveClientsBlockedOn*` functions now use a list iterator rather than looking at `listFirst`, relying on `unblockClient` to delete the head of the list. Before this commit, only `serveClientsBlockedOnStreams` used to work like that. 3. bugfix: CLIENT UNBLOCK ERROR should work even if the command doesn't have a timeout_callback (only relevant to module commands)
-
zhaozhao.zz authored
In some special commands like eval_ro / fcall_ro we allow no-writes commands. But may-replicate commands are no-writes too, that leads crash when client pause write:
-
- 07 Mar, 2022 2 commits
-
-
Shaya Potter authored
Add a new REDISMODULE_EVENT_CONFIG event type for notifying modules when Redis configuration changes.
-
Binbin authored
`Expected '*table size: 4096*' to match '*table size: 8192*'` This test failed once on daily macOS, the reason is because the bgsave has not stopped after the kill and `after 200`. So there is a child process and no rehash triggered. This commit use `waitForBgsave` to wait for it to finish.
-
- 01 Mar, 2022 2 commits
-
-
ranshid authored
In order to resolve some flaky tests which hard rely on examine memory footprint. we introduce the following fixes: # Fix in client-eviction test - by @yoav-steinberg Sometime the libc allocator can use different size client struct allocations. this may cause unexpected memory calculations to fail the test. # Introduce new DEBUG command for disabling reply buffer resizing In order to eliminate reply buffer resizing during specific tests. we introduced the ability to disable (and enable) the resizing cron job Co-authored-by: yoav-steinberg yoav@redislabs.com
-
Harkrishn Patro authored
-
- 28 Feb, 2022 1 commit
-
-
ranshid authored
After introducing #9822 need to prevent client reply buffer shrink to maintain correct client memory math. add needs:debug missing one one test. Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 27 Feb, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
This PR fix 2 issues on Lua scripting: * Server error reply statistics (some errors were counted twice). * Error code and error strings returning from scripts (error code was missing / misplaced). ## Statistics a Lua script user is considered part of the user application, a sophisticated transaction, so we want to count an error even if handled silently by the script, but when it is propagated outwards from the script we don't wanna count it twice. on the other hand, if the script decides to throw an error on its own (using `redis.error_reply`), we wanna count that too. Besides, we do count the `calls` in command statistics for the commands the script calls, we we should certainly also count `failed_calls`. So when a simple `eval "return redis.call('set','x','y')" 0` fails, it should count the failed call to both SET and EVAL, but the `errorstats` and `total_error_replies` should be counted only once. The PR changes the error object that is raised on errors. Instead of raising a simple Lua string, Redis will raise a Lua table in the following format: ``` { err='<error message (including error code)>', source='<User source file name>', line='<line where the error happned>', ignore_error_stats_update=true/false, } ``` The `luaPushError` function was modified to construct the new error table as describe above. The `luaRaiseError` was renamed to `luaError` and is now simply called `lua_error` to raise the table on the top of the Lua stack as the error object. The reason is that since its functionality is changed, in case some Redis branch / fork uses it, it's better to have a compilation error than a bug. The `source` and `line` fields are enriched by the error handler (if possible) and the `ignore_error_stats_update` is optional and if its not present then the default value is `false`. If `ignore_error_stats_update` is true, the error will not be counted on the error stats. When parsing Redis call reply, each error is translated to a Lua table on the format describe above and the `ignore_error_stats_update` field is set to `true` so we will not count errors twice (we counted this error when we invoke the command). The changes in this PR might have been considered as a breaking change for users that used Lua `pcall` function. Before, the error was a string and now its a table. To keep backward comparability the PR override the `pcall` implementation and extract the error message from the error table and return it. Example of the error stats update: ``` 127.0.0.1:6379> lpush l 1 (integer) 2 127.0.0.1:6379> eval "return redis.call('get', 'l')" 0 (error) WRONGTYPE Operation against a key holding the wrong kind of value. script: e471b73f1ef44774987ab00bdf51f21fd9f7974a, on @user_script:1. 127.0.0.1:6379> info Errorstats # Errorstats errorstat_WRONGTYPE:count=1 127.0.0.1:6379> info commandstats # Commandstats cmdstat_eval:calls=1,usec=341,usec_per_call=341.00,rejected_calls=0,failed_calls=1 cmdstat_info:calls=1,usec=35,usec_per_call=35.00,rejected_calls=0,failed_calls=0 cmdstat_lpush:calls=1,usec=14,usec_per_call=14.00,rejected_calls=0,failed_calls=0 cmdstat_get:calls=1,usec=10,usec_per_call=10.00,rejected_calls=0,failed_calls=1 ``` ## error message We can now construct the error message (sent as a reply to the user) from the error table, so this solves issues where the error message was malformed and the error code appeared in the middle of the error message: ```diff 127.0.0.1:6379> eval "return redis.call('set','x','y')" 0 -(error) ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: OOM command not allowed when used memory > 'maxmemory'. +(error) OOM command not allowed when used memory > 'maxmemory' @user_script:1. Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479) ``` ```diff 127.0.0.1:6379> eval "redis.call('get', 'l')" 0 -(error) ERR Error running script (call to f_8a705cfb9fb09515bfe57ca2bd84a5caee2cbbd1): @user_script:1: WRONGTYPE Operation against a key holding the wrong kind of value +(error) WRONGTYPE Operation against a key holding the wrong kind of value script: 8a705cfb9fb09515bfe57ca2bd84a5caee2cbbd1, on @user_script:1. ``` Notica that `redis.pcall` was not change: ``` 127.0.0.1:6379> eval "return redis.pcall('get', 'l')" 0 (error) WRONGTYPE Operation against a key holding the wrong kind of value ``` ## other notes Notice that Some commands (like GEOADD) changes the cmd variable on the client stats so we can not count on it to update the command stats. In order to be able to update those stats correctly we needed to promote `realcmd` variable to be located on the client struct. Tests was added and modified to verify the changes. Related PR's: #10279, #10218, #10278, #10309 Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 23 Feb, 2022 2 commits
-
-
Itamar Haber authored
Adds the ability to track the lag of a consumer group (CG), that is, the number of entries yet-to-be-delivered from the stream. The proposed constant-time solution is in the spirit of "best-effort." Partially addresses #8737. ## Description of approach We add a new "entries_added" property to the stream. This starts at 0 for a new stream and is incremented by 1 with every `XADD`. It is essentially an all-time counter of the entries added to the stream. Given the stream's length and this counter value, we can trivially find the logical "entries_added" counter of the first ID if and only if the stream is contiguous. A fragmented stream contains one or more tombstones generated by `XDEL`s. The new "xdel_max_id" stream property tracks the latest tombstone. The CG also tracks its last delivered ID's as an "entries_read" counter and increments it independently when delivering new messages, unless the this read counter is invalid (-1 means invalid offset). When the CG's counter is available, the reported lag is the difference between added and read counters. Lastly, this also adds a "first_id" field to the stream structure in order to make looking it up cheaper in most cases. ## Limitations There are two cases in which the mechanism isn't able to track the lag. In these cases, `XINFO` replies with `null` in the "lag" field. The first case is when a CG is created with an arbitrary last delivered ID, that isn't "0-0", nor the first or the last entries of the stream. In this case, it is impossible to obtain a valid read counter (short of an O(N) operation). The second case is when there are one or more tombstones fragmenting the stream's entries range. In both cases, given enough time and assuming that the consumers are active (reading and lacking) and advancing, the CG should be able to catch up with the tip of the stream and report zero lag. Once that's achieved, lag tracking would resume as normal (until the next tombstone is set). ## API changes * `XGROUP CREATE` added with the optional named argument `[ENTRIESREAD entries-read]` for explicitly specifying the new CG's counter. * `XGROUP SETID` added with an optional positional argument `[ENTRIESREAD entries-read]` for specifying the CG's counter. * `XINFO` reports the maximal tombstone ID, the recorded first entry ID, and total number of entries added to the stream. * `XINFO` reports the current lag and logical read counter of CGs. * `XSETID` is an internal command that's used in replication/aof. It has been added with the optional positional arguments `[ENTRIESADDED entries-added] [MAXDELETEDID max-deleted-entry-id]` for propagating the CG's offset and maximal tombstone ID of the stream. ## The generic unsolved problem The current stream implementation doesn't provide an efficient way to obtain the approximate/exact size of a range of entries. While it could've been nice to have that ability (#5813) in general, let alone specifically in the context of CGs, the risk and complexities involved in such implementation are in all likelihood prohibitive. ## A refactoring note The `streamGetEdgeID` has been refactored to accommodate both the existing seek of any entry as well as seeking non-deleted entries (the addition of the `skip_tombstones` argument). Furthermore, this refactoring also migrated the seek logic to use the `streamIterator` (rather than `raxIterator`) that was, in turn, extended with the `skip_tombstones` Boolean struct field to control the emission of these. Co-authored-by:
Guy Benoish <guy.benoish@redislabs.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
Binbin authored
The test will fail on slow machines (valgrind or FreeBsd). Because in #10256 when WATCH is called on a key that's already logically expired, we will add an `expired` flag, and we will skip it in `isWatchedKeyExpired` check. Apparently we need to increase the expiration time so that the key can not expire logically then the WATCH is called. Also added retries to make sure it doesn't fail. I suppose 100ms is enough in valgrind, tested locally, no need to retry.
-
- 22 Feb, 2022 3 commits
-
-
Viktor Söderqvist authored
When WATCH is called on a key that's already logically expired, avoid discarding the transaction when the keys is actually deleted. When WATCH is called, a flag is stored if the key is already expired at the time of watch. The expired key is not deleted, only checked. When a key is "touched", if it is deleted and it was already expired when a client watched it, the client is not marked as dirty. Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
-
ranshid authored
Current implementation simple idle client which serves no traffic still use ~17Kb of memory. this is mainly due to a fixed size reply buffer currently set to 16kb. We have encountered some cases in which the server operates in a low memory environments. In such cases a user who wishes to create large connection pools to support potential burst period, will exhaust a large amount of memory to maintain connected Idle clients. Some users may choose to "sacrifice" performance in order to save memory. This commit introduce a dynamic mechanism to shrink and expend the client reply buffer based on periodic observed peak. the algorithm works as follows: 1. each time a client reply buffer has been fully written, the last recorded peak is updated: new peak = MAX( last peak, current written size) 2. during clients cron we check for each client if the last observed peak was: a. matching the current buffer size - in which case we expend (resize) the buffer size by 100% b. less than half the buffer size - in which case we shrink the buffer size by 50% 3. In any case we will **not** resize the buffer in case: a. the current buffer peak is less then the current buffer usable size and higher than 1/2 the current buffer usable size b. the value of (current buffer usable size/2) is less than 1Kib c. the value of (current buffer usable size*2) is larger than 16Kib 4. the peak value is reset to the current buffer position once every **5** seconds. we maintain a new field in the client structure (buf_peak_last_reset_time) which is used to keep track of how long it passed since the last buffer peak reset. ### **Interface changes:** **CIENT LIST** - now contains 2 new extra fields: rbs= < the current size in bytes of the client reply buffer > rbp=< the current value in bytes of the last observed buffer peak position > **INFO STATS** - now contains 2 new statistics: reply_buffer_shrinks = < total number of buffer shrinks performed > reply_buffer_expends = < total number of buffer expends performed > Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
Yoav Steinberg <yoav@redislabs.com>
-
Madelyn Olson authored
This implements the following main pieces of functionality: * Renames key spec "CHANNEL" to be "NOT_KEY", and update the documentation to indicate it's for cluster routing and not for any other key related purpose. * Add the getchannels-api, so that modules can now define commands that are subject to ACL channel permission checks. * Add 4 new flags that describe how a module interacts with a command (SUBSCRIBE, PUBLISH, UNSUBSCRIBE, and PATTERN). They are all technically composable, however not sure how a command could both subscribe and unsubscribe from a command at once, but didn't see a reason to add explicit validation there. * Add two new module apis RM_ChannelAtPosWithFlags and RM_IsChannelsPositionRequest to duplicate the functionality provided by the keys position APIs. * The RM_ACLCheckChannelPermissions (only released in 7.0 RC1) was changed to take flags rather than a boolean literal. * The RM_ACLCheckKeyPermissions (only released in 7.0 RC1) was changed to take flags corresponding to keyspecs instead of custom permission flags. These keyspec flags mimic the flags for ACLCheckChannelPermissions.
-
- 21 Feb, 2022 3 commits
-
-
Oran Agra authored
This is a followup work for #10278, and a discussion about #10279 The changes: - fix failed_calls in command stats for blocked clients that got error. including CLIENT UNBLOCK, and module replying an error from a thread. - fix latency stats for XREADGROUP that filed with -NOGROUP Theory behind which errors should be counted: - error stats represents errors returned to the user, so an error handled by a module should not be counted. - total error counter should be the same. - command stats represents execution of commands (even with RM_Call, and if they fail or get rejected it counts these calls in commandstats, so it should also count failed_calls) Some thoughts about Scripts: for scripts it could be different since they're part of user code, not the infra (not an extension to redis) we certainly want commandstats to contain all calls and errors a simple script is like mult-exec transaction so an error inside it should be counted in error stats a script that replies with an error to the user (using redis.error_reply) should also be counted in error stats but then the problem is that a plain `return redis.call("SET")` should not be counted twice (once for the SET and once for EVAL) so that's something left to be resolved in #10279
-
yoav-steinberg authored
This includes two fixes: * We forgot to count non-key reallocs in defragmentation stats. * Fix the script defrag tests so to make dict entries less signigicant in fragmentation by making the scripts larger. This assures active defrage will complete and reach desired results. Some inherent fragmentation might exists in dict entries which we need to ignore. This lead to occasional CI failures.
-
qetu3790 authored
Consider the following example: 1. geoadd k1 -0.15307903289794921875 85 n1 0.3515625 85.00019260486917005437 n2. 2. geodist k1 n1 n2 returns "4891.9380" 3. but GEORADIUSBYMEMBER k1 n1 4891.94 m only returns n1. n2 is in the boundingbox but out of search areas.So we let search areas contain boundingbox to get n2. Co-authored-by:
Binbin <binloveplay1314@qq.com>
-
- 13 Feb, 2022 1 commit
-
-
Oran Agra authored
This PR handles several aspects 1. Calls to RM_ReplyWithError from thread safe contexts don't violate thread safety. 2. Errors returning from RM_Call to the module aren't counted in the statistics (they might be handled silently by the module) 3. When a module propagates a reply it got from RM_Call to it's client, then the error statistics are counted. This is done by: 1. When appending an error reply to the output buffer, we avoid updating the global error statistics, instead we cache that error in a deferred list in the client struct. 2. When creating a RedisModuleCallReply object, the deferred error list is moved from the client into that object. 3. when a module calls RM_ReplyWithCallReply we copy the deferred replies to the dest client (if that's a real client, then that's when the error statistics are updated to the server) Note about RM_ReplyWithCallReply: if the original reply had an array with errors, and the module replied with just a portion of the original reply, and not the entire reply, the errors are currently not propagated and the errors stats will not get propagated. Fix #10180
-
- 11 Feb, 2022 2 commits
-
-
yoav-steinberg authored
Remove scripts defragger since it was broken since #10126 (released in 7.0 RC1). would crash the server if defragger starts in a server that contains eval scripts. In #10126 the global `lua_script` dict became a dict to a custom `luaScript` struct with an internal `robj` in it instead of a generic `sds` -> `robj` dict. This means we need custom code to defrag it and since scripts should never really cause much fragmentation it makes more sense to simply remove the defrag code for scripts.
-
sundb authored
The bug is introduced by #9323. (released in 7.0 RC1) The define of `REDISMODULE_OPTIONS_HANDLE_IO_ERRORS` and `REDISMODULE_OPTION_NO_IMPLICIT_SIGNAL_MODIFIED` have the same value. This will result in skipping `signalModifiedKey()` after `RM_CloseKey()` if the module has set `REDISMODULE_OPTIONS_HANDLE_REPL_ASYNC_LOAD` option. The implication is missing WATCH and client side tracking invalidations. Other changes: - add `no-implicit-signal-modified` to the options in INFO modules Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 09 Feb, 2022 1 commit
-
-
Wen Hui authored
Add test for AUTH with too many arguments
-