- 30 Aug, 2023 3 commits
-
-
bodong.ybd authored
Before: ``` 127.0.0.1:6379> command getkeys sort_ro key (empty array) 127.0.0.1:6379> ``` After: ``` 127.0.0.1:6379> command getkeys sort_ro key 1) "key" 127.0.0.1:6379> ```
-
Chen Tianjie authored
Add these INFO metrics: * client_query_buffer_limit_disconnections * client_output_buffer_limit_disconnections Sometimes it is useful to monitor whether clients reaches size limit of query buffer and output buffer, to decide whether we need to adjust the buffer size limit or reduce client query payload.
-
nihohit authored
Since the three commands have similar behavior (change config, return OK), the tips that govern how they should behave should be similar. Co-authored-by:
Shachar Langbeheim <shachlan@amazon.com>
-
- 27 Aug, 2023 1 commit
-
-
Binbin authored
This test failed several times: ``` *** [err]: LATENCY GRAPH can output the event graph in tests/unit/latency-monitor.tcl Expected '478' to be more than or equal to '500' (context: type eval line 8 cmd {assert_morethan_equal $high 500} proc ::test) ``` Not sure why, adding some verbose printing that'll print the command result on the next time.
-
- 22 Aug, 2023 1 commit
-
-
Danilo Bargen authored
warning against editing the config file and restarting the server. which will attempt to load an AOF file and disregard the RDB. Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 21 Aug, 2023 4 commits
-
-
Binbin authored
BITCOUNT and BITPOS with non-existing key will return 0 even the arguments are error, before this commit: ``` > flushall OK > bitcount s 0 (integer) 0 > bitpos s 0 0 1 hello (integer) 0 > set s 1 OK > bitcount s 0 (error) ERR syntax error > bitpos s 0 0 1 hello (error) ERR syntax error ``` The reason is that we judged non-existing before parameter checking and returned. This PR fixes it, and after this commit: ``` > flushall OK > bitcount s 0 (error) ERR syntax error > bitpos s 0 0 1 hello (error) ERR syntax error ``` Also BITPOS made the same fix as #12394, check for wrong argument, before checking for key. ``` > lpush mylist a b c (integer) 3 > bitpos mylist 1 a b (error) WRONGTYPE Operation against a key holding the wrong kind of value ```
-
Wen Hui authored
Generally, In any command we first check for the argument and then check if key exist. Some of the examples are ``` 127.0.0.1:6379> getrange no-key invalid1 invalid2 (error) ERR value is not an integer or out of range 127.0.0.1:6379> setbit no-key 1 invalid (error) ERR bit is not an integer or out of range 127.0.0.1:6379> xrange no-key invalid1 invalid2 (error) ERR Invalid stream ID specified as stream command argument ``` **Before change** ``` bitcount no-key invalid1 invalid2 0 ``` **After change** ``` bitcount no-key invalid1 invalid2 (error) ERR value is not an integer or out of range ```
-
Binbin authored
Limit the range of LREM count to -LONG_MAX ~ LONG_MAX. Before the fix, passing -LONG_MAX would cause an overflow and would effectively be the same as passing 0. (Because this condition `toremove && removed == toremove `can never be satisfied). This is a minor fix as it shouldn't really affect users, more like a cleanup.
-
Yves LeBras authored
Initializing `memkeys` to 0 for consistency and clarity. the config struct is anyway zeroed, but other fields are explicitly initialized.
-
- 20 Aug, 2023 2 commits
-
-
Wen Hui authored
In our test case, now we missed some test coverage for client sub-commands. This pr goal is to add some test coverage cases of the following commands: Client caching Client kill Client no-evict Client pause Client reply Client tracking Client setname At the very least, this is useful to make sure there are no leaks and crashes in these code paths.
-
meiravgri authored
This PR purpose is to make the crash report process thread safe. main changes include: 1. `setupSigSegvHandler()` is introduced to initialize the signal handler. This function first initializes the signal handler mutex (if not initialized yet) and then registers the process to the signal handler. 2. **sigsegvHandler** flags : SA_NODEFER - don't add the signal to the process signal mask. We use this flag because we want to be able to handle a second call to the signal manually. removed SA_RESETHAND: this flag resets the signal handler function upon the first entrance to the registered function. The reason to use this flag is to protect from recursively entering the signal handler by the same thread. But, it also means that if a second thread crashes while handling a signal, the process will be terminated immediately and we won't get the crash report. In this PR we discard this flag. The signal handler guard described below purpose is to solve the above issues. 3. Add a **signal handler lock** with ERRORCHECK attributes. The lock's purpose is to ensure that only one thread generates a crash report. Once a second thread enters the signal handler it will be blocked. We use the ERRORCHECK lock in order to protect from possible deadlock in case the thread handling the crash gets a signal. In the latest scenario, we log what we have collected until the handler crashed. At the end of the crash report we reset the signal handler SIG_DFL, with no flags, and rethrow the signal to generate a core dump (if enabled) and exit the process. During the work on this PR we wanted to understand the historical reasons for how crash is handled. With respect to the choice of the flag, we believe the **SA_RESETHAND** was not added for any specific purpose. **SA_ONSTACK** which is removed here from bugReportEnd(), was originally also set in the initial registration to signal handler, but removed in 3ada43e7. In addition, it was removed from another location in deee2c1e with the following description, which is also relevant to why it should be removed from bugReportEnd: > it seems to be some valgrind bug with SA_ONSTACK. > SA_ONSTACK seems unneeded since WD is not recursive (SA_NODEFER was removed), > also, not sure if it's even valid without a call to sigaltstack()
-
- 16 Aug, 2023 5 commits
-
-
Binbin authored
In the past we hardcoded it to 20, causing it to not count keys for more databases.
-
Tyler Bream (Event pipeline) authored
When running cluster info, and the number of keys overflows the integer value, the summary no longer makes sense. This fixes by using an appropriate type to handle values over the max int value.
-
WangYu authored
If dict is rehashing, the entries in the head of table[0] is moved to table[1] and all entries in `table[0][0:rehashidx]` is NULL. `dictNext` start looking for non-NULL entry from table 0 index 0, and the first call of `dictNext` on a rehashing dict will Iterate many times to skip those NULL entries. We can easily skip those entries by setting `iter->index` as `iter->d->rehashidx` when dict is rehashing and it's the first call of dictNext (`iter->index == -1 && iter->table == 0`). Co-authored-by:
sundb <sundbcn@gmail.com>
-
Wen Hui authored
Currently rdbSaveMillisecondTime, rdbSaveDoubleValue api's return type is int but they return the value directly from rdbWriteRaw function which has the return type of ssize_t. As this may cause overflow to int so changed to ssize_t.
-
Oran Agra authored
Add 7.2, drop 6.0 as per https://redis.io/docs/about/releases/ Also replace a few concordances of the `’` char, with standard `'`
-
- 15 Aug, 2023 1 commit
-
-
Binbin authored
We iterate over all replicas to get the result, the time complexity should be O(N), like CLUSTER NODES complexity is O(N).
-
- 10 Aug, 2023 2 commits
-
-
Madelyn Olson authored
When adding a new ACL rule was added, an attempt was made to remove any "overlapping" rules. However, there when a match was found, the search was not resumed at the right location, but instead after the original position of the original command. For example, if the current rules were `-config +config|get` and a rule `+config` was added. It would identify that `-config` was matched, but it would skip over `+config|get`, leaving the compacted rule `-config +config`. This would be evaluated safely, but looks weird. This bug can only be triggered with subcommands, since that is the only way to have sequential matching rules. Resolves #12470. This is also only present in 7.2. I think there was also a minor risk of removing another valid rule, since it would start the search of the next command at an arbitrary point. I couldn't find a valid offset that would have cause a match using any of the existing commands that have subcommands with another command.
-
Binbin authored
After SENTINEL RESET, sometimes the sentinel can sense the master again, causing the test to fail. Here we give it a few more chances.
-
- 05 Aug, 2023 4 commits
-
-
zhaozhao.zz authored
if there are no subscribers, we can ignore the operation
-
zhaozhao.zz authored
Fix the assertion when a busy script (timeout) signal ready keys (like LPUSH), and then an arbitrary client's `allow-busy` command steps into `handleClientsBlockedOnKeys` try wake up clients blocked on keys (like BLPOP). Reproduction process: 1. start a redis with aof `./redis-server --appendonly yes` 2. exec blpop `127.0.0.1:6379> blpop a 0` 3. use another client call a busy script and this script push the blocked key `127.0.0.1:6379> eval "redis.call('lpush','a','b') while(1) do end" 0` 4. user a new client call an allow-busy command like auth `127.0.0.1:6379> auth a` BTW, this issue also break the atomicity of script. This bug has been around for many years, the old versions only have the atomic problem, only 7.0/7.2 has the assertion problem. Co-authored-by:
Oran Agra <oran@redislabs.com>
-
sundb authored
This PR mainly fixes a possible integer overflow in `json_append_string()`. When we use `cjson.encoding()` to encode a string larger than 2GB, at specific compilation flags, an integer overflow may occur leading to truncation, resulting in the part of the string larger than 2GB not being encoded. On the other hand, this overflow doesn't cause any read or write out-of-range or segment fault. 1) using -O0 for lua_cjson (`make LUA_DEBUG=yes`) In this case, `i` will overflow and leads to truncation. When `i` reaches `INT_MAX+1` and overflows to INT_MIN, when compared to len, `i` (1000000..00) is expanded to 64 bits signed integer (1111111.....000000) . At this point i will be greater than len and jump out of the loop, so `for (i = 0; i < len; i++)` will loop up to 2^31 times, and the part of larger than 2GB will be truncated. ```asm `i` => -0x24(%rbp) <+253>: addl $0x1,-0x24(%rbp) ; overflow if i large than 2^31 <+257>: mov -0x24(%rbp),%eax <+260>: movslq %eax,%rdx ; move a 32-bit value with sign extension into a 64-bit signed <+263>: mov -0x20(%rbp),%rax <+267>: cmp %rax,%rdx ; check `i < len` <+270>: jb 0x212600 <json_append_string+148> ``` 2) using -O2/-O3 for lua_cjson (`make LUA_DEBUG=no`, **the default**) In this case, because singed integer overflow is an undefined behavior, `i` will not overflow. `i` will be optimized by the compiler and use 64-bit registers for all subsequent instructions. ```asm <+180>: add $0x1,%rbx ; Using 64-bit register `rbx` for i++ <+184>: lea 0x1(%rdx),%rsi <+188>: mov %rsi,0x10(%rbp) <+192>: mov %al,(%rcx,%rdx,1) <+195>: cmp %rbx,(%rsp) ; check `i < len` <+199>: ja 0x20b63a <json_append_string+154> ``` 3) using 32bit Because `strbuf_ensure_empty_length()` preallocates memory of length (len * 6 + 2), in 32-bit `cjson.encode()` can only handle strings smaller than ((2 ^ 32) - 3 ) / 6. So 32bit is not affected. Also change `i` in `strbuf_append_string()` to `size_t`. Since its second argument `str` is taken from the `char2escape` string array which is never larger than 6, so `strbuf_append_string()` is not at risk of overflow (the bug was unreachable).
-
Binbin authored
GEOHASH / GEODIST / GEOPOS use zsetScore to get the score, in skiplist encoding, we use dictFind to get the score, which is O(1), same as ZSCORE command. It is not clear why these commands had O(Log(N)), and O(N) until now.
-
- 02 Aug, 2023 2 commits
-
-
Meir Shpilraien (Spielrein) authored
Ensure that the function load timeout is disabled during loading from RDB/AOF and on replicas. (#12451) When loading a function from either RDB/AOF or a replica, it is essential not to fail on timeout errors. The loading time may vary due to various factors, such as hardware specifications or the system's workload during the loading process. Once a function has been successfully loaded, it should be allowed to load from persistence or on replicas without encountering a timeout failure. To maintain a clear separation between the engine and Redis internals, the implementation refrains from directly checking the state of Redis within the engine itself. Instead, the engine receives the desired timeout as part of the library creation and duly respects this timeout value. If Redis wishes to disable any timeout, it can simply send a value of 0.
-
zhaozhao.zz authored
When doing merge selector, we should check whether the merge has started (i.e., whether open_bracket_start is -1) every time. Otherwise, encountering an illegal selector pattern could succeed and also cause memory leaks, for example: ``` acl setuser test1 (+PING (+SELECT (+DEL ) ``` The above would leak memory and succeed with only DEL being applied, and would now error after the fix. Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 01 Aug, 2023 1 commit
-
-
Diego Lopez Recas authored
Changing the masterauth while turning into a replica is racy. Turn into replica after changing the masterauth instead.
-
- 30 Jul, 2023 1 commit
-
-
DarrenJiang13 authored
In the tcl foreach loop, the function should compare line rather than the whole file.
-
- 25 Jul, 2023 4 commits
-
-
Harkrishn Patro authored
Additional test coverage for incr/decr operation. integer number could be present in raw encoding format due to operation like append. A incr/decr operation following it optimize the string to int encoding format.
-
zhaozhao.zz authored
A bug introduced in #11657 (7.2 RC1), causes client-eviction (#8687) and INFO to have inaccurate memory usage metrics of MONITOR clients. Because the type in `c->type` and the type in `getClientType()` are confusing (in the later, `CLIENT_TYPE_NORMAL` not `CLIENT_TYPE_SLAVE`), the comment we wrote in `updateClientMemUsageAndBucket` was wrong, and in fact that function didn't skip monitor clients. And since it doesn't skip monitor clients, it was wrong to delete the call for it from `replicationFeedMonitors` (it wasn't a NOP). That deletion could mean that the monitor client memory usage is not always up to date (updated less frequently, but still a candidate for client eviction).
-
nihohit authored
changing the response and request policy of a few commands, see https://redis.io/docs/reference/command-tips 1. RANDOMKEY used to have no response policy, which means that when sent to multiple shards, the responses should be aggregated. this normally applies to commands that return arrays, but since RANDOMKEY replies with a simple string, it actually requires a SPECIAL response policy (for the client to select just one) 2. SCAN used to have no response policy, but although the key names part of the response can be aggregated, the cursor part certainly can't. 3. MSETNX had a request policy of MULTI_SHARD and response policy of AGG_MIN, but in fact the contract with MSETNX is that when one key exists, it returns 0 and doesn't set any key, routing it to multiple shards would mean that if one failed and another succeeded, it's atomicity is broken and it's impossible to return a valid response to the caller. Co-authored-by:
Shachar Langbeheim <shachlan@amazon.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
Harkrishn Patro authored
Add test coverage to validate object encoding update on APPEND command usage on a integer value
-
- 20 Jul, 2023 1 commit
-
-
Makdon authored
When the redis server cluster running on cluster-preferred-endpoint-type unknown-endpoint mode, and receive a request that should be redirected to another redis server node, it does not reply the hostip, but a empty host like MOVED 3999 :6381. The redis-cli would try to connect to an address without a host, which cause the issue: ``` 127.0.0.1:7002> set bar bar -> Redirected to slot [5061] located at :7000 Could not connect to Redis at :7000: No address associated with hostname Could not connect to Redis at :7000: No address associated with hostname not connected> exit ``` In this case, the redis-cli should use the previous hostip when there's no host provided by the server. --------- Co-authored-by:
Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by:
Madelyn Olson <madelynolson@gmail.com>
-
- 18 Jul, 2023 1 commit
-
-
Oran Agra authored
Replaced comment with excessive warning.
-
- 16 Jul, 2023 2 commits
-
-
George Guimares authored
The data structures in the comment are not in sync and don't need to be. Referring to function that handles conversion.
-
Chen Tianjie authored
According to the format shown in https://redis.io/commands/cluster-nodes/ ``` <ip:port@cport[,hostname[,auxiliary_field=value]*]> ``` when there is no hostname, and the auxiliary fields are hidden, the cluster topology should be ``` <ip:port@cport> ``` However in the code we always print the hostname even when it is an empty string, leaving an unnecessary comma tailing after cport, which is weird and conflicts with the doc. ``` 94ca2f6cf85228a49fde7b738ee1209de7bee325 127.0.0.1:6379@16379, myself,master - 0 0 0 connected 0-16383 ```
-
- 13 Jul, 2023 4 commits
-
-
Yossi Gottlieb authored
Upgrade to hiredis 1.2.0.
-
Chayim I. Kirshen authored
-
Chayim I. Kirshen authored
-
Chayim I. Kirshen authored
60e5075d4 Version 1.2.0 (#1202) adef139a7 Remove support in deprecated TLS versions 1.0 and 1.1 (#1205) d543baba6 Install major version symlink of shared objects. 052f99ab2 Ensure functionality without _MSC_VER definition ded32c7d1 Add a test for the TCP_USER_TIMEOUT option. (#1192) 5cbd1f296 Add -Werror as a default. (#1193) af1445638 CI: Update homebrew Redis version. (#1191) 6de326e87 Fix a typo in b6a052f. git-subtree-dir: deps/hiredis git-subtree-split: 60e5075d4ac77424809f855ba3e398df7aacefe8
-
- 10 Jul, 2023 1 commit
-
-
Oran Agra authored
* Fix integer overflows due to using wrong integer size. * Add assertions / panic when overflow still happens. * Deletion of dead code to avoid need to maintain it * Some changes are not because of bugs, but rather paranoia. * Improve cmsgpack and cjson test coverage. Co-authored-by:
Yossi Gottlieb <yossigo@gmail.com>
-