Avoid deferred array reply on genericZrangebyrankCommand() when consumer type is client.
I.e. any ZRANGE / ZREVRNGE (when tank is used).
This was a performance regression introduced in #7844 (v 6.2) mainly affecting pipelined workloads.
Co-authored-by: Oran Agra <oran@redislabs.com>
This is basically just a subtree pull of the latest (unreleased) hiredis.
Unfortunately, the `sds -> hisds` patch was pulled as a subtree update from a remote branch rather than a local redis change. Because of that, it goes away on every subtree update. It is now applied as a local commit so it should survive in the future.
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>
Avoid sprintf/ll2string on setDeferredAggregateLen()/addReplyLongLongWithPrefix() when we can used shared objects.
In some pipelined workloads this achieves about 10% improvement.
Co-authored-by: Oran Agra <oran@redislabs.com>
In case HELLO message received from another sentinel, with same address like another instance registered in the past but with different runid. Then there was cumbersome logic to modify the instance the port to 0 to in order to mark as invalid and later on to delete it. But the deletion is happening during update of instances in such a way that we might end up accessing an instance that was deleted just before.
Didn't find a good reason why to postpone the deletion action of an obsolete instance (deletion is taking place instantly, for other cases ) -> Lets delete at once
There is a mixture of logic of Sentinel address update with the logic of deletion of Sentinels that match a given Address -> Split to two!
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.
There are scenarios where it results in many small objects in the reply list,
such as commands heavily using deferred array replies (`addReplyDeferredLen`).
E.g. what COMMAND command and CLUSTER SLOTS used to do (see #10056, #7123),
but also in case of a transaction or a pipeline of commands that use just one differed array reply.
We used to have to run multiple loops along with multiple calls to `write()` to send data back to
peer based on the current code, but by means of `writev()`, we can gather those scattered
objects in reply list and include the static reply buffer as well, then send it by one system call,
that ought to achieve higher performance.
In the case of TLS, we simply check and concatenate buffers into one big buffer and send it
away by one call to `connTLSWrite()`, if the amount of all buffers exceeds `NET_MAX_WRITES_PER_EVENT`,
then invoke `connTLSWrite()` multiple times to avoid a huge massive of memory copies.
Note that aside of reducing system calls, this change will also reduce the amount of
small TCP packets sent.
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>
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>
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.
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>
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
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.
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>
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 3dad819.
2. Use `LL_WARNING` log level for some error logs (I/O error, Connection failed).
3. Fix typos that saw by the way.
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.
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
* 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.
Reapply this commit on top of hiredis as a local change. Previosuly it
was pulled from a private hiredis branch, which resulted with it going
away on subtree pull.
f8de9a4bd Merge pull request #1046 from redis/rockylinux-ci
a41c9bc8b CentOS 8 is EOL, switch to RockyLinux
be41ed60d Avoid incorrect call to the previous reply's callback (#1040)
f2e8010d9 fix building on AIX and SunOS (#1031)
e73ab2f23 Add timeout support for libuv adapter (#1016)
f2ce5980e Allow sending commands after sending an unsubscribe (#1036)
ff860e55d Correction for command timeout during pubsub (#1038)
24d534493 CMakeLists.txt: allow building without a C++ compiler (#872)
4ece9a02e Fix adapters/libevent.h compilation for 64-bit Windows (#937)
799edfaad Don't link with crypto libs if USE_SSL isn't set.
f74b08182 Makefile: move SSL options into a block and refine rules
f347743b7 Update CMakeLists.txt for more portability (#1005)
f2be74802 Fix integer overflow when format command larger than 4GB (#1030)
58aacdac6 Handle array response in parallell with pubsub using RESP3 (#1014)
d3384260e Support PING while subscribing (RESP2) (#1027)
e3a479e40 FreeBSD build fixes + CI (#1026)
da5a4ff36 Add asynchronous test for pubsub using RESP3 (#1012)
b5716ee82 Valgrind returns error exit code when errors found (#1011)
1aed21a8c Move to using make directly in Cygwin (#1020)
a83f4b890 Correct CMake warning for libevent adapter example
c4333203e Remove unused parameter warning in libev adapter
7ad38dc4a Small tweaks of the async tests
4021726a6 Add asynchronous test for pubsub using RESP2
648763c36 Add build options for enabling async tests
c98c6994d Correcting the build target `coverage` for enabled SSL (#1009)
30ff8d850 Run SSL tests in CI
4a126e8a9 Add valgrind and CMake to tests
b73c2d410 Add Centos8
e9f647384 We should run actions on PRs
6ad4ccf3c Add Cygwin build test
783a3789c Add Windows tests in GitHub actions
0cac8dae1 Switch to GitHub actions
fa900ef76 Fix unused variable warning.
e489846b7 Minor refactor of CVE-2021-32765 fix.
51c740824 Remove extra comma from cmake var. Or it'll be treated as part of the var name.
632bf0718 Merge branch 'release/v1.0.2'
b73128324 Prepare for v1.0.2 GA
d4e6f109a Revert erroneous SONAME bump
a39824a5d Merge branch 'release/v1.0.1'
8d1bfac46 Prepare for v1.0.1 GA
76a7b1000 Fix for integer/buffer overflow CVE-2021-32765
9eca1f36f Allow to override OPENSSL_PREFIX in Linux
2d9d77518 Don't leak memory if an invalid type is set (#906)
f5f31ff9b Added REDIS_NO_AUTO_FREE_REPLIES flag (#962)
5850a8ecd Ensure we curry any connect error to an async context.
b6f86f38c Fix README.md
667dbf536 Merge pull request #935 from kristjanvalur/pr5
9bf6c250e Merge pull request #939 from zmartzone/improve_pr_896_ssl_leak
959af9760 Merge pull request #949 from plan-do-break-fix/Typo-corrections
0743f57bb fix(docs): corrects typos in project README
5f4382247 improve SSL leak fixredis/hiredis#896
e06ecf7e4 Ignore timeout callback from a successful connect
dfa33e60b Change order independant push logic to not change behavior.
6204182aa Handle the case where an invalidation is sent second.
d6a0b192b Merge branch 'reader-updates'
410c24d2a Fix off-by-one error in seekNewline
bd7488d27 read: Validate line items prior to checking for object creation callbacks
5f9242a1f read: Remove obsolete comment on nested multi bulk depth limitation
83c145042 read: Add support for the RESP3 bignum type
c6646cb19 read: Ensure no invalid '\r' or '\n' in simple status/error strings
e43061156 read: Additional validation and test case for RESP3 double
c8adea402 redisReply: Fix parent type assertions during double, nil, bool creation
ff73f1f9e redisReply: Explicitly list nil and bool cases in freeReplyObject() switch.
0f9251884 test: Add test case for RESP3 set
33c06dd50 test: Add test case for RESP3 map
397fe2630 read: Use memchr() in seekNewline() instead of looping over entire string
81c48a982 test: Add test cases for RESP3 bool
51e693f4f read: Add additional RESP3 bool validation
790b4d3b4 test: Add test cases for RESP3 nil
d8899fbc1 read: Add additional RESP3 nil validation
96e8ea611 test: Add test cases for infinite and NaN doubles
f913e9b99 read: Fix double validation and infinity parsing
8039c7d26 test: Add test case for doubles
49539fd1a redisReply: Fix - set len in double objects
53a8144c8 Merge pull request #924 from cheese1/master
9390de006 http -> https
7d99b5635 Merge pull request #917 from Nordix/stack-alloc-dict-iter
4bba72103 Handle OOM during async command callback registration
920128a26 Stack allocate dict iterators
297ecbecb Tiny formatting changes + suppress implicit memcpy warning
f746a28e7 Removed 2 typecasts
940a04f4d Added fuzzer
e4a200040 Merge pull request #896 from ayeganov/bugfix/ssl_leak
aefef8987 Free SSL object when redisSSLConnect fails
e3f88ebcf Merge pull request #894 from jcohen02/fix/issue893
308ffcab8 Updating SSL connection example
297f6551d Merge pull request #889 from redis/wincert
e7dda9785 Formatting
f44945a0a Merge pull request #874 from masariello/position-independent-code
74e78498c Merge pull request #888 from michael-grunder/nil-push-invalidation
b9b9f446f Fix handling of NIL invalidation messages.
acc917548 Merge pull request #885 from gkorland/patch-1
b086f763e clean a warning, remvoe empty else block
b47fae4e7 Merge pull request #881 from timgates42/bugfix_typo_terminated
f989670e5 docs: Fix simple typo, termined -> terminated
773d6ea8a Copy error to redisAsyncContext on timeout
e35300a66 add pdb files to packages for MSVC builds
dde6916b4 Add d suffix to debug libraries so that can packaged together with optimized builds (Release, RelWithDebInfo, etc)
3b68b5018 Enable position-independent code
6693863f4 Add support for system CA certificate store on Windows
2a5a57b90 Remove whitespace
1b40ec509 fixed issue with unit test linking on windows with SSL
d7b1d21e8 Merge branch 'master' of github.com:redis/hiredis
fb0e6c0dd Merge pull request #870 from michael-grunder/cmake-c99
13a35bdb6 Explicitly set c99 in CMake
bea137ca9 Merge pull request #868 from michael-grunder/fix-sockaddr-typo
bd6f86eb6 Fix sockaddr typo
48696e7e5 Don't use non-installed win32.h helper in examples (#863)
faa1c4863 Merge tag 'v1.0.0'
5003906d6 Define a no op assert if we detect NDEBUG (#861)
ea063b7cc Use development specific versions in master
04a27f480 We can run SSL tests everywhere except mingw/Windows (#859)
8966a1fc2 Remove extra whitespace (#858)
34b7f7a0f Keep libev's code style (#857)
07c3618ff Add static library target and cpack support
REVERT: 00272d669 Rename sds calls so they don't conflict in Redis.
git-subtree-dir: deps/hiredis
git-subtree-split: f8de9a4bd433791890572f7b9147e685653ddef9
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.
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
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)
```
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.
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>
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".
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>
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
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.
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
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>
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 d0bc4fff18/src/function_lua.c (L472-L485)
and d0bc4fff18/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 d0bc4fff18/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.
```
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.
- 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
