1. 31 Aug, 2023 1 commit
  2. 30 Aug, 2023 3 commits
    • Roshan Khatri's avatar
      Allows modules to declare new ACL categories. (#12486) · 75199605
      Roshan Khatri authored
      
      
      This PR adds a new Module API int RM_AddACLCategory(RedisModuleCtx *ctx, const char *category_name) to add a new ACL command category.
      
      Here, we initialize the ACLCommandCategories array by allocating space for 64 categories and duplicate the 21 default categories from the predefined array 'ACLDefaultCommandCategories' into the ACLCommandCategories array while ACL initialization. Valid ACL category names can only contain alphanumeric characters, underscores, and dashes.
      
      The API when called, checks for the onload flag, category name validity, and for duplicate category name if present. If the conditions are satisfied, the API adds the new category to the trailing end of the ACLCommandCategories array and assigns the acl_categories flag bit according to the index at which the category is added.
      
      If any error is encountered the errno is set accordingly by the API.
      
      ---------
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      75199605
    • bodong.ybd's avatar
      Fix sort_ro get-keys function return wrong key number (#12522) · b59f53ef
      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>
      ```
      b59f53ef
    • Chen Tianjie's avatar
      Add two stats to count client input and output buffer oom. (#12476) · e3d4b30d
      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.
      e3d4b30d
  3. 27 Aug, 2023 1 commit
    • Binbin's avatar
      Add printing for LATENCY related tests (#12514) · e7926537
      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.
      e7926537
  4. 21 Aug, 2023 2 commits
    • Binbin's avatar
      BITCOUNT and BITPOS with non-existing key and illegal arguments should return error, not 0 (#11734) · 1407ac1f
      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
      ```
      1407ac1f
    • Wen Hui's avatar
      BITCOUNT: check for argument, before checking for key (#12394) · 45d33106
      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
      ```
      45d33106
  5. 20 Aug, 2023 1 commit
    • Wen Hui's avatar
      Added tests for Client commands (#10276) · e532c95d
      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.
      e532c95d
  6. 16 Aug, 2023 1 commit
  7. 10 Aug, 2023 2 commits
    • Madelyn Olson's avatar
      Fixed a bug where sequential matching ACL rules weren't compressed (#12472) · 7c179f9b
      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. 
      7c179f9b
    • Binbin's avatar
      Fix flaky SENTINEL RESET test (#12437) · 6abfda54
      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.
      6abfda54
  8. 05 Aug, 2023 2 commits
    • zhaozhao.zz's avatar
      do not call handleClientsBlockedOnKeys inside yielding command (#12459) · 8226f39f
      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: default avatarOran Agra <oran@redislabs.com>
      8226f39f
    • sundb's avatar
      Avoid mostly harmless integer overflow in cjson (#12456) · da9c2804
      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).
      da9c2804
  9. 02 Aug, 2023 1 commit
  10. 01 Aug, 2023 1 commit
  11. 30 Jul, 2023 1 commit
  12. 25 Jul, 2023 2 commits
  13. 20 Jul, 2023 1 commit
    • Makdon's avatar
      redis-cli: use previous hostip when not provided by redis cluster server (#12273) · 2495b90a
      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: default avatarViktor Söderqvist <viktor.soderqvist@est.tech>
      Co-authored-by: default avatarMadelyn Olson <madelynolson@gmail.com>
      2495b90a
  14. 10 Jul, 2023 1 commit
  15. 06 Jul, 2023 1 commit
    • Sankar's avatar
      Process loss of slot ownership in cluster bus (#12344) · 1190f25c
      Sankar authored
      Process loss of slot ownership in cluster bus
      
      When a node no longer owns a slot, it clears the bit corresponding
      to the slot in the cluster bus messages. The receiving nodes
      currently don't record the fact that the sender stopped claiming
      a slot until some other node in the cluster starts claiming the slot.
      This can cause a slot to go missing during slot migration when subjected
      to inopportune race with addition of new shards or a failover.
      This fix forces the receiving nodes to process the loss of ownership
      to avoid spreading wrong information.
      1190f25c
  16. 05 Jul, 2023 1 commit
    • Binbin's avatar
      Fix and increase tollerance of event loop test, add verbose logs (#12385) · d56a7d9b
      Binbin authored
      The test fails on freebsd CI:
      ```
      *** [err]: stats: eventloop metrics in tests/unit/info.tcl
      Expected '31777' to be less than '16183' (context: type eval line 17 cmd
      {assert_lessthan $el_sum2 [expr $el_sum1+10000] } proc ::test)
      ```
      
      The test added in #11963, fails on freebsd CI which is slow,
      increase tollerance and also add some verbose logs, now we can
      see these logs in verbose mode (for better views):
      ```
      eventloop metrics cycle1: 12, cycle2: 15
      eventloop metrics el_sum1: 315, el_sum2: 411
      eventloop metrics cmd_sum1: 126, cmd_sum2: 137
      [ok]: stats: eventloop metrics (111 ms)
      instantaneous metrics instantaneous_eventloop_cycles_per_sec: 8
      instantaneous metrics instantaneous_eventloop_duration_usec: 55
      [ok]: stats: instantaneous metrics (1603 ms)
      [ok]: stats: debug metrics (112 ms)
      ```
      d56a7d9b
  17. 03 Jul, 2023 1 commit
    • Lior Lahav's avatar
      Fix possible crash in command getkeys (#12380) · b7559d9f
      Lior Lahav authored
      
      
      When getKeysUsingKeySpecs processes a command with more than one key-spec,
      and called with a total of more than 256 keys, it'll call getKeysPrepareResult again,
      but since numkeys isn't updated, getKeysPrepareResult will not bother to copy key
      names from the old result (leaving these slots uninitialized). Furthermore, it did not
      consider the keys it already found when allocating more space.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      b7559d9f
  18. 02 Jul, 2023 1 commit
  19. 27 Jun, 2023 1 commit
    • judeng's avatar
      improve performance for scan command when matching pattern or data type (#12209) · 07ed0eaf
      judeng authored
      
      
      Optimized the performance of the SCAN command in a few ways:
      1. Move the key filtering (by MATCH pattern) in the scan callback,
        so as to avoid collecting them for later filtering.
      2. Reduce a many memory allocations and copying (use a reference
        to the original sds, instead of creating an robj, an excessive 2 mallocs
        and one string duplication)
      3. Compare TYPE filter directly (as integers), instead of inefficient string
        compare per key.
      4. fixed a small bug: when scan zset and hash types, maxiterations uses
        a more accurate number to avoid wrong double maxiterations.
      
      Changes **postponed** for a later version (8.0):
      1. Prepare to move the TYPE filtering to the scan callback as well. this was
        put on hold since it has side effects that can be considered a breaking
        change, which is that we will not attempt to do lazy expire (delete) a key
        that was filtered by not matching the TYPE (changing it would mean TYPE filter
        starts behaving the same as MATCH filter already does in that respect). 
      2. when the specified key TYPE filter is an unknown type, server will reply a error
        immediately instead of doing a full scan that comes back empty handed. 
      
      Benchmark result:
      For different scenarios, we obtained about 30% or more performance improvement.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      07ed0eaf
  20. 26 Jun, 2023 1 commit
    • Chen Tianjie's avatar
      Support TLS service when "tls-cluster" is not enabled and persist both plain... · 22a29935
      Chen Tianjie authored
      Support TLS service when "tls-cluster" is not enabled and persist both plain and TLS port in nodes.conf (#12233)
      
      Originally, when "tls-cluster" is enabled, `port` is set to TLS port. In order to support non-TLS clients, `pport` is used to propagate TCP port across cluster nodes. However when "tls-cluster" is disabled, `port` is set to TCP port, and `pport` is not used, which means the cluster cannot provide TLS service unless "tls-cluster" is on.
      ```
      typedef struct {
          // ...
          uint16_t port;  /* Latest known clients port (TLS or plain). */
          uint16_t pport; /* Latest known clients plaintext port. Only used if the main clients port is for TLS. */
          // ...
      } clusterNode;
      ```
      ```
      typedef struct {
          // ...
          uint16_t port;   /* TCP base port number. */
          uint16_t pport;  /* Sender TCP plaintext port, if base port is TLS */
          // ...
      } clusterMsg;
      ```
      This PR renames `port` and `pport` in `clusterNode` to `tcp_port` and `tls_port`, to record both ports no matter "tls-cluster" is enabled or disabled.
      
      This allows to provide TLS service to clients when "tls-cluster" is disabled: when displaying cluster topology, or giving `MOVED` error, server can provide TLS or TCP port according to client's connection type, no matter what type of connection cluster bus is using.
      
      For backwards compatibility, `port` and `pport` in `clusterMsg` are preserved, when "tls-cluster" is enabled, `port` is set to TLS port and `pport` is set to TCP port, when "tls-cluster" is disabled, `port` is set to TCP port and `pport` is set to TLS port (instead of 0).
      
      Also, in the nodes.conf file, a new aux field displaying an extra port is added to complete the persisted info. We may have `tls_port=xxxxx` or `tcp_port=xxxxx` in the aux field, to complete the cluster topology, while the other port is stored in the normal `<ip>:<port>` field. The format is shown below.
      ```
      <node-id> <ip>:<tcp_port>@<cport>,<hostname>,shard-id=...,tls-port=6379 myself,master - 0 0 0 connected 0-1000
      ```
      Or we can switch the position of two ports, both can be correctly resolved.
      ```
      <node-id> <ip>:<tls_port>@<cport>,<hostname>,shard-id=...,tcp-port=6379 myself,master - 0 0 0 connected 0-1000
      ```
      22a29935
  21. 25 Jun, 2023 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Fix use after free on blocking RM_Call. (#12342) · 153f8f08
      Meir Shpilraien (Spielrein) authored
      blocking RM_Call was introduced on: #11568, It allows a module to perform
      blocking commands and get the reply asynchronously.If the command gets
      block, a special promise CallReply is returned that allow to set the unblock
      handler. The unblock handler will be called when the command invocation
      finish and it gets, as input, the command real reply.
      
      The issue was that the real CallReply was created using a stack allocated
      RedisModuleCtx which is no longer available after the unblock handler finishes.
      So if the module keeps the CallReply after the unblock handler finished, the
      CallReply holds a pointer to invalid memory and will try to access it when the
      CallReply will be released.
      
      The solution is to create the CallReply with a NULL context to make it totally
      detached and can be freed freely when the module wants.
      
      Test was added to cover this case, running the test with valgrind before the
      fix shows the use after free error. With the fix, there are no valgrind errors.
      
      unrelated: adding a missing `$rd close` in many tests in that file.
      153f8f08
  22. 22 Jun, 2023 1 commit
    • guybe7's avatar
      Modules: Unblock from within a timer coverage (#12337) · 32301999
      guybe7 authored
      Apart from adding the missing coverage, this PR also adds `blockedBeforeSleep`
      that gathers all block-related functions from `beforeSleep`
      
      The order inside `blockedBeforeSleep` is different: now `handleClientsBlockedOnKeys`
      (which may unblock clients) is called before `processUnblockedClients` (which handles
      unblocked clients).
      It makes sense to have this order.
      
      There are no visible effects of the wrong ordering, except some cleanups of the now-unblocked
      client would have  happen in the next `beforeSleep` (will now happen in the current one)
      
      The reason we even got into it is because i triggers an assertion in logresreq.c (breaking
      the assumption that `unblockClient` is called **before** actually flushing the reply to the socket):
      `handleClientsBlockedOnKeys` is called, then it calls `moduleUnblockClientOnKey`, which calls
      `moduleUnblockClient`, which adds the client to `moduleUnblockedClients` back to `beforeSleep`,
      we call `handleClientsWithPendingWritesUsingThreads`, it writes the data of buf to the client, so
      `client->bufpos` became 0
      On the next `beforeSleep`, we call `moduleHandleBlockedClients`, which calls `unblockClient`,
      which calls `reqresAppendResponse`, triggering the assert. (because the `bufpos` is 0) - see https://github.com/redis/redis/pull/12301#discussion_r1226386716
      32301999
  23. 21 Jun, 2023 1 commit
    • Madelyn Olson's avatar
      Make nodename test more consistent (#12330) · 73cf0243
      Madelyn Olson authored
      To determine when everything was stable, we couldn't just query the nodename since they aren't API visible by design. Instead, we were using a proxy piece of information which was bumping the epoch and waiting for everyone to observe that. This works for making source Node 0 and Node 1 had pinged, and Node 0 and Node 2 had pinged, but did not guarantee that Node 1 and Node 2 had pinged. Although unlikely, this can cause this failure message. To fix it I hijacked hostnames and used its validation that it has been propagated, since we know that it is stable.
      
      I also noticed while stress testing this sometimes the test took almost 4.5 seconds to finish, which is really close to the current 5 second limit of the log check, so I bumped that up as well just to make it a bit more consistent.
      73cf0243
  24. 20 Jun, 2023 7 commits
    • guybe7's avatar
      Align RM_ReplyWithErrorFormat with RM_ReplyWithError (#12321) · 20fa1560
      guybe7 authored
      Introduced by https://github.com/redis/redis/pull/11923 (Redis 7.2 RC2)
      
      It's very weird and counterintuitive that `RM_ReplyWithError` requires the error-code
      **without** a hyphen while `RM_ReplyWithErrorFormat` requires either the error-code
      **with** a hyphen or no error-code at all
      ```
      RedisModule_ReplyWithError(ctx, "BLA bla bla");
      ```
      vs.
      ```
      RedisModule_ReplyWithErrorFormat(ctx, "-BLA %s", "bla bla");
      ```
      
      This commit aligns RM_ReplyWithErrorFormat to behvae like RM_ReplyWithError.
      it's a breaking changes but it's done before 7.2 goes GA.
      20fa1560
    • Oran Agra's avatar
      Fix broken protocol when PUBLISH emits local push inside MULTI (#12326) · 8ad8f0f9
      Oran Agra authored
      When a connection that's subscribe to a channel emits PUBLISH inside MULTI-EXEC,
      the push notification messes up the EXEC response.
      
      e.g. MULTI, PING, PUSH foo bar, PING, EXEC
      the EXEC's response will contain: PONG, {message foo bar}, 1. and the second PONG
      will be delivered outside the EXEC's response.
      
      Additionally, this PR changes the order of responses in case of a plain PUBLISH (when
      the current client also subscribed to it), by delivering the push after the command's
      response instead of before it.
      This also affects modules calling RM_PublishMessage in a similar way, so that we don't
      run the risk of getting that push mixed together with the module command's response.
      8ad8f0f9
    • Binbin's avatar
      zrangeGenericCommand add check for negative offset (#9052) · d9c2ef8a
      Binbin authored
      Now we will check the offset in zrangeGenericCommand.
      With a negative offset, we will throw an error and return.
      
      This also resolve the issue of zeroing the destination key
      in case of the "store" variant when we input a negative offset.
      ```
      127.0.0.1:6379> set key value
      OK
      127.0.0.1:6379> zrangestore key myzset 0 10 byscore limit -1 10
      (integer) 0
      127.0.0.1:6379> exists key
      (integer) 0
      ```
      
      This change affects the following commands:
      - ZRANGE / ZRANGESTORE / ZRANGEBYLEX / ZRANGEBYSCORE
      - ZREVRANGE / ZREVRANGEBYSCORE / ZREVRANGEBYLEX
      d9c2ef8a
    • Wen Hui's avatar
      adding geo command edge cases tests (#12274) · 66ea178c
      Wen Hui authored
      For geosearch and georadius we have already test coverage for wrong type, but we dont have for geodist, geohash, geopos commands. So adding the wrong type test cases for geodist, geohash, geopos commands.
      
      Existing code, we have verify_geo_edge_response_bymember function for wrong type test cases which has member as an option. But the function is being called in other test cases where the output is not inline with these commnds(geodist, geohash, geopos). So I could not include these commands(geodist, geohash, geopos) as part of existing function, hence implemented a new function verify_geo_edge_response_generic and called from the test case.
      66ea178c
    • Binbin's avatar
      Fix ZRANK/ZREVRANK reply_schema description (#12331) · d306d861
      Binbin authored
      The parameter name is WITHSCORE instead of WITHSCORES.
      d306d861
    • Wen Hui's avatar
      Sanitizer reported memory leak for '--invalid' option or port number is missed... · 813924b4
      Wen Hui authored
      
      Sanitizer reported memory leak for '--invalid' option or port number is missed cases to redis-server. (#12322)
      
      Observed that the sanitizer reported memory leak as clean up is not done
      before the process termination in negative/following cases:
      
      **- when we passed '--invalid' as option to redis-server.**
      
      ```
       -vm:~/mem-leak-issue/redis$ ./src/redis-server --invalid
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 2
      >>> 'invalid'
      Bad directive or wrong number of arguments
      
      =================================================================
      ==865778==ERROR: LeakSanitizer: detected memory leaks
      
      Direct leak of 8 byte(s) in 1 object(s) allocated from:
          #0 0x7f0985f65867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
          #1 0x558ec86686ec in ztrymalloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:117
          #2 0x558ec86686ec in ztrymalloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:135
          #3 0x558ec86686ec in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:276
          #4 0x558ec86686ec in zrealloc /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:327
          #5 0x558ec865dd7e in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1172
          #6 0x558ec87a1be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
          #7 0x558ec87a13b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
          #8 0x558ec85e6f15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
          #9 0x7f09856e5d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
      
      SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).
      
      ```
      
      **- when we pass '--port' as option and missed to add port number to redis-server.**
      
      ```
      vm:~/mem-leak-issue/redis$ ./src/redis-server --port
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 2
      >>> 'port'
      wrong number of arguments
      
      =================================================================
      ==865846==ERROR: LeakSanitizer: detected memory leaks
      
      Direct leak of 8 byte(s) in 1 object(s) allocated from:
          #0 0x7fdcdbb1f867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
          #1 0x557e8b04f6ec in ztrymalloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:117
          #2 0x557e8b04f6ec in ztrymalloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:135
          #3 0x557e8b04f6ec in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:276
          #4 0x557e8b04f6ec in zrealloc /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:327
          #5 0x557e8b044d7e in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1172
          #6 0x557e8b188be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
          #7 0x557e8b1883b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
          #8 0x557e8afcdf15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
          #9 0x7fdcdb29fd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
      
      Indirect leak of 10 byte(s) in 1 object(s) allocated from:
          #0 0x7fdcdbb1fc18 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
          #1 0x557e8b04f9aa in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:287
          #2 0x557e8b04f9aa in ztryrealloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:317
          #3 0x557e8b04f9aa in zrealloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:342
          #4 0x557e8b033f90 in _sdsMakeRoomFor /home/ubuntu/mem-leak-issue/redis/src/sds.c:271
          #5 0x557e8b033f90 in sdsMakeRoomFor /home/ubuntu/mem-leak-issue/redis/src/sds.c:295
          #6 0x557e8b033f90 in sdscatlen /home/ubuntu/mem-leak-issue/redis/src/sds.c:486
          #7 0x557e8b044e1f in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1165
          #8 0x557e8b188be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
          #9 0x557e8b1883b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
          #10 0x557e8afcdf15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
          #11 0x7fdcdb29fd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
      
      SUMMARY: AddressSanitizer: 18 byte(s) leaked in 2 allocation(s).
      
      ```
      
      As part analysis found that the sdsfreesplitres is not called when this condition checks are being hit.
      
      Output after the fix:
      
      
      ```
      vm:~/mem-leak-issue/redis$ ./src/redis-server --invalid
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 2
      >>> 'invalid'
      Bad directive or wrong number of arguments
      vm:~/mem-leak-issue/redis$
      
      ===========================================
      vm:~/mem-leak-issue/redis$ ./src/redis-server --jdhg
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 2
      >>> 'jdhg'
      Bad directive or wrong number of arguments
      
      ---------------------------------------------------------------------------
      vm:~/mem-leak-issue/redis$ ./src/redis-server --port
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 2
      >>> 'port'
      wrong number of arguments
      ```
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      813924b4
    • Shaya Potter's avatar
      Add ability for modules to know which client is being cmd filtered (#12219) · 07316f16
      Shaya Potter authored
      Adds API
      - RedisModule_CommandFilterGetClientId()
      
      Includes addition to commandfilter test module to validate that it works
      by performing the same command from 2 different clients
      07316f16
  25. 18 Jun, 2023 1 commit
    • Wen Hui's avatar
      Cluster human readable nodename feature (#9564) · 070453ee
      Wen Hui authored
      
      
      This PR adds a human readable name to a node in clusters that are visible as part of error logs. This is useful so that admins and operators of Redis cluster have better visibility into failures without having to cross-reference the generated ID with some logical identifier (such as pod-ID or EC2 instance ID). This is mentioned in #8948. Specific nodenames can be set by using the variable cluster-announce-human-nodename. The nodename is gossiped using the clusterbus extension in #9530.
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      070453ee
  26. 16 Jun, 2023 3 commits
    • sundb's avatar
      Use Reservoir Sampling for random sampling of dict, and fix hang during fork (#12276) · b00a2351
      sundb authored
      
      
      ## Issue:
      When a dict has a long chain or the length of the chain is longer than
      the number of samples, we will never be able to sample the elements
      at the end of the chain using dictGetSomeKeys().
      This could mean that SRANDMEMBER can be hang in and endless loop.
      The most severe case, is the pathological case of when someone uses SCAN+DEL
      or SSCAN+SREM creating an unevenly distributed dict.
      This was amplified by the recent change in #11692 which prevented a
      down-sizing rehashing while there is a fork.
      
      ## Solution
      1. Before, we will stop sampling when we reach the maximum number
        of samples, even if there is more data after the current chain.
        Now when we reach the maximum we use the Reservoir Sampling
        algorithm to fairly sample the end of the chain that cannot be sampled
      2. Fix the rehashing code, so that the same as it allows rehashing for up-sizing
        during fork when the ratio is extreme, it will allow it for down-sizing as well.
      
      Issue was introduced (or became more severe) by #11692
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      b00a2351
    • Binbin's avatar
      Fix SPOP/RESTORE propagation when doing lazy free (#12320) · 439b0315
      Binbin authored
      In SPOP, when COUNT is greater than or equal to set's size,
      we will remove the set. In dbDelete, we will do DEL or UNLINK
      according to the lazy flag. This is also required for propagate.
      
      In RESTORE, we won't store expired keys into the db, see #7472.
      When used together with REPLACE, it should emit a DEL or UNLINK
      according to the lazy flag.
      
      This PR also adds tests to cover the propagation. The RESTORE
      test will also cover #7472.
      439b0315
    • YaacovHazan's avatar
      Removing duplicated tests (#12318) · 5da9eecd
      YaacovHazan authored
      In 4ba47d2d the following tests added in both tracking.tcl and introspection.tcl
      
      - Coverage: Basic CLIENT CACHING
      - Coverage: Basic CLIENT REPLY
      - Coverage: Basic CLIENT TRACKINGINFO
      - Coverage: Basic CLIENT GETREDIR
      5da9eecd