1. 21 Sep, 2022 24 commits
    • sundb's avatar
      Fix crash due to delete entry from compress quicklistNode and wrongly split quicklistNode (#11242) · 01358df3
      sundb authored
      This PR mainly deals with 2 crashes introduced in #9357,
      and fix the QUICKLIST-PACKED-THRESHOLD mess in external test mode.
      
      1. Fix crash due to deleting an entry from a compress quicklistNode
         When inserting a large element, we need to create a new quicklistNode first,
         and then delete its previous element, if the node where the deleted element is
         located is compressed, it will cause a crash.
         Now add `dont_compress` to quicklistNode, if we want to use a quicklistNode
         after some operation, we can use this flag like following:
      
          ```c
          node->dont_compress = 1; /* Prevent to be compressed */
          some_operation(node); /* This operation might try to compress this node */
          some_other_operation(node); /* We can use this node without decompress it */
          node->dont_compress = 0; /* Re-able compression */
          quicklistCompressNode(node);
          ```
      
         Perhaps in the future, we could just disable the current entry from being
         compressed during the iterator loop, but that would require more work.
      
      2. Fix crash due to wrongly split quicklist
         before #9357, the offset param of _quicklistSplitNode() will not negative.
         For now, when offset is negative, the split extent will be wrong.
         following example:
          ```c
          int orig_start = after ? offset + 1 : 0;
          int orig_extent = after ? -1 : offset;
          int new_start = after ? 0 : offset;
          int new_extent = after ? offset + 1 : -1;
          # offset: -2, after: 1, node->count: 2
          # current wrong range: [-1,-1] [0,-1]
          # correct range: [1,-1] [0, 1]
          ```
      
         Because only `_quicklistInsert()` splits the quicklistNode and only
         `quicklistInsertAfter()`, `quicklistInsertBefore()` call _quicklistInsert(), 
         so `quicklistReplaceEntry()` and `listTypeInsert()` might occur this crash.
         But the iterator of `listTypeInsert()` is alway from head to tail(iter->offset is
         always positive), so it is not affected.
         The final conclusion is this crash only occur when we insert a large element
         with negative index into a list, that affects `LSET` command and `RM_ListSet`
         module api.
           
      3. In external test mode, we need to restore quicklist packed threshold after
         when the end of test.
      4. Show `node->count` in quicklistRepr().
      5. Add new tcl proc `config_get_set` to support restoring config in tests.
      
      (cherry picked from commit 13d25dd9)
      01358df3
    • zhaozhao.zz's avatar
      fix infinite sleep in performEvictions when have lazyfree jobs (#11237) · 2f6537dc
      zhaozhao.zz authored
      This bug is introduced in #7653. (Redis 6.2.0)
      
      When `server.maxmemory_eviction_tenacity` is 100, `eviction_time_limit_us` is
      `ULONG_MAX`, and if we cannot find the best key to delete (e.g. maxmemory-policy
      is `volatile-lru` and all keys with ttl have been evicted), in `cant_free` redis will sleep
      forever if some items are being freed in the lazyfree thread.
      
      (cherry picked from commit 464aa041)
      2f6537dc
    • Viktor Söderqvist's avatar
      6edbf707
    • Madelyn Olson's avatar
      Prevent use after free for inbound cluster link (#11255) · 7fc1a8c5
      Madelyn Olson authored
      
      (cherry picked from commit 6c03786b)
      7fc1a8c5
    • chendianqiang's avatar
      Correctly handle scripts with shebang (not read-only) on a cluster replica (#11223) · 8c702f8d
      chendianqiang authored
      
      
      EVAL scripts are by default not considered `write` commands, so they were allowed on a replica.
      But when adding a shebang, they become `write` command (unless the `no-writes` flag is added).
      With this change we'll handle them as write commands, and reply with MOVED instead of
      READONLY when executed on a redis cluster replica.
      Co-authored-by: default avatarchendianqiang <chendianqiang@meituan.com>
      (cherry picked from commit e42d98ed)
      8c702f8d
    • Huang Zhw's avatar
      Remove the NONDETERMINISTIC_OUTPUT flag from most CLUSTER sub-commands. (#11157) · 63db10ef
      Huang Zhw authored
      TLDR: the CLUSTER command originally had the `random` flag,
      so all the sub-commands initially got that new flag, but in fact many
      of them don't need it.
      The only effect of this change is on the output of COMMAND INFO.
      
      (cherry picked from commit a7da7473)
      63db10ef
    • Oran Agra's avatar
      Fix assertion when a key is lazy expired during cluster key migration (#11176) · a2a28b80
      Oran Agra authored
      Redis 7.0 has #9890 which added an assertion when the propagation queue
      was not flushed and we got to beforeSleep.
      But it turns out that when processCommands calls getNodeByQuery and
      decides to reject the command, it can lead to a key that was lazy
      expired and is deleted without later flushing the propagation queue.
      
      This change prevents lazy expiry from deleting the key at this stage
      (not as part of a command being processed in `call`)
      
      (cherry picked from commit c789fb0a)
      a2a28b80
    • Itamar Haber's avatar
      Replaces a made-up term with a real one (#11169) · 1e2876e0
      Itamar Haber authored
      (cherry picked from commit 407b5c91)
      1e2876e0
    • Itamar Haber's avatar
      Adds historical note about lower-case geo units support (#11162) · dbb7337e
      Itamar Haber authored
      This change was part of #9656 (Redis 7.0)
      
      (cherry picked from commit 31ef410e)
      dbb7337e
    • Binbin's avatar
      Fix memory leak in moduleFreeCommand (#11147) · fa60cd13
      Binbin authored
      Currently, we call zfree(cmd->args), but the argument array
      needs to be freed recursively (there might be sub-args).
      Also fixed memory leaks on cmd->tips and cmd->history.
      
      Fixes #11145
      
      (cherry picked from commit fc3956e8)
      fa60cd13
    • DarrenJiang13's avatar
      fix the client type in trackingInvalidateKey() (#11052) · 4dfc487f
      DarrenJiang13 authored
      Fix bug with scripts ignoring client tracking NOLOOP and
      send an invalidation message anyway.
      
      (cherry picked from commit 44859a41)
      4dfc487f
    • Huang Zhw's avatar
      acl: bitfield with get and set|incrby can be executed with readonly permission (#11086) · ab524282
      Huang Zhw authored
      `bitfield` with `get` may not be readonly.
      
      ```
      127.0.0.1:6384> acl setuser hello on nopass %R~* +@all
      
      
      OK
      127.0.0.1:6384> auth hello 1
      OK
      127.0.0.1:6384> bitfield hello set i8 0 1
      (error) NOPERM this user has no permissions to access one of the keys used as arguments
      127.0.0.1:6384> bitfield hello set i8 0 1 get i8 0
      1) (integer) 0
      2) (integer) 1
      ```
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit ec5034a2)
      ab524282
    • Wen Hui's avatar
      Fix function load error message (#10964) · d1b3276b
      Wen Hui authored
      Update error messages for function load
      
      (cherry picked from commit beb9746a)
      d1b3276b
    • Rudi Floren's avatar
      Fix wrong commands json docs for CLIENT KILL (#10970) · 631eb383
      Rudi Floren authored
      The docs state that there is a new and an old argument format.
      The current state of the arguments allows mixing the old and new format,
      thus the need for two additional oneof blocks.
      One for differentiating the new from the old format and then one to
      allow setting multiple filters using the new format.
      
      (cherry picked from commit 4ce3fd51)
      631eb383
    • Huang Zhw's avatar
      tracking pending invalidation message of flushdb sent by (#11068) · 5632bbdc
      Huang Zhw authored
      trackingHandlePendingKeyInvalidations should use proto.
      
      (cherry picked from commit 61451b02)
      5632bbdc
    • Huang Zhw's avatar
      When client tracking is on, invalidation message of flushdb in a (#11038) · ba6ed445
      Huang Zhw authored
      When FLUSHDB / FLUSHALL / SWAPDB is inside MULTI / EXEC, the
      client side tracking invalidation message was interleaved with transaction response.
      
      
      (cherry picked from commit 6f0a27e3)
      ba6ed445
    • Meir Shpilraien (Spielrein)'s avatar
      Fix #11030, use lua_rawget to avoid triggering metatables and crash. (#11032) · ddf1bcf7
      Meir Shpilraien (Spielrein) authored
      Fix #11030, use lua_rawget to avoid triggering metatables.
      
      #11030 shows how return `_G` from the Lua script (either function or eval), cause the
      Lua interpreter to Panic and the Redis processes to exit with error code 1.
      Though return `_G` only panic on Redis 7 and 6.2.7, the underline issue exists on older
      versions as well (6.0 and 6.2). The underline issue is returning a table with a metatable
      such that the metatable raises an error.
      
      The following example demonstrate the issue:
      ```
      127.0.0.1:6379> eval "local a = {}; setmetatable(a,{__index=function() foo() end}) return a" 0
      Error: Server closed the connection
      ```
      ```
      PANIC: unprotected error in call to Lua API (user_script:1: Script attempted to access nonexistent global variable 'foo')
      ```
      
      The Lua panic happened because when returning the result to the client, Redis needs to
      introspect the returning table and transform the table into a resp. In order to scan the table,
      Redis uses `lua_gettable` api which might trigger the metatable (if exists) and might raise an error.
      This code is not running inside `pcall` (Lua protected call), so raising an error causes the
      Lua to panic and exit. Notice that this is not a crash, its a Lua panic that exit with error code 1.
      
      Returning `_G` panics on Redis 7 and 6.2.7 because on those versions `_G` has a metatable
      that raises error when trying to fetch a none existing key.
      
      ### Solution
      
      Instead of using `lua_gettable` that might raise error and cause the issue, use `lua_rawget`
      that simply return the value from the table without triggering any metatable logic.
      This is promised not to raise and error.
      
      The downside of this solution is that it might be considered as breaking change, if someone
      rely on metatable in the returned value. An alternative solution is to wrap this entire logic
      with `pcall` (Lua protected call), this alternative require a much bigger refactoring.
      
      ### Back Porting
      
      The same fix will work on older versions as well (6.2, 6.0). Notice that on those version,
      the issue can cause Redis to crash if inside the metatable logic there is an attempt to accesses
      Redis (`redis.call`). On 7.0, there is not crash and the `redis.call` is executed as if it was done
      from inside the script itself.
      
      ### Tests
      
      Tests was added the verify the fix
      
      (cherry picked from commit 020e046b)
      ddf1bcf7
    • Binbin's avatar
      fsync the old aof file when open a new INCR AOF (#11004) · 5da7fdb7
      Binbin authored
      
      
      In rewriteAppendOnlyFileBackground, after flushAppendOnlyFile(1),
      and before openNewIncrAofForAppend, we should call redis_fsync
      to fsync the aof file.
      
      Because we may open a new INCR AOF in openNewIncrAofForAppend,
      in the case of using everysec policy, the old AOF file may not
      be fsynced in time (or even at all).
      
      When using everysec, we don't want to pay the disk latency from
      the main thread, so we will do a background fsync.
      
      Adding a argument for bioCreateCloseJob, a `need_fsync` flag to
      indicate that a fsync is required before the file is closed. So we will
      fsync the old AOF file before we close it.
      
      A cleanup, we make union become a union, since the free_* args and
      the fd / fsync args are never used together.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 03fff10a)
      5da7fdb7
    • chenyang8094's avatar
      Register abs-expire apis (#11025) · c0394286
      chenyang8094 authored
      RM_SetAbsExpire and RM_GetAbsExpire were not actually operational since
      they were introduced, due to omission in API registration.
      
      (cherry picked from commit 39d216a3)
      c0394286
    • Pavel Krush's avatar
      fixed complexity of bzmpop and zmpop commands (#11026) · 61a59b53
      Pavel Krush authored
      
      
      Swap M and N in the complexity formula of [B]ZMPOP
      Co-authored-by: default avatarPavel Krush <neon@pushwoosh.com>
      (cherry picked from commit 5879e490)
      61a59b53
    • Tian's avatar
      Don't update node ip when peer fd is closed (#10696) · babe90c9
      Tian authored
      
      (cherry picked from commit d00b8af8)
      babe90c9
    • guybe7's avatar
      Adds LASTID to XCLAIM docs (#11017) · 9e3dfa96
      guybe7 authored
      It seems it was overlooked when we first created the json files
      
      (cherry picked from commit 6d6e932f)
      9e3dfa96
    • Wen Hui's avatar
      Fix EVALSHA_RO and EVAL_RO command json file (#11015) · d28314cb
      Wen Hui authored
      
      
      these are missing from the RO_ commands, present in the other ones.
      Co-authored-by: default avatarUbuntu <lucas.guang.yang1@huawei.com>
      (cherry picked from commit 56828bab)
      d28314cb
    • Wen Hui's avatar
      Add optional for FCALL and FCALL_RO command json file (#10988) · a8b6efea
      Wen Hui authored
      
      
      According to the Redis functions documentation, FCALL command format could be
      FCALL function_name numberOfKeys [key1, key2, key3.....] [arg1, arg2, arg3.....]
      
      So in the json file of fcall and fcall_ro, we should add optional for key and arg part.
      Just like EVAL...
      Co-authored-by: default avatarBinbin <binloveplay1314@qq.com>
      (cherry picked from commit 2d3240f3)
      a8b6efea
  2. 18 Jul, 2022 3 commits
  3. 11 Jul, 2022 5 commits
    • Oran Agra's avatar
      7.0.3 · 85abb7cf
      Oran Agra authored
      85abb7cf
    • Oran Agra's avatar
      update help.h (#10961) · b4568fc7
      Oran Agra authored
      b4568fc7
    • Binbin's avatar
      Add cluster-port support to redis-cli --cluster (#10344) · 35e8ae3e
      Binbin authored
      
      
      In #9389, we add a new `cluster-port` config and make cluster bus port configurable,
      and currently redis-cli --cluster create/add-node doesn't support with a configurable `cluster-port` instance.
      Because redis-cli uses the old way (port + 10000) to send the `CLUSTER MEET` command.
      
      Now we add this support on redis-cli `--cluster`, note we don't need to explicitly pass in the
      `cluster-port` parameter, we can get the real `cluster-port` of the node in `clusterManagerNodeLoadInfo`,
      so the `--cluster create` and `--cluster add-node` interfaces have not changed.
      
      We will use the `cluster-port` when we are doing `CLUSTER MEET`, also note that `CLUSTER MEET` bus-port
      parameter was added in 4.0, so if the bus_port (the one in redis-cli) is 0, or equal (port + 10000),
      we just call `CLUSTER MEET` with 2 arguments, using the old form.
      Co-authored-by: default avatarMadelyn Olson <34459052+madolson@users.noreply.github.com>
      35e8ae3e
    • Binbin's avatar
      Add --check-system in redis-server usage (#10960) · 1e85b89a
      Binbin authored
      1e85b89a
    • Madelyn Olson's avatar
      Fix crash during handshake and cluster shards call (#10942) · e6a1b2ea
      Madelyn Olson authored
      * Fix an engine crash when there are nodes in handshaking and a user calls cluster shards
      e6a1b2ea
  4. 10 Jul, 2022 1 commit
  5. 07 Jul, 2022 1 commit
  6. 06 Jul, 2022 1 commit
  7. 05 Jul, 2022 1 commit
  8. 04 Jul, 2022 4 commits
    • Harkrishn Patro's avatar
      9bcdd153
    • Harkrishn Patro's avatar
      Optimize number of realloc syscall during multi/exec flow (#10921) · a3704d4e
      Harkrishn Patro authored
      ## Issue
      During the MULTI/EXEC flow, each command gets queued until the `EXEC`
      command is received and during this phase on every command queue, a
      `realloc` is being invoked. This could be expensive based on the realloc
      behavior (if copy to a new memory location). 
      
      
      ## Solution
      In order to reduce the no. of syscall, couple of optimization I've used.
      
      1. By default, reserve memory for atleast two commands. `MULTI/EXEC` for a
      single command doesn't have any significance. Hence, I believe customer wouldn't use it.
      2. For further reservation, increase the memory allocation in exponent growth (power of 2).
      This reduces the no. of `realloc` call from `N` to `log(N)` times.
      
      ## Other changes:
      
      * Include multi exec queued command array in client memory consumption calculation
      (affects client eviction too)
      a3704d4e
    • Qu Chen's avatar
      Unlock cluster config file upon server shutdown. (#10912) · 33b7ff38
      Qu Chen authored
      Currently in cluster mode, Redis process locks the cluster config file when
      starting up and holds the lock for the entire lifetime of the process.
      When the server shuts down, it doesn't explicitly release the lock on the
      cluster config file. We noticed a problem with restart testing that if you shut down
      a very large redis-server process (i.e. with several hundred GB of data stored),
      it takes the OS a while to free the resources and unlock the cluster config file.
      So if we immediately try to restart the redis server process, it might fail to acquire
      the lock on the cluster config file and fail to come up.
      
      This fix explicitly releases the lock on the cluster config file upon a shutdown rather
      than relying on the OS to release the lock, which is a cleaner and safer approach to
      free up resources acquired. 
      33b7ff38
    • Harkrishn Patro's avatar
      Account sharded pubsub channels memory consumption (#10925) · 0ab885a6
      Harkrishn Patro authored
      Account sharded pubsub channels memory consumption in client memory usage
      computation to accurately evict client based on the set threshold for `maxmemory-clients`.
      0ab885a6