summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
* Allow SENTINEL CONFIG SET and SENTINEL CONFIG GET to handle multiple ↵HEADunstableWen Hui2023-05-172-95/+181
| | | | | | | | | parameters. (#10362) Extend SENTINEL CONFIG SET and SENTINEL CONFIG GET to be compatible with variadic CONFIG SET and CONFIG GET and allow multiple parameters to be modified in a single call atomically. Co-authored-by: Oran Agra <oran@redislabs.com>
* Fix for set max entries edge case in setTypeCreate / setTypeMaybeConvert ↵Binbin2023-05-162-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (#12183) In the judgment in setTypeCreate, we should judge size_hint <= max_entries. This results in the following inconsistencies: ``` 127.0.0.1:6379> config set set-max-intset-entries 5 set-max-listpack-entries 5 OK 127.0.0.1:6379> sadd intset_set1 1 2 3 4 5 (integer) 5 127.0.0.1:6379> object encoding intset_set1 "hashtable" 127.0.0.1:6379> sadd intset_set2 1 2 3 4 (integer) 4 127.0.0.1:6379> sadd intset_set2 5 (integer) 1 127.0.0.1:6379> object encoding intset_set2 "intset" 127.0.0.1:6379> sadd listpack_set1 a 1 2 3 4 (integer) 5 127.0.0.1:6379> object encoding listpack_set1 "hashtable" 127.0.0.1:6379> sadd listpack_set2 a 1 2 3 (integer) 4 127.0.0.1:6379> sadd listpack_set2 4 (integer) 1 127.0.0.1:6379> object encoding listpack_set2 "listpack" ``` This was introduced in #12019, added corresponding tests.
* Fix bug: LPOS RANK LONG_ MIN causes overflow (#12167)JJ Lu2023-05-141-1/+1
| | | | | Limit the range of RANK to -LONG_ MAX ~ LONG_ MAX. Without this limit, passing -9223372036854775808 would effectively be the same as passing -1.
* Add basic eventloop latency measurement. (#11963)Chen Tianjie2023-05-124-34/+152
| | | | | | | | | | | | | | | | | | | | | | The measured latency(duration) includes the list below, which can be shown by `INFO STATS`. ``` eventloop_cycles // ever increasing counter eventloop_duration_sum // cumulative duration of eventloop in microseconds eventloop_duration_cmd_sum // cumulative duration of executing commands in microseconds instantaneous_eventloop_cycles_per_sec // average eventloop count per second in recent 1.6s instantaneous_eventloop_duration_usec // average single eventloop duration in recent 1.6s ``` Also added some experimental metrics, which are shown only when `INFO DEBUG` is called. This section isn't included in the default INFO, or even in `INFO ALL` and the fields in this section can change in the future without considering backwards compatibility. ``` eventloop_duration_aof_sum // cumulative duration of writing AOF eventloop_duration_cron_sum // cumulative duration cron jobs (serverCron, beforeSleep excluding IO and AOF) eventloop_cmd_per_cycle_max // max number of commands executed in one eventloop eventloop_duration_max // max duration of one eventloop ``` All of these are being reset by CONFIG RESETSTAT
* fix moduleNotifyKeyUnlink to open the key with READ rather than WRITE (#12156)Oran Agra2023-05-111-1/+1
| | | just a plain overlook by #9406
* redis-cli - add option --count for scan (#12042)kell0gg2023-05-111-2/+8
| | | | | When using scan in redis-cli, the SCAN COUNT is fixed, which means the full scan can take a long time if there are a lot of keys, this will let users specify a bigger COUNT option.
* Correct COMMAND DOCS summary, like COMMAND INFO (#12152)Binbin2023-05-103-10/+20
| | | | | | This pattern is from COMMAND INFO: Returns information about one, multiple or all commands. Also re-generate commands.def, the GEO change was missing in #12151.
* fix `GEORADIUS[BYMEMBER]` `STORE` & `STOREDIST` args spec (#12151)Leibale Eidelman2023-05-094-30/+40
| | | | | in GEO commands, `STORE` and `STOREDIST` are mutually exclusive. use `oneof` block to contain them
* Correct zrangeGenericCommand comment (#12136)tison2023-05-081-1/+1
| | | Fix incorrect documentation about the arguments of ZRANGE commands
* Remove several instances of duplicate "the" in comments (#12144)cui fliter2023-05-082-2/+2
| | | Remove several instances of duplicate "the" in comments
* Minor performance improvement to SADD and HSET (#12019)Madelyn Olson2023-05-083-7/+39
| | | | | | | | | | | | | | | | | | For sets and hashes that will eventually be stored as the hash encoding, it's much faster to immediately convert them to their hash encoding and then perform the insertions since it avoids the O(N) search and frequent reallocations. This change checks the number of arguments in the incoming command, and converts the data-structure if the number of new entries exceeds the listpack-max-entries configuration. This can cause us to over-allocate memory if their are duplicate entries in the input, which is unexpected. unstable Summary: throughput summary: 805.54 requests per second latency summary (msec): avg min p50 p95 p99 max 61.908 25.680 68.351 73.279 75.967 79.295 hset-improvement Summary: throughput summary: 4701.46 requests per second latency summary (msec): avg min p50 p95 p99 max 10.546 0.832 11.959 12.471 13.119 14.967
* Fix defrag CI for 32bit after merge of jemalloc 5.3Oran Agra2023-05-081-1/+2
| | | | | The new mallctl seems to set the output sz when EINVAL occors. that messes up the retry mechanism that does /2 on each iteration.
* Delete empty key if fails after moduleCreateEmptyKey() in module (#12129)sundb2023-05-071-0/+3
| | | | | | | | When `RM_ZsetAdd()`/`RM_ZsetIncrby()`/`RM_StreamAdd()` fails, if a new key happens to be created using `moduleCreateEmptyKey()`, we should clean up the empty key. ## Test 1) Add new module commands(`zset.add` and `zset.incrby`) to cover `RM_ZsetAdd()`/`RM_ZsetIncrby()`. 2) Add a large-memory test to cover `RM_StreamAdd()`.
* Free backlog only if rsi is invalid when master reboot (#12088)zhaozhao.zz2023-05-061-1/+3
| | | | | | | | | | | | When master reboot from RDB, if rsi in RDB is valid we should not free replication backlog, even if master_repl_offset or repl-offset is 0. Since if master doesn't send any data to replicas master_repl_offset is 0, it's a valid number. A clear example: 1. start a master and apply some write commands, the master's master_repl_offset is 0 since it has no replicas. 2. stop write commands on master, and start another instance and replicaof the master, trigger an FULLRESYNC 3. the master's master_repl_offset is still 0 (set a large number for repl-ping-replica-period), do BGSAVE and restart the master 4. master load master_repl_offset from RDB's rsi and it's still 0, and we should make sure replica can partially resync with master.
* Fix name of module API in documentation (#12119)Madelyn Olson2023-05-031-1/+1
| | | API incorrectly uses ExecutionUnit instead of Notification.
* Remove prototypes with empty declarations (#12020)Madelyn Olson2023-05-0242-210/+209
| | | Technically declaring a prototype with an empty declaration has been deprecated since the early days of C, but we never got a warning for it. C2x will apparently be introducing a breaking change if you are using this type of declarator, so Clang 15 has started issuing a warning with -pedantic. Although not apparently a problem for any of the compiler we build on, if feels like the right thing is to properly adhere to the C standard and use (void).
* [redis-benchmark] Adding --seed option to seed the RNG (#11945)Hassaan Khan2023-05-021-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | Adds ability to set the random seed so that more consistent repeatable benchmarks are possible. Example usage: Adding 2 hash items ``` src/redis-benchmark -r 100 -n 2 --seed 250 hset myhash:__rand_int__ age __rand_int__ ``` Monitor: 1st benchmark invocation: ``` 1679332814.824357 [0 127.0.0.1:36686] "hset" "myhash:000000000022" "age" "000000000069" 1679332814.824404 [0 127.0.0.1:36690] "hset" "myhash:000000000007" "age" "000000000043" ``` 2nd benchmark invocation: ``` 1679332814.824357 [0 127.0.0.1:36686] "hset" "myhash:000000000022" "age" "000000000069" 1679332814.824404 [0 127.0.0.1:36690] "hset" "myhash:000000000007" "age" "000000000043" ```
* Add missing reply schema and coverage tests (#12079)Binbin2023-04-276-6/+121
| | | | | | | | | | | | | | | | | | | | The change in #12018 break the CI (fixed by #12083). There are quite a few sentinel commands that are missing both test coverage and also schema. PR added reply-schema to the following commands: - sentinel debug - sentinel info-cache - sentinel pendding-scripts - sentinel reset - sentinel simulate-failure Added some very basic tests for other sentinel commands, just so that they have some coverage. - sentinel help - sentinel masters - sentinel myid - sentinel sentinels - sentinel slaves These tests should be improved / replaced in a followup PR.
* minor optimization for slowlog get (#12103)judeng2023-04-251-7/+8
| | | | We can always know the array length of the response, so there is no need to use addReplyDeferredLen which may introduce some additional overheads.
* iterate clients fairly in clientsCron() (#12025)zhaozhao.zz2023-04-241-4/+3
| | | | | | | Every time when accept a connection, we add the client to `server.clients` list's tail, but in `clientsCron` we rotate the tail to head at first, and then process the head. It means that the "new" client would be processed before "old" client, moreover if connections established and then freed frequently, the "old" client may have no chance to be processed. To fix it, we need take a fair way to iterate the list, that is take the current head and process, and then rotate the head to tail, thus we can make sure all clients could be processed step by step. p.s. client has `client_list_node` pointer, we don't need put the current client to head to avoid O(N) when remove it.
* Report AOF failure status to systemd in shutdown (#12065)Binbin2023-04-241-0/+2
| | | | | | | | | | | Since we do report the RDB error in below: ``` serverLog(LL_WARNING,"Error trying to save the DB, can't exit."); if (server.supervised_mode == SUPERVISED_SYSTEMD) redisCommunicateSystemd("STATUS=Error trying to save the DB, can't exit.\n"); goto error; ``` This may be an overlook in #6052
* after calling aeSetDontWait from beforesleep, aeProcessEvent may be still ↵Pengfei Han2023-04-231-27/+22
| | | | | | | | | wait once (#12068) The code in aeProcessEvent was testing AE_DONT_WAIT flag at the wrong time. The flag is set by by beforeSleep, but was was tested before calling beforeSleep, which would result in aeProcessEvent waiting when it shouldn't have, impacting TLS's HasPendingData. Co-authored-by: Oran Agra <oran@redislabs.com>
* Minor change around the req-res validator, skip sentinel commands (#12083)guybe72023-04-201-6/+3
| | | | | | | | | 1. Check for missing schema only after the docs contain sentinel commands 2. The ignore-list in the C file contain only commands that cannot have a reply schema. The one in the py file is an extension of that list 3. Temp: skipsentinel commands don't have a schema or test coverage yet, add them to the py list Solve CI error introduced by #12018
* Fix typos (#12076)Juho Kim2023-04-203-4/+4
| | | "lst" to "last" in commands help
* Misuse of bool in redis (#12077)YaacovHazan2023-04-205-2/+6
| | | | | | | | | | | | We currently do not allow the use of bool type in redis project. We didn't catch it in script.c because we included hdr_histogram.h in server.h Removing it (but still having it in some c files) reducing the chance to miss the usage of bool type in the future and catch it in compiling stage. It also removes the dependency on hdr_histogram for every unit that includes server.h
* Move startup system check to before daeomniztion and modules init (#12067)Oran Agra2023-04-191-24/+28
| | | | | | 1. it's a bad idea to print these errors and exit after daemonization (if we'll exit, the failure may not be detected) 2. it's not nice to exit (or even do the check that uses `fork`) after modules already started (could create threads, or do some changes)
* Updating reply_schema for sentinal commands (#12018)Wen Hui2023-04-194-2/+36
| | | | | | | | | | | | | | Some sentinel subcommands are missing the reply_schema in the json file, so add the proper reply_schema part in json file as sentinel replicas commands. The schema validator was skipping coverage test for sentinel commands, this was initially done just in order to focus on redis commands and leave sentinel coverage for later, so this check is now removed. sentinel commands that were missing reply schema: * sentinel masters * sentinel myid * sentinel sentinels <master-name> * sentinel slaves (deprecated) <master-name>
* Update dict.c dict_can_resize comment (#12059)Pengfei Han2023-04-181-3/+3
| | | | The comment for dict_can_resize in dict.c should be updated. Currently 0 means DICT_RESIZE_ENABLE, but should actually be DICT_RESIZE_AVOID or 1.
* Fix some compile warnings and errors when building with gcc-12 or clang (#12035)sundb2023-04-188-62/+80
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR is to fix the compilation warnings and errors generated by the latest complier toolchain, and to add a new runner of the latest toolchain for daily CI. ## Fix various compilation warnings and errors 1) jemalloc.c COMPILER: clang-14 with FORTIFY_SOURCE WARNING: ``` src/jemalloc.c:1028:7: warning: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Wstring-concatenation] "/etc/malloc.conf", ^ src/jemalloc.c:1027:3: note: place parentheses around the string literal to silence warning "\"name\" of the file referenced by the symbolic link named " ^ ``` REASON: the compiler to alert developers to potential issues with string concatenation that may miss a comma, just like #9534 which misses a comma. SOLUTION: use `()` to tell the compiler that these two line strings are continuous. 2) config.h COMPILER: clang-14 with FORTIFY_SOURCE WARNING: ``` In file included from quicklist.c:36: ./config.h:319:76: warning: attribute declaration must precede definition [-Wignored-attributes] char *strcat(char *restrict dest, const char *restrict src) __attribute__((deprecated("please avoid use of unsafe C functions. prefer use of redis_strlcat instead"))); ``` REASON: Enabling _FORTIFY_SOURCE will cause the compiler to use `strcpy()` with check, it results in a deprecated attribute declaration after including <features.h>. SOLUTION: move the deprecated attribute declaration from config.h to fmacro.h before "#include <features.h>". 3) networking.c COMPILER: GCC-12 WARNING: ``` networking.c: In function ‘addReplyDouble.part.0’: networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 876 | dbuf[start] = '$'; | ^ networking.c:868:14: note: at offset -5 into destination object ‘dbuf’ of size 5152 868 | char dbuf[MAX_LONG_DOUBLE_CHARS+32]; | ^ networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 876 | dbuf[start] = '$'; | ^ networking.c:868:14: note: at offset -6 into destination object ‘dbuf’ of size 5152 868 | char dbuf[MAX_LONG_DOUBLE_CHARS+32]; ``` REASON: GCC-12 predicts that digits10() may return 9 or 10 through `return 9 + (v >= 1000000000UL)`. SOLUTION: add an assert to let the compiler know the possible length; 4) redis-cli.c & redis-benchmark.c COMPILER: clang-14 with FORTIFY_SOURCE WARNING: ``` redis-benchmark.c:1621:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL redis-cli.c:3015:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL ``` REASON: when _FORTIFY_SOURCE is enabled, the compiler will use the print() with check, which is a macro. this may result in the use of directives within the macro, which is undefined behavior. SOLUTION: move the directives-related code out of `print()`. 5) server.c COMPILER: gcc-13 with FORTIFY_SOURCE WARNING: ``` In function 'lookupCommandLogic', inlined from 'lookupCommandBySdsLogic' at server.c:3139:32: server.c:3102:66: error: '*(robj **)argv' may be used uninitialized [-Werror=maybe-uninitialized] 3102 | struct redisCommand *base_cmd = dictFetchValue(commands, argv[0]->ptr); | ~~~~^~~ ``` REASON: The compiler thinks that the `argc` returned by `sdssplitlen()` could be 0, resulting in an empty array of size 0 being passed to lookupCommandLogic. this should be a false positive, `argc` can't be 0 when strings are not NULL. SOLUTION: add an assert to let the compiler know that `argc` is positive. 6) sha1.c COMPILER: gcc-12 WARNING: ``` In function ‘SHA1Update’, inlined from ‘SHA1Final’ at sha1.c:195:5: sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread] 152 | SHA1Transform(context->state, &data[i]); | ^ sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’ sha1.c: In function ‘SHA1Final’: sha1.c:56:6: note: in a call to function ‘SHA1Transform’ 56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64]) | ^ In function ‘SHA1Update’, inlined from ‘SHA1Final’ at sha1.c:198:9: sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread] 152 | SHA1Transform(context->state, &data[i]); | ^ sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’ sha1.c: In function ‘SHA1Final’: sha1.c:56:6: note: in a call to function ‘SHA1Transform’ 56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64]) ``` REASON: due to the bug[https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922], when enable LTO, gcc-12 will not see `diagnostic ignored "-Wstringop-overread"`, resulting in a warning. SOLUTION: temporarily set SHA1Update to noinline to avoid compiler warnings due to LTO being enabled until the above gcc bug is fixed. 7) zmalloc.h COMPILER: GCC-12 WARNING: ``` In function ‘memset’, inlined from ‘moduleCreateContext’ at module.c:877:5, inlined from ‘RM_GetDetachedThreadSafeContext’ at module.c:8410:5: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:10: warning: ‘__builtin_memset’ writing 104 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=] 59 | return __builtin___memset_chk (__dest, __ch, __len, ``` REASON: due to the GCC-12 bug [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503], GCC-12 cannot see alloc_size, which causes GCC to think that the actual size of memory is 0 when checking with __glibc_objsize0(). SOLUTION: temporarily set malloc-related interfaces to `noinline` to avoid compiler warnings due to LTO being enabled until the above gcc bug is fixed. ## Other changes 1) Fixed `ps -p [pid]` doesn't output `<defunct>` when using procps 4.x causing `replication child dies when parent is killed - diskless` test to fail. 2) Add a new fortify CI with GCC-13 and ubuntu-lunar docker image.
* Fix RDB check regression caused by PR 12022 (#12051)Joe Hu2023-04-175-13/+14
| | | | | | | | | | | | | | | The nightly tests showed that the recent PR #12022 caused random failures in aof.tcl on checking RDB preamble inside an AOF file. Root cause: When checking RDB preamble in an AOF file, what's passed into redis_check_rdb is aof_filename, not aof_filepath. The newly introduced isFifo function does not check return status of the stat call and hence uses the uninitailized stat_p object. Fix: 1. Fix isFifo by checking stat call's return code. 2. Pass aof_filepath instead of aof_filename to redis_check_rdb. 3. move the FIFO check to rdb.c since the limitation is the re-opening of the file, and not anything specific about redis-check-rdb.
* avoid incorrect shrinking of querybuf when client is reading a big argv (#12000)judeng2023-04-162-3/+8
| | | | | | | | | | | | | this pr fix two wrongs: 1. When client’s querybuf is pre-allocated for a fat argv, we need to update the querybuf_peak of the client immediately to completely avoid the unexpected shrinking of querybuf in the next clientCron (before data arrives to set the peak). 2. the protocol's bulklen does not include `\r\n`, but the allocation and the data we read does. so in `clientsCronResizeQueryBuffer`, the `resize` or `querybuf_peak` should add these 2 bytes. the first bug is likely to hit us on large payloads over slow connections, in which case transferring the payload can take longer and a cron event will be triggered (specifically if there are not a lot of clients)
* improve performance of keys command by using static objects (#12036)judeng2023-04-161-5/+4
| | | Improve performance by avoiding redundancy memory malloc/free
* Fix redis_check_rdb() hang when rdb is FIFO (#12022)Joe Hu2023-04-141-0/+11
| | | When loading RDB over the named piped, redis_check_rdb() is hung at fopen, because fopen blocks until another process opens the FIFO for writing. The fix is to check if RDB is FIFO. If yes, return an error.
* Update reply_schema details for info and hset commands json files ↵Wen Hui2023-04-132-0/+2
| | | | | | | accordingly. (#12017) These commands had an empty description. Co-authored-by: Oran Agra <oran@redislabs.com>
* Print IP and port on Possible SECURITY ATTACK detected (#12024)Binbin2023-04-121-1/+7
| | | | Add a print statement to indicate which IP/port is sending the attack. So that the offending connection can be tracked down, if necessary.
* Add RM_ReplyWithErrorFormat that can support format (#11923)Binbin2023-04-124-2/+32
| | | | | | | | | | | | | | | * Add RM_ReplyWithErrorFormat that can support format Reply with the error create from a printf format and arguments. If the error code is already passed in the string 'fmt', the error code provided is used, otherwise the string "-ERR " for the generic error code is automatically added. The usage is, for example: RedisModule_ReplyWithErrorFormat(ctx, "An error: %s", "foo"); RedisModule_ReplyWithErrorFormat(ctx, "-WRONGTYPE Wrong Type: %s", "foo"); The function always returns REDISMODULE_OK.
* Use dummy allocator to make accesses defined as per standard (#11982)sundb2023-04-106-37/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ## Issue When we use GCC-12 later or clang 9.0 later to build with `-D_FORTIFY_SOURCE=3`, we can see the following buffer overflow: ``` === REDIS BUG REPORT START: Cut & paste starting from here === 6263:M 06 Apr 2023 08:59:12.915 # Redis 255.255.255 crashed by signal: 6, si_code: -6 6263:M 06 Apr 2023 08:59:12.915 # Crashed running the instruction at: 0x7f03d59efa7c ------ STACK TRACE ------ EIP: /lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x12c)[0x7f03d59efa7c] Backtrace: /lib/x86_64-linux-gnu/libc.so.6(+0x42520)[0x7f03d599b520] /lib/x86_64-linux-gnu/libc.so.6(pthread_kill+0x12c)[0x7f03d59efa7c] /lib/x86_64-linux-gnu/libc.so.6(raise+0x16)[0x7f03d599b476] /lib/x86_64-linux-gnu/libc.so.6(abort+0xd3)[0x7f03d59817f3] /lib/x86_64-linux-gnu/libc.so.6(+0x896f6)[0x7f03d59e26f6] /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x2a)[0x7f03d5a8f76a] /lib/x86_64-linux-gnu/libc.so.6(+0x1350c6)[0x7f03d5a8e0c6] src/redis-server 127.0.0.1:25111(+0xd5e80)[0x557cddd3be80] src/redis-server 127.0.0.1:25111(feedReplicationBufferWithObject+0x78)[0x557cddd3c768] src/redis-server 127.0.0.1:25111(replicationFeedSlaves+0x1a4)[0x557cddd3cbc4] src/redis-server 127.0.0.1:25111(+0x8721a)[0x557cddced21a] src/redis-server 127.0.0.1:25111(call+0x47a)[0x557cddcf38ea] src/redis-server 127.0.0.1:25111(processCommand+0xbf4)[0x557cddcf4aa4] src/redis-server 127.0.0.1:25111(processInputBuffer+0xe6)[0x557cddd22216] src/redis-server 127.0.0.1:25111(readQueryFromClient+0x3a8)[0x557cddd22898] src/redis-server 127.0.0.1:25111(+0x1b9134)[0x557cdde1f134] src/redis-server 127.0.0.1:25111(aeMain+0x119)[0x557cddce5349] src/redis-server 127.0.0.1:25111(main+0x466)[0x557cddcd6716] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90)[0x7f03d5982d90] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80)[0x7f03d5982e40] src/redis-server 127.0.0.1:25111(_start+0x25)[0x557cddcd7025] ``` The main reason is that when FORTIFY_SOURCE is enabled, GCC or clang will enhance some common functions, such as `strcpy`, `memcpy`, `fgets`, etc, so that they can detect buffer overflow errors and stop program execution, thus improving the safety of the program. We use `zmalloc_usable_size()` everywhere to use memory blocks, but that is an abuse since the malloc_usable_size() isn't meant for this kind of use, it is for diagnostics only. That is also why the behavior is flaky when built with _FORTIFY_SOURCE, the compiler can sense that we reach outside the allocated block and SIGABRT. ### Solution If we need to use the additional memory we got, we need to use a dummy realloc with `alloc_size` attribute and no inlining, (see `extend_to_usable`) to let the compiler see the large of memory we need to use. This can either be an implicit call inside `z*usable` that returns the size, so that the caller doesn't have any other worry, or it can be a normal zmalloc call which means that if the caller wants to use zmalloc_usable_size it must also use extend_to_usable. ### Changes This PR does the following: 1) rename the current z[try]malloc_usable family to z[try]malloc_internal and don't expose them to users outside zmalloc.c, 2) expose a new set of `z[*]_usable` family that use z[*]_internal and `extend_to_usable()` implicitly, the caller gets the size of the allocation and it is safe to use. 3) go over all the users of `zmalloc_usable_size` and convert them to use the `z[*]_usable` family if possible. 4) in the places where the caller can't use `z[*]_usable` and store the real size, and must still rely on zmalloc_usable_size, we still make sure that the allocation used `z[*]_usable` (which has a call to `extend_to_usable()`) and ignores the returning size, this way a later call to `zmalloc_usable_size` is still safe. [4] was done for module.c and listpack.c, all the others places (sds, reply proto list, replication backlog, client->buf) are using [3]. Co-authored-by: Oran Agra <oran@redislabs.com>
* Add RM_RdbLoad and RM_RdbSave module API functions (#11852)Ozan Tezcan2023-04-094-18/+198
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add `RM_RdbLoad()` and `RM_RdbSave()` to load/save RDB files from the module API. In our use case, we have our clustering implementation as a module. As part of this implementation, the module needs to trigger RDB save operation at specific points. Also, this module delivers RDB files to other nodes (not using Redis' replication). When a node receives an RDB file, it should be able to load the RDB. Currently, there is no module API to save/load RDB files. This PR adds four new APIs: ```c RedisModuleRdbStream *RM_RdbStreamCreateFromFile(const char *filename); void RM_RdbStreamFree(RedisModuleRdbStream *stream); int RM_RdbLoad(RedisModuleCtx *ctx, RedisModuleRdbStream *stream, int flags); int RM_RdbSave(RedisModuleCtx *ctx, RedisModuleRdbStream *stream, int flags); ``` The first step is to create a `RedisModuleRdbStream` object. This PR provides a function to create RedisModuleRdbStream from the filename. (You can load/save RDB with the filename). In the future, this API can be extended if needed: e.g., `RM_RdbStreamCreateFromFd()`, `RM_RdbStreamCreateFromSocket()` to save/load RDB from an `fd` or a `socket`. Usage: ```c /* Save RDB */ RedisModuleRdbStream *stream = RedisModule_RdbStreamCreateFromFile("example.rdb"); RedisModule_RdbSave(ctx, stream, 0); RedisModule_RdbStreamFree(stream); /* Load RDB */ RedisModuleRdbStream *stream = RedisModule_RdbStreamCreateFromFile("example.rdb"); RedisModule_RdbLoad(ctx, stream, 0); RedisModule_RdbStreamFree(stream); ```
* Add help message for client setinfo (#11995)bodong.ybd2023-04-041-0/+4
| | | | | The new sub-command was missing from CLIENT HELP Co-authored-by: Binbin <binloveplay1314@qq.com>
* check for known-slave in sentinel rewrite config (#11775)Subhi Al Hasan2023-04-043-4/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix the following config file error ``` *** FATAL CONFIG FILE ERROR (Redis 6.2.7) *** Reading the configuration file, at line 152 >>> 'sentinel known-replica XXXX 127.0.0.1 5001' Duplicate hostname and port for replica. ``` that is happening when a user uses the legacy key "known-slave" in the config file and a config rewrite occurs. The config rewrite logic won't replace the old line "sentinel known-slave XXXX 127.0.0.1 5001" and would add a new line with "sentinel known-replica XXXX 127.0.0.1 5001" which results in the error above "Duplicate hostname and port for replica." example: Current sentinal.conf ``` ... sentinel known-slave XXXX 127.0.0.1 5001 sentinel example-random-option X ... ``` after the config rewrite logic runs: ``` .... sentinel known-slave XXXX 127.0.0.1 5001 sentinel example-random-option X # Generated by CONFIG REWRITE sentinel known-replica XXXX 127.0.0.1 5001 ``` This bug only exists in Redis versions >=6.2 because prior to that it was hidden by the effects of this bug https://github.com/redis/redis/issues/5388 that was fixed in https://github.com/redis/redis/pull/8271 and was released in versions >=6.2
* Fix local clients detection (#11664)gx2023-04-041-1/+1
| | | Match 127.0.0.0/8 instead of just `127.0.0.1` to detect the local clients.
* Changed activeExpireCycle server.masterhost check to iAmMaster in ↵Binbin2023-04-041-1/+1
| | | | | | | | | | | | | beforeSleep (#11997) In cluster mode, when a node restart as a replica, it doesn't immediately sync with the master, replication is enabled in clusterCron. It means that sometime server.masterhost is NULL and we wrongly judge it in beforeSleep. In this case, we may trigger a fast activeExpireCycle in beforeSleep, but the node's flag is actually a replica, that can lead to data inconsistency. In this PR, we use iAmMaster to replace the `server.masterhost == NULL` This is an overlook in #7001, and more discussion in #11783.
* redis-cli - handle sensitive command redaction for variadic CONFIG SET (#11975)Wen Hui2023-04-021-6/+9
| | | | | | | | | | | | | | | In the Redis 7.0 and newer version, config set command support multiply `<parameter> <value>` pairs, thus the previous sensitive command condition does not apply anymore For example: The command: **config set maxmemory 1GB masteruser aa** will be written to redis_cli historyfile In this PR, we update the condition for these sensitive commands config set masteruser <username> config set masterauth <master-password> config set requirepass foobared
* Disconnect pub-sub subscribers when revoking `allchannels` permission (#11992)Slava Koyfman2023-04-021-0/+7
| | | | | | | | | | | | | | | | The existing logic for killing pub-sub clients did not handle the `allchannels` permission correctly. For example, if you: ACL SETUSER foo allchannels Have a client authenticate as the user `foo` and subscribe to a channel, and then: ACL SETUSER foo resetchannels The subscribed client would not be disconnected, though new clients under that user would be blocked from subscribing to any channels. This was caused by an incomplete optimization in `ACLKillPubsubClientsIfNeeded` checking whether the new channel permissions were a strict superset of the old ones.
* Reimplement cli hints based on command arg docs (#10515)Jason Elbaum2023-03-3011-9699/+11795
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the command argument specs are available at runtime (#9656), this PR addresses #8084 by implementing a complete solution for command-line hinting in `redis-cli`. It correctly handles nearly every case in Redis's complex command argument definitions, including `BLOCK` and `ONEOF` arguments, reordering of optional arguments, and repeated arguments (even when followed by mandatory arguments). It also validates numerically-typed arguments. It may not correctly handle all possible combinations of those, but overall it is quite robust. Arguments are only matched after the space bar is typed, so partial word matching is not supported - that proved to be more confusing than helpful. When the user's current input cannot be matched against the argument specs, hinting is disabled. Partial support has been implemented for legacy (pre-7.0) servers that do not support `COMMAND DOCS`, by falling back to a statically-compiled command argument table. On startup, if the server does not support `COMMAND DOCS`, `redis-cli` will now issue an `INFO SERVER` command to retrieve the server version (unless `HELLO` has already been sent, in which case the server version will be extracted from the reply to `HELLO`). The server version will be used to filter the commands and arguments in the command table, removing those not supported by that version of the server. However, the static table only includes core Redis commands, so with a legacy server hinting will not be supported for module commands. The auto generated help.h and the scripts that generates it are gone. Command and argument tables for the server and CLI use different structs, due primarily to the need to support different runtime data. In order to generate code for both, macros have been added to `commands.def` (previously `commands.c`) to make it possible to configure the code generation differently for different use cases (one linked with redis-server, and one with redis-cli). Also adding a basic testing framework for the command hints based on new (undocumented) command line options to `redis-cli`: `--test_hint 'INPUT'` prints out the command-line hint for a given input string, and `--test_hint_file <filename>` runs a suite of test cases for the hinting mechanism. The test suite is in `tests/assets/test_cli_hint_suite.txt`, and it is run from `tests/integration/redis-cli.tcl`. Co-authored-by: Oran Agra <oran@redislabs.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
* Fixed tracking of command duration for multi/eval/module/wait (#11970)Madelyn Olson2023-03-295-1/+14
| | | | | | | | | In #11012, we changed the way command durations were computed to handle the same command being executed multiple times. This commit fixes some misses from that commit. * Wait commands were not correctly reporting their duration if the timeout was reached. * Multi/scripts/and modules with RM_Call were not properly resetting the duration between inner calls, leading to them reporting cumulative duration. * When a blocked client is freed, the call and duration are always discarded. This commit also adds an assert if the duration is not properly reset, potentially indicating that a report to call statistics was missed. The assert potentially be removed in the future, as it's mainly intended to detect misses in tests.
* Overhauls command summaries and man pages. (#11942)Itamar Haber2023-03-29389-783/+782
| | | | | | | | | | | | | | This is an attempt to normalize/formalize command summaries. Main actions performed: * Starts with the continuation of the phrase "The XXXX command, when called, ..." for user commands. * Starts with "An internal command...", "A container command...", etc... when applicable. * Always uses periods. * Refrains from referring to other commands. If this is needed, backquotes should be used for command names. * Tries to be very clear about the data type when applicable. * Tries to mention additional effects, e.g. "The key is created if it doesn't exist" and "The set is deleted if the last member is removed." * Prefers being terse over verbose. * Tries to be consistent.
* Fix fork done handler wrongly update fsync metrics and enhance AOF_ ↵Binbin2023-03-293-13/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | FSYNC_ALWAYS (#11973) This PR fix several unrelated bugs that were discovered by the same set of tests (WAITAOF tests in #11713), could make the `WAITAOF` test hang. The change in `backgroundRewriteDoneHandler` is about MP-AOF. That leftover / old code assumes that we started a new AOF file just now (when we have a new base into which we're gonna incrementally write), but the fact is that with MP-AOF, the fork done handler doesn't really affect the incremental file being maintained by the parent process, there's no reason to re-issue `SELECT`, and no reason to update any of the fsync variables in that flow. This should have been deleted with MP-AOF (introduced in #9788, 7.0). The damage is that the update to `aof_fsync_offset` will cause us to miss an fsync in `flushAppendOnlyFile`, that happens if we stop write commands in `AOF_FSYNC_EVERYSEC` while an AOFRW is in progress. This caused a new `WAITAOF` test to sometime hang forever. Also because of MP-AOF, we needed to change `aof_fsync_offset` to `aof_last_incr_fsync_offset` and match it to `aof_last_incr_size` in `flushAppendOnlyFile`. This is because in the past we compared `aof_fsync_offset` and `aof_current_size`, but with MP-AOF it could be the total AOF file will be smaller after AOFRW, and the (already existing) incr file still has data that needs to be fsynced. The change in `flushAppendOnlyFile`, about the `AOF_FSYNC_ALWAYS`, it is follow #6053 (the details is in #5985), we also check `AOF_FSYNC_ALWAYS` to handle a case where appendfsync is changed from everysec to always while there is data that's written but not yet fsynced.
* Clang: fix for -flto argument (#11961)Rafi Einstein2023-03-271-2/+7
| | | | Starting with the recent #11926 Makefile specifies `-flto=auto` which is unsupported on clang. Additionally, detecting clang correctly requires actually running it, since on MacOS gcc can be an alias for clang.
* Fix reply schema validator with RESET command (#11953)Oran Agra2023-03-221-0/+4
| | | The reply schema validator is failing since the recent changes to introspection.tcl that use the RESET command, this happens because this test forces RESP3, but RESET command didn't respect that and set back RESP2.