1. 14 Nov, 2022 2 commits
  2. 13 Nov, 2022 1 commit
  3. 12 Nov, 2022 1 commit
  4. 10 Nov, 2022 1 commit
  5. 09 Nov, 2022 3 commits
    • Viktor Söderqvist's avatar
      Listpack encoding for sets (#11290) · 4e472a1a
      Viktor Söderqvist authored
      Small sets with not only integer elements are listpack encoded, by default
      up to 128 elements, max 64 bytes per element, new config `set-max-listpack-entries`
      and `set-max-listpack-value`. This saves memory for small sets compared to using a hashtable.
      
      Sets with only integers, even very small sets, are still intset encoded (up to 1G
      limit, etc.). Larger sets are hashtable encoded.
      
      This PR increments the RDB version, and has an effect on OBJECT ENCODING
      
      Possible conversions when elements are added:
      
          intset -> listpack
          listpack -> hashtable
          intset -> hashtable
      
      Note: No conversion happens when elements are deleted. If all elements are
      deleted and then added again, the set is deleted and recreated, thus implicitly
      converted to a smaller encoding.
      4e472a1a
    • Viktor Söderqvist's avatar
      Deprecate QUIT (#11439) · 07d18706
      Viktor Söderqvist authored
      Clients should not use this command.
      Instead, clients should simply close the connection when they're not used anymore.
      Terminating a connection on the client side is preferable, as it eliminates `TIME_WAIT`
      lingering sockets on the server side.
      07d18706
    • Oran Agra's avatar
      diskless master, avoid bgsave child hung when fork parent crashes (#11463) · ccaef5c9
      Oran Agra authored
      During a diskless sync, if the master main process crashes, the child would
      have hung in `write`. This fix closes the read fd on the child side, so that if the
      parent crashes, the child will get a write error and exit.
      
      This change also fixes disk-based replication, BGSAVE and AOFRW.
      In that case the child wouldn't have been hang, it would have just kept
      running until done which may be pointless.
      
      There is a certain degree of risk here. in case there's a BGSAVE child that could
      maybe succeed and the parent dies for some reason, the old code would have let
      the child keep running and maybe succeed and avoid data loss.
      On the other hand, if the parent is restarted, it would have loaded an old rdb file
      (or none), and then the child could reach the end and rename the rdb file (data
      conflicting with what the parent has), or also have a race with another BGSAVE
      child that the new parent started.
      
      Note that i removed a comment saying a write error will be ignored in the child
      and handled by the parent (this comment was very old and i don't think relevant).
      ccaef5c9
  6. 08 Nov, 2022 1 commit
  7. 07 Nov, 2022 1 commit
  8. 06 Nov, 2022 2 commits
  9. 04 Nov, 2022 2 commits
    • Binbin's avatar
      Bump vmactions/freebsd-vm to 0.3.0 to fix FreeBSD daily (#11476) · 5246bf45
      Binbin authored
      Our FreeBSD daily has been failing recently:
      ```
        Config file: freebsd-13.1.conf
        cd: /Users/runner/work/redis/redis: No such file or directory
        gmake: *** No targets specified and no makefile found.  Stop.
      ```
      
      Upgrade vmactions/freebsd-vm to the latest version (0.3.0) can work.
      I've tested it, but don't know why, but first let's fix it.
      5246bf45
    • Binbin's avatar
      Introduce socket shutdown into connection type, used if a fork is active (#11376) · fac188b4
      Binbin authored
      Introduce socket `shutdown()` into connection type, and use it
      on normal socket if a fork is active. This allows us to close
      client connections when there are child processes sharing the
      file descriptors.
      
      Fixes #10077. The reason is that since the `fork()` child is holding
      the file descriptors, the `close` in `unlinkClient -> connClose`
      isn't sufficient. The client will not realize that the connection is
      disconnected until the child process ends.
      
      Let's try to be conservative and only use shutdown when the fork is active.
      fac188b4
  10. 03 Nov, 2022 2 commits
    • Madelyn Olson's avatar
      Retain ACL categories used to generate ACL for displaying them later (#11224) · c337c0a8
      Madelyn Olson authored
      Retain ACL categories used to generate ACL for displaying them later
      c337c0a8
    • Binbin's avatar
      Block some specific characters in module command names (#11434) · 8764611c
      Binbin authored
      Today we don't place any specific restrictions on module command names.
      This can cause ambiguous scenarios. For example, someone might name a
      command like "module|feature" which would be incorrectly parsed by the
      ACL system as a subcommand.
      
      In this PR, we will block some chars that we know can mess things up.
      Specifically ones that can appear ok at first and cause problems in some
      cases (we rather surface the issue right away).
      
      There are these characters:
       * ` ` (space) - issues with old inline protocol.
       * `\r`, `\n` (newline) - can mess up the protocol on acl error replies.
       * `|` - sub-commands.
       * `@` - ACL categories
       * `=`, `,` - info and client list fields.
      
      note that we decided to leave `:` out as it's handled by `getSafeInfoString`
      and is more likely to already been used by existing modules.
      8764611c
  11. 02 Nov, 2022 4 commits
    • Wen Hui's avatar
      Fix XSETID with max_deleted_entry_id issue (#11444) · 7395e370
      Wen Hui authored
      Resolve an edge case where the ID of a stream is updated retroactively
      to an ID lower than the already set max_deleted_entry_id.
      
      Currently, if we have command as below:
      **xsetid mystream 1-1 MAXDELETEDID 1-2**
      Then we will get the following error:
      **(error) ERR The ID specified in XSETID is smaller than the provided max_deleted_entry_id**
      Becuase the provided MAXDELETEDID 1-2 is greated than input last-id: 1-1
      
      Then we could assume there is a similar situation:
      step 1: we add three items in the mystream
      
      **127.0.0.1:6381> xadd mystream 1-1 a 1
      "1-1"
      127.0.0.1:6381> xadd mystream 1-2 b 2
      "1-2"
      127.0.0.1:6381> xadd mystream 1-3 c 3
      "1-3"**
      
      step 2: we could check the mystream infomation as below:
      **127.0.0.1:6381> xinfo stream mystream
       1) "length"
       2) (integer) 3
       7) "last-generated-id"
       8) "1-3"
       9) "max-deleted-entry-id"
      10) "0-0"
      
      step 3: we delete the item id 1-2 and 1-3 as below:
      **127.0.0.1:6381> xdel mystream 1-2
      (integer) 1
      127.0.0.1:6381> xdel mystream 1-3
      (integer) 1**
      
      step 4: we check the mystream information:
      127.0.0.1:6381> xinfo stream mystream
       1) "length"
       2) (integer) 1
       7) "last-generated-id"
       8) "1-3"
       9) "max-deleted-entry-id"
      10) "1-3"
      
      we could notice that the **max-deleted-entry-id update to 1-3**, so right now, if we just run:
      **xsetid mystream 1-2** 
      the above command has the same effect with **xsetid mystream 1-2  MAXDELETEDID 1-3**
      
      So we should return an error to the client that **(error) ERR The ID specified in XSETID is smaller than current max_deleted_entry_id**
      7395e370
    • Wen Hui's avatar
      Fix command BITFIELD_RO and BITFIELD argument json file, add some test cases for them (#11445) · fea9bbbe
      Wen Hui authored
      According to the source code, the commands can be executed with only key name,
      and no GET/SET/INCR operation arguments.
      change the docs to reflect that by marking these arguments as optional.
      also add tests.
      fea9bbbe
    • Binbin's avatar
      Print IP and port on cluster bus message sanity check (#11443) · e632e62e
      Binbin authored
      
      
      * Print IP and port on cluster bus message sanity check
      
      Add a print statement to indicate which IP/port is sending
      the error messages. That way we can at least check to see
      if it is a node in the cluster or some other nefarious nodes.
      
      It is proposed in #11339.
      
      Unrelated changes: the return check for connAddrPeerName should
      be -1 instead of C_ERR, although the value of C_ERR is also -1.
      Co-authored-by: default avatarMadelyn Olson <34459052+madolson@users.noreply.github.com>
      e632e62e
    • Brennan's avatar
      Re-design cluster link send buffer to improve memory management (#11343) · 47c493e0
      Brennan authored
      Re-design cluster link send queue to improve memory management
      47c493e0
  12. 01 Nov, 2022 1 commit
  13. 29 Oct, 2022 1 commit
  14. 27 Oct, 2022 2 commits
    • Moti Cohen's avatar
      Refactor and (internally) rebrand from pause-clients to pause-actions (#11098) · c0d72262
      Moti Cohen authored
      Renamed from "Pause Clients" to "Pause Actions" since the mechanism can pause
      several actions in redis, not just clients (e.g. eviction, expiration).
      
      Previously each pause purpose (which has a timeout that's tracked separately from others purposes),
      also implicitly dictated what it pauses (reads, writes, eviction, etc). Now it is explicit, and
      the actions that are paused (bit flags) are defined separately from the purpose.
      
      - Previously, when using feature pause-client it also implicitly means to make the server static:
        - Pause replica traffic
        - Pauses eviction processing
        - Pauses expire processing
      
      Making the server static is used also for failover and shutdown. This PR internally rebrand
      pause-client API to become pause-action API. It also Simplifies pauseClients structure
      by replacing pointers array with static array.
      
      The context of this PR is to add another trigger to pause-client which will activated in case
      of OOM as throttling mechanism ([see here](https://github.com/redis/redis/issues/10907)).
      In this case we want only to pause client, and eviction actions.
      c0d72262
    • Shaya Potter's avatar
      RM_Call - only enforce OOM on scripts if 'M' flag is sent (#11425) · 38028dab
      Shaya Potter authored
      
      
      RM_Call is designed to let modules call redis commands disregarding the
      OOM state (the module is responsible to declare its command flags to redis,
      or perform the necessary checks).
      The other (new) alternative is to pass the "M" flag to RM_Call so that redis can
      OOM reject commands implicitly.
      
      However, Currently, RM_Call enforces OOM on scripts (excluding scripts that
      declared `allow-oom`) in all cases, regardless of the RM_Call "M" flag being present.
      
      This PR fixes scripts to be consistent with other commands being executed by RM_Call.
      It modifies the flow in effect treats scripts as if they if they have the ALLOW_OOM script
      flag, if the "M" flag is not passed (i.e. no OOM checking is being performed by RM_Call,
      so no OOM checking should be done on script).
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      38028dab
  15. 26 Oct, 2022 1 commit
  16. 25 Oct, 2022 2 commits
  17. 24 Oct, 2022 2 commits
    • guybe7's avatar
      Cleanup: Remove redundant arg from moduleCreateArgvFromUserFormat (#11426) · f8970fdb
      guybe7 authored
      We do not need to return the length of argv because it is equal to argc, which we return anyway.
      This change makes the code cleaner and adds a comment to explain something that might not be immediately clear.
      f8970fdb
    • guybe7's avatar
      Set errno in case XADD with partial ID fails (#11424) · 737a0905
      guybe7 authored
      This is a rare failure mode of a new feature of redis 7 introduced in #9217
      (when the incremental part of the ID overflows).
      Till now, the outcome of that error was undetermined (could easily result in
      `Elements are too large to be stored` wrongly, due to unset `errno`).
      737a0905
  18. 23 Oct, 2022 2 commits
    • Moti Cohen's avatar
      Fix sentinel function that compares hostnames (if failed resolve) (#11419) · bd23b15a
      Moti Cohen authored
      Funcion sentinelAddrEqualsHostname() of sentinel makes DNS resolve
      and based on it determines if two IP addresses are equal. Now, If the
      DNS resolve command fails, the function simply returns 0, even if the
      hostnames are identical.
      
      This might become an issue in case of failover such that sentinel might
      receives from Redis instance, response to regular INFO query it sent,
      and wrongly decide that the instance is pointing to is different leader
      than the one recorded because of this function, yet hostnames are
      identical. In turn sentinel disconnects the connection between sentinel
      and valid slave which leads to -failover-abort-no-good-slave.
      See issue #11241.
      
      I managed to reproduce only part of the flow in which the function
      return wrong result and trigger +fix-slave-config.
      
      The fix is even if the function failed to resolve then compare based on
      hostnames. That is our best effort as long as the server is unavailable
      for some reason. It is fine since Redis instance cannot have multiple
      hostnames for a given setup
      bd23b15a
    • qetu3790's avatar
      Fix wrong tips when the user pass wrong # of arguments to redis.set_repl(). (#11415) · 20df1424
      qetu3790 authored
      redis.set_repl() needs one arg, but the tips says two.
      20df1424
  19. 22 Oct, 2022 2 commits
    • Binbin's avatar
      Make PFMERGE source key optional in docs, add tests with one input key, add... · 9e1b879f
      Binbin authored
      Make PFMERGE source key optional in docs, add tests with one input key, add tests on missing source keys (#11205)
      
      The following two cases will create an empty destkey HLL:
      1. called with no source keys, like `pfmerge destkey`
      2. called with non-existing source keys, like `pfmerge destkey non-existing-source-key`
      
      In the first case, in `PFMERGE`, the dest key is actually one of the source keys too.
      So `PFMERGE k1 k2` is equivalent to `SUNIONSTORE k1 k1 k2`,
      and `PFMERGE k1` is equivalent to `SUNIONSTORE k1 k1`.
      So the first case is reasonable, the source key is actually optional.
      
      And the second case, `PFMERGE` on missing keys should succeed and create an empty dest.
      This is consistent with `PFCOUNT`, and also with `SUNIONSTORE`, no need to change.
      9e1b879f
    • sundb's avatar
      Fix crash due to to reuse iterator entry after list deletion in module (#11383) · 6dd21355
      sundb authored
      
      
      In the module, we will reuse the list iterator entry for RM_ListDelete, but `listTypeDelete` will only update
      `quicklistEntry->zi` but not `quicklistEntry->node`, which will result in `quicklistEntry->node` pointing to
      a freed memory address if the quicklist node is deleted. 
      
      This PR sync `key->u.list.index` and `key->u.list.entry` to list iterator after `RM_ListDelete`.
      
      This PR also optimizes the release code of the original list iterator.
      Co-authored-by: default avatarViktor Söderqvist <viktor@zuiderkwast.se>
      6dd21355
  20. 19 Oct, 2022 1 commit
  21. 18 Oct, 2022 5 commits
    • guybe7's avatar
      Blocked module clients should be aware when a key is deleted (#11310) · b57fd010
      guybe7 authored
      The use case is a module that wants to implement a blocking command on a key that
      necessarily exists and wants to unblock the client in case the key is deleted (much like
      what we implemented for XREADGROUP in #10306)
      
      New module API:
      * RedisModule_BlockClientOnKeysWithFlags
      
      Flags:
      * REDISMODULE_BLOCK_UNBLOCK_NONE
      * REDISMODULE_BLOCK_UNBLOCK_DELETED
      
      ### Detailed description of code changes
      
      blocked.c:
      1. Both module and stream functions are called whether the key exists or not, regardless of
        its type. We do that in order to allow modules/stream to unblock the client in case the key
        is no longer present or has changed type (the behavior for streams didn't change, just code
        that moved into serveClientsBlockedOnStreamKey)
      2. Make sure afterCommand is called in serveClientsBlockedOnKeyByModule, in order to propagate
        actions from moduleTryServeClientBlockedOnKey.
      3. handleClientsBlockedOnKeys: call propagatePendingCommands directly after lookupKeyReadWithFlags
        to prevent a possible lazy-expire DEL from being mixed with any command propagated by the
        preceding functions.
      4. blockForKeys: Caller can specifiy that it wants to be awakened if key is deleted.
         Minor optimizations (use dictAddRaw).
      5. signalKeyAsReady became signalKeyAsReadyLogic which can take a boolean in case the key is deleted.
        It will only signal if there's at least one client that awaits key deletion (to save calls to
        handleClientsBlockedOnKeys).
        Minor optimizations (use dictAddRaw)
      
      db.c:
      1. scanDatabaseForDeletedStreams is now scanDatabaseForDeletedKeys and will signalKeyAsReady
        for any key that was removed from the database or changed type. It is the responsibility of the code
        in blocked.c to ignore or act on deleted/type-changed keys.
      2. Use the new signalDeletedKeyAsReady where needed
      
      blockedonkey.c + tcl:
      1. Added test of new capabilities (FSL.BPOPGT now requires the key to exist in order to work)
      b57fd010
    • Meir Shpilraien (Spielrein)'s avatar
      Avoid saving module aux on RDB if no aux data was saved by the module. (#11374) · b43f2548
      Meir Shpilraien (Spielrein) authored
      ### Background
      
      The issue is that when saving an RDB with module AUX data, the module AUX metadata
      (moduleid, when, ...) is saved to the RDB even though the module did not saved any actual data.
      This prevent loading the RDB in the absence of the module (although there is no actual data in
      the RDB that requires the module to be loaded).
      
      ### Solution
      
      The solution suggested in this PR is that module AUX will be saved on the RDB only if the module
      actually saved something during `aux_save` function.
      
      To support backward compatibility, we introduce `aux_save2` callback that acts the same as
      `aux_save` with the tiny change of avoid saving the aux field if no data was actually saved by
      the module. Modules can use the new API to make sure that if they have no data to save,
      then it will be possible to load the created RDB even without the module.
      
      ### Concerns
      
      A module may register for the aux load and save hooks just in order to be notified when
      saving or loading starts or completed (there are better ways to do that, but it still possible
      that someone used it).
      
      However, if a module didn't save a single field in the save callback, it means it's not allowed
      to read in the read callback, since it has no way to distinguish between empty and non-empty
      payloads. furthermore, it means that if the module did that, it must never change it, since it'll
      break compatibility with it's old RDB files, so this is really not a valid use case.
      
      Since some modules (ones who currently save one field indicating an empty payload), need
      to know if saving an empty payload is valid, and if Redis is gonna ignore an empty payload
      or store it, we opted to add a new API (rather than change behavior of an existing API and
      expect modules to check the redis version)
      
      ### Technical Details
      
      To avoid saving AUX data on RDB, we change the code to first save the AUX metadata
      (moduleid, when, ...) into a temporary buffer. The buffer is then flushed to the rio at the first
      time the module makes a write operation inside the `aux_save` function. If the module saves
      nothing (and `aux_save2` was used), the entire temporary buffer is simply dropped and no
      data about this AUX field is saved to the RDB. This make it possible to load the RDB even in
      the absence of the module.
      
      Test was added to verify the fix.
      b43f2548
    • Shuning's avatar
      keyIsExpired checks server.loading before calling getExpire (#11393) · 20d286f7
      Shuning authored
      
      
      Seems excessive to call getExpire if we don't need it.
      This can maybe have some speedup on AOF file loading (saving a dictFind call)
      Co-authored-by: default avatarlvshuning <lvshuning@meituan.com>
      20d286f7
    • DarrenJiang13's avatar
      fix malloc macro in listpack.c (#11398) · ba1f09d3
      DarrenJiang13 authored
      fix some malloc macros in `listpack.c`.
      listpack has it's own malloc aliases, but in some places normal redis malloc calls have slipped in.
      ba1f09d3
    • Binbin's avatar
      Bump codespell from 2.2.1 to 2.2.2 in /.codespell (#11399) · a9d561af
      Binbin authored
      And fix a few newly detected typo.
      Closes #11394
      a9d561af
  22. 16 Oct, 2022 1 commit
    • Shaya Potter's avatar
      Unify ACL failure error messaging. (#11160) · 3193f086
      Shaya Potter authored
      Motivation: for applications that use RM ACL verification functions, they would
      want to return errors back to the user, in ways that are consistent with Redis.
      While investigating how we should return ACL errors to the user, we realized that
      Redis isn't consistent, and currently returns ACL error strings in 3 primary ways.
      
      [For the actual implications of this change, see the "Impact" section at the bottom]
      
      1. how it returns an error when calling a command normally
         ACL_DENIED_CMD -> "this user has no permissions to run the '%s' command"
         ACL_DENIED_KEY -> "this user has no permissions to access one of the keys used as arguments"
         ACL_DENIED_CHANNEL -> "this user has no permissions to access one of the channels used as arguments"
      
      2. how it returns an error when calling via 'acl dryrun' command
         ACL_DENIED_CMD ->  "This user has no permissions to run the '%s' command"
         ACL_DENIED_KEY -> "This user has no permissions to access the '%s' key"
         ACL_DENIED_CHANNEL -> "This user has no permissions to access the '%s' channel"
      
      3. how it returns an error via RM_Call (and scripting is similar).
         ACL_DENIED_CMD -> "can't run this command or subcommand";
         ACL_DENIED_KEY -> "can't access at least one of the keys mentioned in the command arguments";
         ACL_DENIED_CHANNEL -> "can't publish to the channel mentioned in the command";
         
         In addition, if one wants to use RM_Call's "dry run" capability instead of the RM ACL
         functions directly, one also sees a different problem than it returns ACL errors with a -ERR,
         not a -PERM, so it can't be returned directly to the caller.
      
      This PR modifies the code to generate a base message in a common manner with the ability
      to set verbose flag for acl dry run errors, and keep it unset for normal/rm_call/script cases
      
      ```c
      sds getAclErrorMessage(int acl_res, user *user, struct redisCommand *cmd, sds errored_val, int verbose) {
          switch (acl_res) {
          case ACL_DENIED_CMD:
              return sdscatfmt(sdsempty(), "User %S has no permissions to run "
                                           "the '%S' command", user->name, cmd->fullname);
          case ACL_DENIED_KEY:
              if (verbose) {
                  return sdscatfmt(sdsempty(), "User %S has no permissions to access "
                                               "the '%S' key", user->name, errored_val);
              } else {
                  return sdsnew("No permissions to access a key");
              }
          case ACL_DENIED_CHANNEL:
              if (verbose) {
                  return sdscatfmt(sdsempty(), "User %S has no permissions to access "
                                               "the '%S' channel", user->name, errored_val);
              } else {
                  return sdsnew("No permissions to access a channel");
              }
          }
      ```
      
      The caller can append/prepend the message (adding NOPERM for normal/RM_Call or indicating it's within a script).
      
      Impact:
      - Plain commands, as well as scripts and RM_Call now include the user name.
      - ACL DRYRUN remains the only one that's verbose (mentions the offending channel or key name)
      - Changes RM_Call ACL errors from being a `-ERR` to being `-NOPERM` (besides for textual changes)
        **This somewhat a breaking change, but it only affects the RM_Call with both `C` and `E`, or `D`**
      - Changes ACL errors in scripts textually from being
        `The user executing the script <old non unified text>`
        to
        `ACL failure in script: <new unified text>`
      3193f086