1. 13 Nov, 2021 1 commit
    • Binbin's avatar
      Tune expire test threshold. (#9775) · 174eedce
      Binbin authored
      I have seen this CI failure twice on MacOS:
      
      *** [err]: PEXPIRE/PSETEX/PEXPIREAT can set sub-second expires in tests/unit/expire.tcl
      Expected 'somevalue {} somevalue {} somevalue {}' to equal or match '{} {} {} {} somevalue {}'
      
      I did some loop test in my own daily CI, the results show that is
      not particularly stable. Change the threshold from 30 to 50.
      174eedce
  2. 09 Nov, 2021 1 commit
    • YaacovHazan's avatar
      fix short timeout in replication short read tests (#9763) · 03406fcb
      YaacovHazan authored
      In both tests, "diskless loading short read" and "diskless loading short read with module",
      the timeout of waiting for the replica to respond to a short read and log it, is too short.
      
      Also, add --dump-logs in runtest-moduleapi for valgrind runs.
      03406fcb
  3. 04 Nov, 2021 2 commits
    • Eduardo Semprebon's avatar
      Replica keep serving data during repl-diskless-load=swapdb for better availability (#9323) · 91d0c758
      Eduardo Semprebon authored
      
      
      For diskless replication in swapdb mode, considering we already spend replica memory
      having a backup of current db to restore in case of failure, we can have the following benefits
      by instead swapping database only in case we succeeded in transferring db from master:
      
      - Avoid `LOADING` response during failed and successful synchronization for cases where the
        replica is already up and running with data.
      - Faster total time of diskless replication, because now we're moving from Transfer + Flush + Load
        time to Transfer + Load only. Flushing the tempDb is done asynchronously after swapping.
      - This could be implemented also for disk replication with similar benefits if consumers are willing
        to spend the extra memory usage.
      
      General notes:
      - The concept of `backupDb` becomes `tempDb` for clarity.
      - Async loading mode will only kick in if the replica is syncing from a master that has the same
        repl-id the one it had before. i.e. the data it's getting belongs to a different time of the same timeline. 
      - New property in INFO: `async_loading` to differentiate from the blocking loading
      - Slot to Key mapping is now a field of `redisDb` as it's more natural to access it from both server.db
        and the tempDb that is passed around.
      - Because this is affecting replicas only, we assume that if they are not readonly and write commands
        during replication, they are lost after SYNC same way as before, but we're still denying CONFIG SET
        here anyways to avoid complications.
      
      Considerations for review:
      - We have many cases where server.loading flag is used and even though I tried my best, there may
        be cases where async_loading should be checked as well and cases where it shouldn't (would require
        very good understanding of whole code)
      - Several places that had different behavior depending on the loading flag where actually meant to just
        handle commands coming from the AOF client differently than ones coming from real clients, changed
        to check CLIENT_ID_AOF instead.
      
      **Additional for Release Notes**
      - Bugfix - server.dirty was not incremented for any kind of diskless replication, as effect it wouldn't
        contribute on triggering next database SAVE
      - New flag for RM_GetContextFlags module API: REDISMODULE_CTX_FLAGS_ASYNC_LOADING
      - Deprecated RedisModuleEvent_ReplBackup. Starting from Redis 7.0, we don't fire this event.
        Instead, we have the new RedisModuleEvent_ReplAsyncLoad holding 3 sub-events: STARTED,
        ABORTED and COMPLETED.
      - New module flag REDISMODULE_OPTIONS_HANDLE_REPL_ASYNC_LOAD for RedisModule_SetModuleOptions
        to allow modules to declare they support the diskless replication with async loading (when absent, we fall
        back to disk-based loading).
      Co-authored-by: default avatarEduardo Semprebon <edus@saxobank.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      91d0c758
    • Itamar Haber's avatar
      Fixes LPOP/RPOP wrong replies when count is 0 (#9692) · 06dd202a
      Itamar Haber authored
      Introduced in #8179, this fixes the command's replies in the 0 count edge case.
      [BREAKING] changes the reply type when count is 0 to an empty array (instead of nil)
      Moves LPOP ... 0 fast exit path after type check to reply with WRONGTYPE
      06dd202a
  4. 03 Nov, 2021 2 commits
    • perryitay's avatar
      Add support for list type to store elements larger than 4GB (#9357) · f27083a4
      perryitay authored
      
      
      Redis lists are stored in quicklist, which is currently a linked list of ziplists.
      Ziplists are limited to storing elements no larger than 4GB, so when bigger
      items are added they're getting truncated.
      This PR changes quicklists so that they're capable of storing large items
      in quicklist nodes that are plain string buffers rather than ziplist.
      
      As part of the PR there were few other changes in redis: 
      1. new DEBUG sub-commands: 
         - QUICKLIST-PACKED-THRESHOLD - set the threshold of for the node type to
           be plan or ziplist. default (1GB)
         - QUICKLIST <key> - Shows low level info about the quicklist encoding of <key>
      2. rdb format change:
         - A new type was added - RDB_TYPE_LIST_QUICKLIST_2 . 
         - container type (packed / plain) was added to the beginning of the rdb object
           (before the actual node list).
      3. testing:
         - Tests that requires over 100MB will be by default skipped. a new flag was
           added to 'runtest' to run the large memory tests (not used by default)
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      f27083a4
    • guybe7's avatar
      Fix COMMAND GETKEYS on EVAL without keys (#9733) · f11a2d4d
      guybe7 authored
      Add new no-mandatory-keys flag to support COMMAND GETKEYS of commands
      which have no mandatory keys.
      
      In the past we would have got this error:
      ```
      127.0.0.1:6379> command getkeys eval "return 1" 0
      (error) ERR Invalid arguments specified for command
      ```
      f11a2d4d
  5. 02 Nov, 2021 3 commits
    • Oran Agra's avatar
      Solve issues with tracking test in external mode (#9726) · d25dc089
      Oran Agra authored
      The issue was that setting maxmemory to used_memory and expecting
      eviction is insufficient, since we need to take
      mem_not_counted_for_evict into consideration.
      
      This test got broken by #9166
      d25dc089
    • Oran Agra's avatar
      attempt to fix tracking test issue with external tests due to lazy free (#9722) · 87321deb
      Oran Agra authored
      The External tests started failing recently for unclear reason:
      ```
      *** [err]: Tracking invalidation message of eviction keys should be before response in tests/unit/tracking.tcl
      Expected '0' to be equal to 'invalidate volatile-key' (context: type eval line 21 cmd {assert_equal $res {invalidate volatile-key}} proc ::test)
      ```
      
      I suspect the issue is that the used_memory sample is taken while a lazy free is still being processed.
      87321deb
    • menwen's avatar
      fix defrag test looking at the wrong latency metric (#9723) · d5ca72e3
      menwen authored
      the latency event was renamed in #7726, and the outcome was that the test was
      ineffective (unable to measure the max latency, always seeing 0)
      d5ca72e3
  6. 01 Nov, 2021 1 commit
    • Oran Agra's avatar
      fix valgrind issues with long double module test (#9709) · f1f3cceb
      Oran Agra authored
      The module test in reply.tcl was introduced by #8521 but didn't run until recently (see #9639)
      and then it started failing with valgrind.
      This is because valgrind uses 64 bit long double (unlike most other platforms that have at least 80 bits)
      But besides valgrind, the tests where also incompatible with ARM32, which also uses 64 bit long doubles.
      
      We now use appropriate value to avoid issues with either valgrind or ARM32
      
      In all the double tests, i use 3.141, which is safe since since addReplyDouble uses
      `%.17Lg` which is able to represent this value without adding any digits due to precision loss. 
      
      In the long double, since we use `%.17Lf` in ld2string, it preserves 17 significant
      digits, rather than 17 digit after the decimal point (like in `%.17Lg`).
      So to make these similar, i use value lower than 1 (no digits left of
      the period)
      
      Lastly, we have the same issue with TCL (no long doubles) so we read
      raw protocol in that test.
      
      Note that the only error before this fix (in both valgrind and ARM32 is this:
      ```
      *** [err]: RM_ReplyWithLongDouble: a float reply in tests/unit/moduleapi/reply.tcl
      Expected '3.141' to be equal to '3.14100000000000001' (context: type eval line 2 cmd {assert_equal 3.141 [r rw.longdouble 3.141]} proc ::test)
      ```
      so the changes to debug.c and scripting.tcl aren't really needed, but i consider them a cleanup
      (i.e. scripting.c validated a different constant than the one that's sent to it from debug.c).
      
      Another unrelated change is to add the RESP version to the repeated tests in reply.tcl
      f1f3cceb
  7. 31 Oct, 2021 1 commit
    • Binbin's avatar
      Fix multiple COUNT in LMPOP/BLMPOP/ZMPOP/BZMPOP (#9701) · 03357883
      Binbin authored
      The previous code did not check whether COUNT is set.
      So we can use `lmpop 2 key1 key2 left count 1 count 2`.
      
      This situation can occur in LMPOP/BLMPOP/ZMPOP/BZMPOP commands.
      LMPOP/BLMPOP introduced in #9373, ZMPOP/BZMPOP introduced in #9484.
      03357883
  8. 26 Oct, 2021 1 commit
  9. 25 Oct, 2021 2 commits
    • Shaya Potter's avatar
      Add RM_ReplyWithBigNumber module API (#9639) · 12ce2c39
      Shaya Potter authored
      
      
      Let modules use additional type of RESP3 response (unused by redis so far)
      Also fix tests that where introduced in #8521 but didn't actually run.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      12ce2c39
    • Wang Yuan's avatar
      Replication backlog and replicas use one global shared replication buffer (#9166) · c1718f9d
      Wang Yuan authored
      ## Background
      For redis master, one replica uses one copy of replication buffer, that is a big waste of memory,
      more replicas more waste, and allocate/free memory for every reply list also cost much.
      If we set client-output-buffer-limit small and write traffic is heavy, master may disconnect with
      replicas and can't finish synchronization with replica. If we set  client-output-buffer-limit big,
      master may be OOM when there are many replicas that separately keep much memory.
      Because replication buffers of different replica client are the same, one simple idea is that
      all replicas only use one replication buffer, that will effectively save memory.
      
      Since replication backlog content is the same as replicas' output buffer, now we
      can discard replication backlog memory and use global shared replication buffer
      to implement replication backlog mechanism.
      
      ## Implementation
      I create one global "replication buffer" which contains content of replication stream.
      The structure of "replication buffer" is similar to the reply list that exists in every client.
      But the node of list is `replBufBlock`, which has `id, repl_offset, refcount` fields.
      ```c
      /* Replication buffer blocks is the list of replBufBlock.
       *
       * +--------------+       +--------------+       +--------------+
       * | refcount = 1 |  ...  | refcount = 0 |  ...  | refcount = 2 |
       * +--------------+       +--------------+       +--------------+
       *      |                                            /       \
       *      |                                           /         \
       *      |                                          /           \
       *  Repl Backlog                               Replia_A      Replia_B
       * 
       * Each replica or replication backlog increments only the refcount of the
       * 'ref_repl_buf_node' which it points to. So when replica walks to the next
       * node, it should first increase the next node's refcount, and when we trim
       * the replication buffer nodes, we remove node always from the head node which
       * refcount is 0. If the refcount of the head node is not 0, we must stop
       * trimming and never iterate the next node. */
      
      /* Similar with 'clientReplyBlock', it is used for shared buffers between
       * all replica clients and replication backlog. */
      typedef struct replBufBlock {
          int refcount;           /* Number of replicas or repl backlog using. */
          long long id;           /* The unique incremental number. */
          long long repl_offset;  /* Start replication offset of the block. */
          size_t size, used;
          char buf[];
      } replBufBlock;
      ```
      So now when we feed replication stream into replication backlog and all replicas, we only need
      to feed stream into replication buffer `feedReplicationBuffer`. In this function, we set some fields of
      replication backlog and replicas to references of the global replication buffer blocks. And we also
      need to check replicas' output buffer limit to free if exceeding `client-output-buffer-limit`, and trim
      replication backlog if exceeding `repl-backlog-size`.
      
      When sending reply to replicas, we also need to iterate replication buffer blocks and send its
      content, when totally sending one block for replica, we decrease current node count and
      increase the next current node count, and then free the block which reference is 0 from the
      head of replication buffer blocks.
      
      Since now we use linked list to manage replication backlog, it may cost much time for iterating
      all linked list nodes to find corresponding replication buffer node. So we create a rax tree to
      store some nodes  for index, but to avoid rax tree occupying too much memory, i record
      one per 64 nodes for index.
      
      Currently, to make partial resynchronization as possible as much, we always let replication
      backlog as the last reference of replication buffer blocks, backlog size may exceeds our setting
      if slow replicas that reference vast replication buffer blocks, and this method doesn't increase
      memory usage since they share replication buffer. To avoid freezing server for freeing unreferenced
      replication buffer blocks when we need to trim backlog for exceeding backlog size setting,
      we trim backlog incrementally (free 64 blocks per call now), and make it faster in
      `beforeSleep` (free 640 blocks).
      
      ### Other changes
      - `mem_total_replication_buffers`: we add this field in INFO command, it means the total
        memory of replication buffers used.
      - `mem_clients_slaves`:  now even replica is slow to replicate, and its output buffer memory
        is not 0, but it still may be 0, since replication backlog and replicas share one global replication
        buffer, only if replication buffer memory is more than the repl backlog setting size, we consider
        the excess as replicas' memory. Otherwise, we think replication buffer memory is the consumption
        of repl backlog.
      - Key eviction
        Since all replicas and replication backlog share global replication buffer, we think only the
        part of exceeding backlog size the extra separate consumption of replicas.
        Because we trim backlog incrementally in the background, backlog size may exceeds our
        setting if slow replicas that reference vast replication buffer blocks disconnect.
        To avoid massive eviction loop, we don't count the delayed freed replication backlog into
        used memory even if there are no replicas, i.e. we also regard this memory as replicas's memory.
      - `client-output-buffer-limit` check for replica clients
        It doesn't make sense to set the replica clients output buffer limit lower than the repl-backlog-size
        config (partial sync will succeed and then replica will get disconnected). Such a configuration is
        ignored (the size of repl-backlog-size will be used). This doesn't have memory consumption
        implications since the replica client will share the backlog buffers memory.
      - Drop replication backlog after loading data if needed
        We always create replication backlog if server is a master, we need it because we put DELs in
        it when loading expired keys in RDB, but if RDB doesn't have replication info or there is no rdb,
        it is not possible to support partial resynchronization, to avoid extra memory of replication backlog,
        we drop it.
      - Multi IO threads
       Since all replicas and replication backlog use global replication buffer,  if I/O threads are enabled,
        to guarantee data accessing thread safe, we must let main thread handle sending the output buffer
        to all replicas. But before, other IO threads could handle sending output buffer of all replicas.
      
      ## Other optimizations
      This solution resolve some other problem:
      - When replicas disconnect with master since of out of output buffer limit, releasing the output
        buffer of replicas may freeze server if we set big `client-output-buffer-limit` for replicas, but now,
        it doesn't cause freezing.
      - This implementation may mitigate reply list copy cost time(also freezes server) when one replication
        has huge reply buffer and another replica can copy buffer for full synchronization. now, we just copy
        reference info, it is very light.
      - If we set replication backlog size big, it also may cost much time to copy replication backlog into
        replica's output buffer. But this commit eliminates this problem.
      - Resizing replication backlog size doesn't empty current replication backlog content.
      c1718f9d
  10. 21 Oct, 2021 1 commit
  11. 20 Oct, 2021 2 commits
    • Oran Agra's avatar
      fix new cluster tests issues (#9657) · 7d6744c7
      Oran Agra authored
      Following #9483 the daily CI exposed a few problems.
      
      * The cluster creation code (uses redis-cli) is complicated to test with TLS enabled.
        for now i'm just skipping them since the tests we run there don't really need that kind of coverage
      * cluster port binding failures
        note that `find_available_port` already looks for a free cluster port
        but the code in `wait_server_started` couldn't detect the failure of binding
        (the text it greps for wasn't found in the log)
      7d6744c7
    • guybe7's avatar
      Treat subcommands as commands (#9504) · 43e736f7
      guybe7 authored
      ## Intro
      
      The purpose is to allow having different flags/ACL categories for
      subcommands (Example: CONFIG GET is ok-loading but CONFIG SET isn't)
      
      We create a small command table for every command that has subcommands
      and each subcommand has its own flags, etc. (same as a "regular" command)
      
      This commit also unites the Redis and the Sentinel command tables
      
      ## Affected commands
      
      CONFIG
      Used to have "admin ok-loading ok-stale no-script"
      Changes:
      1. Dropped "ok-loading" in all except GET (this doesn't change behavior since
      there were checks in the code doing that)
      
      XINFO
      Used to have "read-only random"
      Changes:
      1. Dropped "random" in all except CONSUMERS
      
      XGROUP
      Used to have "write use-memory"
      Changes:
      1. Dropped "use-memory" in all except CREATE and CREATECONSUMER
      
      COMMAND
      No changes.
      
      MEMORY
      Used to have "random read-only"
      Changes:
      1. Dropped "random" in PURGE and USAGE
      
      ACL
      Used to have "admin no-script ok-loading ok-stale"
      Changes:
      1. Dropped "admin" in WHOAMI, GENPASS, and CAT
      
      LATENCY
      No changes.
      
      MODULE
      No changes.
      
      SLOWLOG
      Used to have "admin random ok-loading ok-stale"
      Changes:
      1. Dropped "random" in RESET
      
      OBJECT
      Used to have "read-only random"
      Changes:
      1. Dropped "random" in ENCODING and REFCOUNT
      
      SCRIPT
      Used to have "may-replicate no-script"
      Changes:
      1. Dropped "may-replicate" in all except FLUSH and LOAD
      
      CLIENT
      Used to have "admin no-script random ok-loading ok-stale"
      Changes:
      1. Dropped "random" in all except INFO and LIST
      2. Dropped "admin" in ID, TRACKING, CACHING, GETREDIR, INFO, SETNAME, GETNAME, and REPLY
      
      STRALGO
      No changes.
      
      PUBSUB
      No changes.
      
      CLUSTER
      Changes:
      1. Dropped "admin in countkeysinslots, getkeysinslot, info, nodes, keyslot, myid, and slots
      
      SENTINEL
      No changes.
      
      (note that DEBUG also fits, but we decided not to convert it since it's for
      debugging and anyway undocumented)
      
      ## New sub-command
      This commit adds another element to the per-command output of COMMAND,
      describing the list of subcommands, if any (in the same structure as "regular" commands)
      Also, it adds a new subcommand:
      ```
      COMMAND LIST [FILTERBY (MODULE <module-name>|ACLCAT <cat>|PATTERN <pattern>)]
      ```
      which returns a set of all commands (unless filters), but excluding subcommands.
      
      ## Module API
      A new module API, RM_CreateSubcommand, was added, in order to allow
      module writer to define subcommands
      
      ## ACL changes:
      1. Now, that each subcommand is actually a command, each has its own ACL id.
      2. The old mechanism of allowed_subcommands is redundant
      (blocking/allowing a subcommand is the same as blocking/allowing a regular command),
      but we had to keep it, to support the widespread usage of allowed_subcommands
      to block commands with certain args, that aren't subcommands (e.g. "-select +select|0").
      3. I have renamed allowed_subcommands to allowed_firstargs to emphasize the difference.
      4. Because subcommands are commands in ACL too, you can now use "-" to block subcommands
      (e.g. "+client -client|kill"), which wasn't possible in the past.
      5. It is also possible to use the allowed_firstargs mechanism with subcommand.
      For example: `+config -config|set +config|set|loglevel` will block all CONFIG SET except
      for setting the log level.
      6. All of the ACL changes above required some amount of refactoring.
      
      ## Misc
      1. There are two approaches: Either each subcommand has its own function or all
         subcommands use the same function, determining what to do according to argv[0].
         For now, I took the former approaches only with CONFIG and COMMAND,
         while other commands use the latter approach (for smaller blamelog diff).
      2. Deleted memoryGetKeys: It is no longer needed because MEMORY USAGE now uses the "range" key spec.
      4. Bugfix: GETNAME was missing from CLIENT's help message.
      5. Sentinel and Redis now use the same table, with the same function pointer.
         Some commands have a different implementation in Sentinel, so we redirect
         them (these are ROLE, PUBLISH, and INFO).
      6. Command stats now show the stats per subcommand (e.g. instead of stats just
         for "config" you will have stats for "config|set", "config|get", etc.)
      7. It is now possible to use COMMAND directly on subcommands:
         COMMAND INFO CONFIG|GET (The pipeline syntax was inspired from ACL, and
         can be used in functions lookupCommandBySds and lookupCommandByCString)
      8. STRALGO is now a container command (has "help")
      
      ## Breaking changes:
      1. Command stats now show the stats per subcommand (see (5) above)
      43e736f7
  12. 19 Oct, 2021 2 commits
    • qetu3790's avatar
      Release clients blocked on module commands in cluster resharding and down state (#9483) · 4962c552
      qetu3790 authored
      
      
      Prevent clients from being blocked forever in cluster when they block with their own module command
      and the hash slot is migrated to another master at the same time.
      These will get a redirection message when unblocked.
      Also, release clients blocked on module commands when cluster is down (same as other blocked clients)
      
      This commit adds basic tests for the main (non-cluster) redis test infra that test the cluster.
      This was done because the cluster test infra can't handle some common test features,
      but most importantly we only build the test modules with the non-cluster test suite.
      
      note that rather than really supporting cluster operations by the test infra, it was added (as dup code)
      in two files, one for module tests and one for non-modules tests, maybe in the future we'll refactor that.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      4962c552
    • Wen Hui's avatar
      Make Cluster-bus port configurable with new cluster-port config (#9389) · 1c2b5f53
      Wen Hui authored
      Make Cluster-bus port configurable with new cluster-port config
      1c2b5f53
  13. 18 Oct, 2021 2 commits
  14. 08 Oct, 2021 1 commit
  15. 07 Oct, 2021 2 commits
    • yoav-steinberg's avatar
      obuf based eviction tests run until eviction occurs (#9611) · 834e8843
      yoav-steinberg authored
      obuf based eviction tests run until eviction occurs instead of assuming a certain
      amount of writes will fill the obuf enough for eviction to occur.
      This handles the kernel buffering written data and emptying the obuf even though
      no one actualy reads from it.
      
      The tests have a new timeout of 20sec: if the test doesn't pass after 20 sec it'll fail.
      Hopefully this enough for our slow CI targets.
      
      This also eliminates the need to skip some tests in TLS.
      834e8843
    • Huang Zhw's avatar
      Make tracking invalidation messages always after command's reply (#9422) · fd135f3e
      Huang Zhw authored
      Tracking invalidation messages were sometimes sent in inconsistent order,
      before the command's reply rather than after.
      In addition to that, they were sometimes embedded inside other commands
      responses, like MULTI-EXEC and MGET.
      fd135f3e
  16. 06 Oct, 2021 1 commit
  17. 05 Oct, 2021 2 commits
  18. 04 Oct, 2021 4 commits
    • Meir Shpilraien (Spielrein)'s avatar
      Fix invalid memory write on lua stack overflow (CVE-2021-32626) (#9591) · 0f8b634c
      Meir Shpilraien (Spielrein) authored
      When LUA call our C code, by default, the LUA stack has room for 10
      elements. In most cases, this is more than enough but sometimes it's not
      and the caller must verify the LUA stack size before he pushes elements.
      
      On 3 places in the code, there was no verification of the LUA stack size.
      On specific inputs this missing verification could have lead to invalid
      memory write:
      1. On 'luaReplyToRedisReply', one might return a nested reply that will
         explode the LUA stack.
      2. On 'redisProtocolToLuaType', the Redis reply might be deep enough
         to explode the LUA stack (notice that currently there is no such
         command in Redis that returns such a nested reply, but modules might
         do it)
      3. On 'ldbRedis', one might give a command with enough arguments to
         explode the LUA stack (all the arguments will be pushed to the LUA
         stack)
      
      This commit is solving all those 3 issues by calling 'lua_checkstack' and
      verify that there is enough room in the LUA stack to push elements. In
      case 'lua_checkstack' returns an error (there is not enough room in the
      LUA stack and it's not possible to increase the stack), we will do the
      following:
      1. On 'luaReplyToRedisReply', we will return an error to the user.
      2. On 'redisProtocolToLuaType' we will exit with panic (we assume this
         scenario is rare because it can only happen with a module).
      3. On 'ldbRedis', we return an error.
      0f8b634c
    • Oran Agra's avatar
      Fix protocol parsing on 'ldbReplParseCommand' (CVE-2021-32672) (#9590) · b0ca3be2
      Oran Agra authored
      
      
      The protocol parsing on 'ldbReplParseCommand' (LUA debugging)
      Assumed protocol correctness. This means that if the following
      is given:
      *1
      $100
      test
      The parser will try to read additional 94 unallocated bytes after
      the client buffer.
      This commit fixes this issue by validating that there are actually enough
      bytes to read. It also limits the amount of data that can be sent by
      the debugger client to 1M so the client will not be able to explode
      the memory.
      Co-authored-by: default avatarmeir@redislabs.com <meir@redislabs.com>
      b0ca3be2
    • Oran Agra's avatar
      Fix ziplist and listpack overflows and truncations (CVE-2021-32627, CVE-2021-32628) (#9589) · c5e6a620
      Oran Agra authored
      
      
      - fix possible heap corruption in ziplist and listpack resulting by trying to
        allocate more than the maximum size of 4GB.
      - prevent ziplist (hash and zset) from reaching size of above 1GB, will be
        converted to HT encoding, that's not a useful size.
      - prevent listpack (stream) from reaching size of above 1GB.
      - XADD will start a new listpack if the new record may cause the previous
        listpack to grow over 1GB.
      - XADD will respond with an error if a single stream record is over 1GB
      - List type (ziplist in quicklist) was truncating strings that were over 4GB,
        now it'll respond with an error.
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      c5e6a620
    • Oran Agra's avatar
      Prevent unauthenticated client from easily consuming lots of memory (CVE-2021-32675) (#9588) · fba15850
      Oran Agra authored
      This change sets a low limit for multibulk and bulk length in the
      protocol for unauthenticated connections, so that they can't easily
      cause redis to allocate massive amounts of memory by sending just a few
      characters on the network.
      The new limits are 10 arguments of 16kb each (instead of 1m of 512mb)
      fba15850
  19. 03 Oct, 2021 2 commits
  20. 30 Sep, 2021 1 commit
  21. 29 Sep, 2021 1 commit
  22. 26 Sep, 2021 1 commit
    • yoav-steinberg's avatar
      Client eviction ci issues (#9549) · 66002530
      yoav-steinberg authored
      Fixing CI test issues introduced in #8687
      - valgrind warnings in readQueryFromClient when client was freed by processInputBuffer
      - adding DEBUG pause-cron for tests not to be time dependent.
      - skipping a test that depends on socket buffers / events not compatible with TLS
      - making sure client got subscribed by not using deferring client
      66002530
  23. 23 Sep, 2021 3 commits
    • yoav-steinberg's avatar
      Client eviction (#8687) · 2753429c
      yoav-steinberg authored
      
      
      ### Description
      A mechanism for disconnecting clients when the sum of all connected clients is above a
      configured limit. This prevents eviction or OOM caused by accumulated used memory
      between all clients. It's a complimentary mechanism to the `client-output-buffer-limit`
      mechanism which takes into account not only a single client and not only output buffers
      but rather all memory used by all clients.
      
      #### Design
      The general design is as following:
      * We track memory usage of each client, taking into account all memory used by the
        client (query buffer, output buffer, parsed arguments, etc...). This is kept up to date
        after reading from the socket, after processing commands and after writing to the socket.
      * Based on the used memory we sort all clients into buckets. Each bucket contains all
        clients using up up to x2 memory of the clients in the bucket below it. For example up
        to 1m clients, up to 2m clients, up to 4m clients, ...
      * Before processing a command and before sleep we check if we're over the configured
        limit. If we are we start disconnecting clients from larger buckets downwards until we're
        under the limit.
      
      #### Config
      `maxmemory-clients` max memory all clients are allowed to consume, above this threshold
      we disconnect clients.
      This config can either be set to 0 (meaning no limit), a size in bytes (possibly with MB/GB
      suffix), or as a percentage of `maxmemory` by using the `%` suffix (e.g. setting it to `10%`
      would mean 10% of `maxmemory`).
      
      #### Important code changes
      * During the development I encountered yet more situations where our io-threads access
        global vars. And needed to fix them. I also had to handle keeps the clients sorted into the
        memory buckets (which are global) while their memory usage changes in the io-thread.
        To achieve this I decided to simplify how we check if we're in an io-thread and make it
        much more explicit. I removed the `CLIENT_PENDING_READ` flag used for checking
        if the client is in an io-thread (it wasn't used for anything else) and just used the global
        `io_threads_op` variable the same way to check during writes.
      * I optimized the cleanup of the client from the `clients_pending_read` list on client freeing.
        We now store a pointer in the `client` struct to this list so we don't need to search in it
        (`pending_read_list_node`).
      * Added `evicted_clients` stat to `INFO` command.
      * Added `CLIENT NO-EVICT ON|OFF` sub command to exclude a specific client from the
        client eviction mechanism. Added corrosponding 'e' flag in the client info string.
      * Added `multi-mem` field in the client info string to show how much memory is used up
        by buffered multi commands.
      * Client `tot-mem` now accounts for buffered multi-commands, pubsub patterns and
        channels (partially), tracking prefixes (partially).
      * CLIENT_CLOSE_ASAP flag is now handled in a new `beforeNextClient()` function so
        clients will be disconnected between processing different clients and not only before sleep.
        This new function can be used in the future for work we want to do outside the command
        processing loop but don't want to wait for all clients to be processed before we get to it.
        Specifically I wanted to handle output-buffer-limit related closing before we process client
        eviction in case the two race with each other.
      * Added a `DEBUG CLIENT-EVICTION` command to print out info about the client eviction
        buckets.
      * Each client now holds a pointer to the client eviction memory usage bucket it belongs to
        and listNode to itself in that bucket for quick removal.
      * Global `io_threads_op` variable now can contain a `IO_THREADS_OP_IDLE` value
        indicating no io-threading is currently being executed.
      * In order to track memory used by each clients in real-time we can't rely on updating
        these stats in `clientsCron()` alone anymore. So now I call `updateClientMemUsage()`
        (used to be `clientsCronTrackClientsMemUsage()`) after command processing, after
        writing data to pubsub clients, after writing the output buffer and after reading from the
        socket (and maybe other places too). The function is written to be fast.
      * Clients are evicted if needed (with appropriate log line) in `beforeSleep()` and before
        processing a command (before performing oom-checks and key-eviction).
      * All clients memory usage buckets are grouped as follows:
        * All clients using less than 64k.
        * 64K..128K
        * 128K..256K
        * ...
        * 2G..4G
        * All clients using 4g and up.
      * Added client-eviction.tcl with a bunch of tests for the new mechanism.
      * Extended maxmemory.tcl to test the interaction between maxmemory and
        maxmemory-clients settings.
      * Added an option to flag a numeric configuration variable as a "percent", this means that
        if we encounter a '%' after the number in the config file (or config set command) we
        consider it as valid. Such a number is store internally as a negative value. This way an
        integer value can be interpreted as either a percent (negative) or absolute value (positive).
        This is useful for example if some numeric configuration can optionally be set to a percentage
        of something else.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      2753429c
    • YaacovHazan's avatar
      Adding ACL support for modules (#9309) · a56d4533
      YaacovHazan authored
      This commit introduced a new flag to the RM_Call:
      'C' - Check if the command can be executed according to the ACLs associated with it.
      
      Also, three new API's added to check if a command, key, or channel can be executed or accessed
      by a user, according to the ACLs associated with it.
      - RM_ACLCheckCommandPerm
      - RM_ACLCheckKeyPerm
      - RM_ACLCheckChannelPerm
      
      The user for these API's is a RedisModuleUser object, that for a Module user returned by the RM_CreateModuleUser API, or for a general ACL user can be retrieved by these two new API's:
      - RM_GetCurrentUserName - Retrieve the user name of the client connection behind the current context.
      - RM_GetModuleUserFromUserName - Get a RedisModuleUser from a user name
      
      As a result of getting a RedisModuleUser from name, it can now also access the general ACL users (not just ones created by the module).
      This mean the already existing API RM_SetModuleUserACL(), can be used to change the ACL rules for such users.
      a56d4533
    • Binbin's avatar
      Add ZMPOP/BZMPOP commands. (#9484) · 14d6abd8
      Binbin authored
      This is similar to the recent addition of LMPOP/BLMPOP (#9373), but zset.
      
      Syntax for the new ZMPOP command:
      `ZMPOP numkeys [<key> ...] MIN|MAX [COUNT count]`
      
      Syntax for the new BZMPOP command:
      `BZMPOP timeout numkeys [<key> ...] MIN|MAX [COUNT count]`
      
      Some background:
      - ZPOPMIN/ZPOPMAX take only one key, and can return multiple elements.
      - BZPOPMIN/BZPOPMAX take multiple keys, but return only one element from just one key.
      - ZMPOP/BZMPOP can take multiple keys, and can return multiple elements from just one key.
      
      Note that ZMPOP/BZMPOP can take multiple keys, it eventually operates on just on key.
      And it will propagate as ZPOPMIN or ZPOPMAX with the COUNT option.
      
      As new commands, if we can not pop any elements, the response like:
      - ZMPOP: Return a NIL in both RESP2 and RESP3, unlike ZPOPMIN/ZPOPMAX return emptyarray.
      - BZMPOP: Return a NIL in both RESP2 and RESP3 when timeout is reached, like BZPOPMIN/BZPOPMAX.
      
      For the normal response is nested arrays in RESP2 and RESP3:
      ```
      ZMPOP/BZMPOP
      1) keyname
      2) 1) 1) member1
            2) score1
         2) 1) member2
            2) score2
      
      In RESP2:
      1) "myzset"
      2) 1) 1) "three"
            2) "3"
         2) 1) "two"
            2) "2"
      
      In RESP3:
      1) "myzset"
      2) 1) 1) "three"
            2) (double) 3
         2) 1) "two"
            2) (double) 2
      ```
      14d6abd8
  24. 22 Sep, 2021 1 commit
    • Oran Agra's avatar
      tune lazyfree test timeout (#9527) · 5f7789d3
      Oran Agra authored
      i've seen this CI failure a couple of times on MacOS:
      
      *** [err]: lazy free a stream with all types of metadata in tests/unit/lazyfree.tcl
      lazyfree isn't done
      
      only reason i can think of is that 500ms is sometimes not enough on slow systems.
      5f7789d3