- 22 Feb, 2022 5 commits
-
-
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.
-
Binbin authored
Remove ALLOW_BUSY from REPLICAOF and add it to REPLCONF. Add DEPRECATED doc flag to SLAVEOF, mark it as deprecated. (#10315) * Remove ALLOW_BUSY from REPLICAOF and add it to REPLCONF * mark SLAVEOF as deprecated
-
chenyang8094 authored
Since we didn't copy the null terminator to temp_filepath, dirname could return the wrong result.
-
YaacovHazan authored
Make sure the status return from loading multiple AOF files reflects the overall result, not just the one of the last file. When one of the AOF files succeeded to load, but the last AOF file was empty, the loadAppendOnlyFiles will return AOF_EMPTY. This commit changes this behavior, and return AOF_OK in that case. This can happen for example, when loading old AOF file, and no more commands processed, the manifest file will include base AOF file with data, and empty incr AOF file. Co-authored-by:
chenyang8094 <chenyang8094@users.noreply.github.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 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>
-
- 20 Feb, 2022 2 commits
-
-
rangerzhang authored
There is no variable named by REPL_STATE_RECEIVE_PSYNC_REPLY, it should be REPL_STATE_RECEIVE_PSYNC_REPLY according to the contexts.
-
Binbin authored
publishshard was added in #8621 (7.0 RC1), but the publishshard_sent stat is not shown in CLUSTER INFO command. Other changes: 1. Remove useless `needhelp` statements, it was removed in 3dad8196. 2. Use `LL_WARNING` log level for some error logs (I/O error, Connection failed). 3. Fix typos that saw by the way.
-
- 17 Feb, 2022 2 commits
-
-
yoav-steinberg authored
Add aof_rewrites and rdb_snapshots counters to info. This is useful to figure our if a rewrite or snapshot happened since last check. This was part of the (ongoing) effort to provide a safe backup solution for multipart-aof backups.
-
chenyang8094 authored
Modifications of this PR: 1. Support the verification of `Multi Part AOF`, while still maintaining support for the old-style `AOF/RDB-preamble`. `redis-check-aof` will automatically choose which mode to use according to the incoming file format. `Usage: redis-check-aof [--fix|--truncate-to-timestamp $timestamp] <AOF/manifest>` 2. Refactor part of the code to make it easier to understand 3. Currently only supports truncate (`--fix` or `--truncate-to-timestamp`) the last AOF file (may be `BASE` or `INCR`) The reasons for 3 above: - for `--fix`: Only the last AOF may be truncated, this is guaranteed by redis - for `--truncate-to-timestamp`: Normally, we only have `BASE` + `INCR` files at most, and `BASE` cannot be truncated(It only contains a timestamp annotation at the beginning of the file), so only `INCR` can be truncated. If we have a `BASE+INCR1+INCR2` file (meaning we have an interrupted AOFRW), Only `INCR2` files can be truncated at this time. If we still insist on truncate `INCR1`, we need to manually delete `INCR2` and update the manifest file, then re-run `redis-check-aof` - If we want to support truncate any file, we need to add very complicated code to support the atomic modification of multiple file deletion and update manifest, I think this is unnecessary
-
- 16 Feb, 2022 2 commits
-
-
Ping Xie authored
* Provide a fallback static_assert implementation * Use sds for clusterNode.hostname
-
Matteo Baccan authored
-
- 15 Feb, 2022 1 commit
-
-
Yossi Gottlieb authored
* Drop obsolete initialization calls. * Use decoder API for DH parameters. * Enable auto DH parameters if not explicitly used, which should be the preferred configuration going forward.
-
- 14 Feb, 2022 1 commit
-
-
YaacovHazan authored
In order to make sure no more commands processed, we wait that the 'load handlers' will disconncet. The test by mistake waited on the (last) slave instead of the master.
-
- 13 Feb, 2022 3 commits
-
-
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
-
Oran Agra authored
sometimes you just wanna run one test on one system (e.g. memefficiency on macos), so you want all other tests to be skipped
-
Binbin authored
Added regression tests for #10020 / #10081 / #10243. The above PRs fixed some crashes due to an asserting, see function `clientHasPendingReplies` (introduced in #9166). This commit added some tests to cover the above scenario. These tests will all fail in #9166, althought fixed not, there is value in adding these tests to cover and verify the changes. And it also can cover #8868 (verify the logs). Other changes: 1. Reduces the wait time in `waitForBgsave` and `waitForBgrewriteaof` from 1s to 50ms, which should reduce the time for some tests. 2. Improve the test infra to print context when `assert_match` fails. 3. Improve the test infra to print `$error` when `assert_error` fails. ``` Expected an error matching 'ERR*' but got 'OK' (context: type eval line 4 cmd {assert_error "ERR*" {r set a b}} proc ::test) ```
-
- 11 Feb, 2022 4 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>
-
chenyang8094 authored
In multi-part aof, We no longer have the concept of `RDB-preamble`, so the related logs should be removed. However, in order to print compatible logs when loading old-style AOFs, we also have to keep the relevant code. Additionally, when saving an RDB, change the RDB aux field from "aof-preamble" to "aof-base".
-
Harkrishn Patro authored
-
- 10 Feb, 2022 2 commits
-
-
Omer Shadmi authored
Use binary representation for key values dumped crash to log, so that if they contain null chars they're still printed correctly. Additionally limit their length to 128 chars Co-authored-by:
Oran Agra <oran@redislabs.com>
-
Pengcheng Huang authored
Co-authored-by:
yoav-steinberg <yoav@monfort.co.il>
-
- 09 Feb, 2022 4 commits
-
-
Wen Hui authored
Add test for AUTH with too many arguments
-
chenyang8094 authored
append for PR #9812
-
Binbin authored
* Fix INFO SENTINEL memory leak Introduced in #6891 * remove the copy-paste sentence
-
mowenliunian authored
Fixed some syntax errors in the comments
-
- 08 Feb, 2022 8 commits
-
-
Oran Agra authored
The theory is that a replica gets disconnected from within REPLCONF ACK, so when we go up the stack, we'll crash when attempting to access c->cmd->flags
-
gms authored
There are two issues in SENTINEL DEBUG: 1. The error message should mention SENTINEL DEBUG 2. Add missing reuturn in args parse. ``` redis> sentinel debug INFO-PERIOD aaa (error) ERR Invalid argument 'aaa' for SENTINEL SET 'INFO-PERIOD' redis> sentinel debug a b c d (error) ERR Unknown option or number of arguments for SENTINEL SET 'a' redis> ping (error) ERR Unknown option or number of arguments for SENTINEL SET 'b' ``` Introduced in #9291. Also do some cleanups in the code.
-
weiguo authored
-
Viktor Söderqvist authored
There's an assertion added recently to make sure that non-write commands don't use lookupKeyWrite, It was initially meant to be used only on read-only replicas, but we thought it'll not have enough coverage, so used it on the masters too. We now realize that in some cases this can cause issues for modules, so we remove the assert. Other than that, we also make sure not to force expireIfNeeded on read-only replicas. even if they somehow run a write command. See https://github.com/redis/redis/pull/9572#discussion_r800179373
-
Wen Hui authored
This is an enhancement for INFO command, previously INFO only support one argument for different info section , if user want to get more categories information, either perform INFO all / default or calling INFO for multiple times. **Description of the feature** The goal of adding this feature is to let the user retrieve multiple categories via the INFO command, and still avoid emitting the same section twice. A use case for this is like Redis Sentinel, which periodically calling INFO command to refresh info from monitored Master/Slaves, only Server and Replication part categories are used for parsing information. If the INFO command can return just enough categories that client side needs, it can save a lot of time for client side parsing it as well as network bandwidth. **Implementation** To share code between redis, sentinel, and other users of INFO (DEBUG and modules), we have a new `genInfoSectionDict` function that returns a dict and some boolean flags (e.g. `all`) to the caller (built from user input). Sentinel is later purging unwanted sections from that, and then it is forwarded to the info `genRedisInfoString`. **Usage Examples** INFO Server Replication INFO CPU Memory INFO default commandstats Co-authored-by:
Oran Agra <oran@redislabs.com>
-
yoav-steinberg authored
This PR handles inconsistencies in errors returned from lua scripts. Details of the problem can be found in #10165. ### Changes - Remove double stack trace. It's enough that a stack trace is automatically added by the engine's error handler see https://github.com/redis/redis/blob/d0bc4fff18afdf9e5421cc88e23ffbb876ecaec3/src/function_lua.c#L472-L485 and https://github.com/redis/redis/blob/d0bc4fff18afdf9e5421cc88e23ffbb876ecaec3/src/eval.c#L243-L255 - Make sure all errors a preceded with an error code. Passing a simple string to `luaPushError()` will prepend it with a generic `ERR` error code. - Make sure lua error table doesn't include a RESP `-` error status. Lua stores redis error's as a lua table with a single `err` field and a string. When the string is translated back to RESP we add a `-` to it. See https://github.com/redis/redis/blob/d0bc4fff18afdf9e5421cc88e23ffbb876ecaec3/src/script_lua.c#L510-L517 So there's no need to store it in the lua table. ### Before & After ```diff --- <unnamed> +++ <unnamed> @@ -1,14 +1,14 @@ 1: config set maxmemory 1 2: +OK 3: eval "return redis.call('set','x','y')" 0 - 4: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: @user_script: 1: -OOM command not allowed when used memory > 'maxmemory'. + 4: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: OOM command not allowed when used memory > 'maxmemory'. 5: eval "return redis.pcall('set','x','y')" 0 - 6: -@user_script: 1: -OOM command not allowed when used memory > 'maxmemory'. + 6: -OOM command not allowed when used memory > 'maxmemory'. 7: eval "return redis.call('select',99)" 0 8: -ERR Error running script (call to 4ad5abfc50bbccb484223905f9a16f09cd043ba8): @user_script:1: ERR DB index is out of range 9: eval "return redis.pcall('select',99)" 0 10: -ERR DB index is out of range 11: eval_ro "return redis.call('set','x','y')" 0 -12: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: @user_script: 1: Write commands are not allowed from read-only scripts. +12: -ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: ERR Write commands are not allowed from read-only scripts. 13: eval_ro "return redis.pcall('set','x','y')" 0 -14: -@user_script: 1: Write commands are not allowed from read-only scripts. +14: -ERR Write commands are not allowed from read-only scripts. ```
-
guybe7 authored
Fix #7021 #8924 #10198 # Intro Before this commit X[AUTO]CLAIM used to transfer deleted entries from one PEL to another, but reply with "nil" for every such entry (instead of the entry id). The idea (for XCLAIM) was that the caller could see this "nil", realize the entry no longer exists, and XACK it in order to remove it from PEL. The main problem with that approach is that it assumes there's a correlation between the index of the "id" arguments and the array indices, which there isn't (in case some of the input IDs to XCLAIM never existed/read): ``` 127.0.0.1:6379> XADD x 1 f1 v1 "1-0" 127.0.0.1:6379> XADD x 2 f1 v1 "2-0" 127.0.0.1:6379> XADD x 3 f1 v1 "3-0" 127.0.0.1:6379> XGROUP CREATE x grp 0 OK 127.0.0.1:6379> XREADGROUP GROUP grp Alice COUNT 2 STREAMS x > 1) 1) "x" 2) 1) 1) "1-0" 2) 1) "f1" 2) "v1" 2) 1) "2-0" 2) 1) "f1" 2) "v1" 127.0.0.1:6379> XDEL x 1 2 (integer) 2 127.0.0.1:6379> XCLAIM x grp Bob 0 0-99 1-0 1-99 2-0 1) (nil) 2) (nil) ``` # Changes Now, X[AUTO]CLAIM acts in the following way: 1. If one tries to claim a deleted entry, we delete it from the PEL we found it in (and the group PEL too). So de facto, such entry is not claimed, just cleared from PEL (since anyway it doesn't exist in the stream) 2. since we never claim deleted entries, X[AUTO]CLAIM will never return "nil" instead of an entry. 3. add a new element to XAUTOCLAIM's response (see below) # Knowing which entries were cleared from the PEL The caller may want to log any entries that were found in a PEL but deleted from the stream itself (it would suggest that there might be a bug in the application: trimming the stream while some entries were still no processed by the consumers) ## XCLAIM the set {XCLAIM input ids} - {XCLAIM returned ids} contains all the entry ids that were not claimed which means they were deleted (assuming the input contains only entries from some PEL). The user doesn't need to XACK them because XCLAIM had already deleted them from the source PEL. ## XAUTOCLAIM XAUTOCLAIM has a new element added to its reply: it's an array of all the deleted stream IDs it stumbled upon. This is somewhat of a breaking change since X[AUTO]CLAIM used to be able to reply with "nil" and now it can't... But since it was undocumented (and generally a bad idea to rely on it, as explained above) the breakage is not that bad.
-
Oran Agra authored
- add COMMAND GETKEYSANDFLAGS sub-command - add RM_KeyAtPosWithFlags and GetCommandKeysWithFlags - RM_KeyAtPos and RM_CreateCommand set flags requiring full access for keys - RM_CreateCommand set VARIABLE_FLAGS - expose `variable_flags` flag in COMMAND INFO key-specs - getKeysFromCommandWithSpecs prefers key-specs over getkeys-api - add tests for all of these
-
- 07 Feb, 2022 3 commits
-
-
ivanstosic-janea authored
The protocol error was caused by the buggy `writeHandler` in `redis-benchmark.c`, which didn't handle one of the cases, thereby repeating data, leading to protocol errors when the values being sent are very long. This PR fixes #10233, issue introduced by #7959
-
Avital-Fine authored
-
Binbin authored
If summary or since is empty, we used to return NULL in COMMAND DOCS. Currently all redis commands will have these two fields. But not for module command, summary and since are optional for RM_SetCommandInfo. With the change in #10043, if a module command doesn't have the summary or since, redis-cli will crash (see #10250). In this commit, COMMAND DOCS avoid adding summary or since when they are missing.
-