1. 30 Jan, 2022 9 commits
    • Oran Agra's avatar
      Revent the attempt to fix cluster rebalance test (#10207) (#10212) · d364ede5
      Oran Agra authored
      It seems that fix didn't really solve the problem with ASAN,
      and also introduced issues with other CI runs.
      
      unrelated:
      - make runtest-cluster able to take multiple --single arguments
      d364ede5
    • Oran Agra's avatar
      update help.h before release (#10210) · ef931259
      Oran Agra authored
      ef931259
    • Ping Xie's avatar
      Fix cluster bus extensions backwards compatibility (#10206) · 8013af6f
      Ping Xie authored
      Before this commit, notused1 was incorrectly resized resulting with a clusterMsg that is not backwards compatible as expected.
      8013af6f
    • Moti Cohen's avatar
      Improve srand entropy (and fix Sentinel failures) (#10197) · 52b2fbe9
      Moti Cohen authored
      As Sentinel relies upon consensus algorithm, all sentinel instances,
      randomize a time to initiate their next attempt to become the
      leader of the group. But time after time, all raffled the same value.
      
      The problem is in the line `srand(time(NULL)^getpid())` such that
      all spinned up containers get same time (in seconds) and same pid
      which is always 1. Added material `tv_usec` and verify that even
      consecutive calls brings different values and makes the difference.
      52b2fbe9
    • Tobias Nießen's avatar
    • Harkrishn Patro's avatar
      Set default channel permission to resetchannels for 7.0 (#10181) · a43b6922
      Harkrishn Patro authored
      For backwards compatibility in 6.x, channels default permission was set to `allchannels` however with 7.0,
      we should modify it and the default value should be `resetchannels` for better security posture.
      Also, with selectors in ACL, a client doesn't have to set channel rules everytime and by default
      the value will be `resetchannels`.
      
      Before this change
      ```
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      127.0.0.1:6379>  acl setuser hp on nopass +@all ~*
      OK
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      2) "user hp on nopass ~* &* +@all"
      127.0.0.1:6379>  acl setuser hp1 on nopass -@all (%R~sales*)
      OK
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      2) "user hp on nopass ~* &* +@all"
      3) "user hp1 on nopass &* -@all (%R~sales* &* -@all)"
      ```
      
      After this change
      ```
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      127.0.0.1:6379> acl setuser hp on nopass +@all ~*
      OK
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      2) "user hp on nopass ~* resetchannels +@all"
      127.0.0.1:6379> acl setuser hp1 on nopass -@all (%R~sales*)
      OK
      127.0.0.1:6379> acl list
      1) "user default on nopass ~* &* +@all"
      2) "user hp on nopass ~* resetchannels +@all"
      3) "user hp1 on nopass resetchannels -@all (%R~sales* resetchannels -@all)"
      ```
      a43b6922
    • guybe7's avatar
      Add key-specs notes (#10193) · eedec155
      guybe7 authored
      Add optional `notes` to keyspecs.
      
      Other changes:
      
      1. Remove the "incomplete" flag from SORT and SORT_RO: it is misleading since "incomplete" means "this spec may not return all the keys it describes" but SORT and SORT_RO's specs (except the input key) do not return any keys at all.
      So basically:
      If a spec's begin_search is "unknown" you should not use it at all, you must use COMMAND KEYS;
      if a spec itself is "incomplete", you can use it to get a partial list of keys, but if you want all of them you must use COMMAND GETKEYS;
      otherwise, the spec will return all the keys
      
      2. `getKeysUsingKeySpecs` handles incomplete specs internally
      eedec155
    • Oran Agra's avatar
      fix cluster rebalance test race (#10207) · be0d2933
      Oran Agra authored
      Try to fix the rebalance cluster test that's failing with ASAN daily:
      
      Looks like `redis-cli --cluster rebalance` gets `ERR Please use SETSLOT only with masters` in `clusterManagerMoveSlot()`.
      it happens when `12-replica-migration-2.tcl` is run with ASAN in GH Actions.
      in `Resharding all the master #0 slots away from it`
      
      So the fix (assuming i got it right) is to call `redis-cli --cluster check` before `--cluster rebalance`.
      p.s. it looks like a few other checks in these tests needed that wait, added them too.
      
      Other changes:
      * in instances.tcl, make sure to catch tcl test crashes and let the rest of the code proceed, so that if there was
        a redis crash, we'll find it and print it too.
      * redis-cli, try to make sure it prints an error instead of silently exiting.
      
      specifically about redis-cli:
      1. clusterManagerMoveSlot used to print an error, only if the caller also asked for it (should be the other way around).
      2. clusterManagerCommandReshard asked for an error, but didn't use it (probably tried to avoid the double print).
      3. clusterManagerCommandRebalance didn't ask for the error, now it does.
      4. making sure that other places in clusterManagerCommandRebalance print something before exiting with an error.
      be0d2933
    • Binbin's avatar
      Fix some wrong commands arguments since (#10208) · 21135471
      Binbin authored
      ZADD NX and XX was introduced in 3.0.2, not 6.2.0
      ZADD GT and LT was introduced in 6.2.0, not 3.0.2
      
      Add missing `COUNT ANY` history in georadius_ro
      Add missing `SHUTDOWN [NOW] [FORCE] [ABORT]` since in shutdown.json
      21135471
  2. 29 Jan, 2022 5 commits
    • Moti Cohen's avatar
      Fixed Sentinel support for hostnames (#10146) · 79f089bd
      Moti Cohen authored
      Sentinel tries to resolve instances hostname to IP only during registration. 
      It might be that the instance is unavailable during that time, such as 
      leader crashed and failover took place. Yet, promoted replica must support:
      
       - Register leader, even if it fails to resolve its hostname during failover
       - Try later to resolve it, if instance is disconnected. Note that
         this condition also support ip-change of an instance.
      79f089bd
    • Ozan Tezcan's avatar
      Skip external server and codeql cron tests on forks (#10204) · 2d391e62
      Ozan Tezcan authored
      keep the push triggers for all repos, but run the scheduled ones only on redis/redis
      2d391e62
    • Oran Agra's avatar
      doc improvement about acl first-arg (#10199) · 75a950cb
      Oran Agra authored
      We recently removed capabilities from the first-arg feature of ACL and added a warning.
      but we didn't document it.
      ref: #10147 and redis/redis-doc#1761
      75a950cb
    • Binbin's avatar
      commands arguments improvement about unix-time type (#10203) · 93d95155
      Binbin authored
      Change the name to unix-time-seconds or unix-time-milliseconds
      to be consistent. Change the type from INTEGER to UNIX_TIME.
      
      SET (EXAT and PXAT) was already ok.
      and naming aside, both PXAT and EXAT everywhere used unit-time (for both milliseconds and seconds).
      the only ones that where wrong are GETEX and XCLAIM (using "integer" for both seconds and milliseconds)
      93d95155
    • weiguo's avatar
      Use object size then pointer size when malloc (#10201) (#10202) · 76784e4e
      weiguo authored
      By a happy coincidence, sizeof(sds *) is equal to sizeof(sds) here,
      while it's logically consistent to use sizeof(sds) instead.
      76784e4e
  3. 28 Jan, 2022 1 commit
  4. 27 Jan, 2022 1 commit
  5. 26 Jan, 2022 6 commits
    • Binbin's avatar
      Allow SET without GET arg on write-only ACL. Allow BITFIELD GET on read-only ACL (#10148) · d6169258
      Binbin authored
      
      
      SET is a R+W command, because it can also do `GET` on the data.
      SET without GET is a write-only command.
      SET with GET is a read+write command.
      
      In #9974, we added ACL to let users define write-only access.
      So when the user uses SET with GET option, and the user doesn't
      have the READ permission on the key, we need to reject it,
      but we rather not reject users with write-only permissions from using
      the SET command when they don't use GET.
      
      In this commit, we add a `getkeys_proc` function to control key
      flags in SET command. We also add a new key spec flag (VARIABLE_FLAGS)
      means that some keys might have different flags depending on arguments.
      
      We also handle BITFIELD command, add a `bitfieldGetKeys` function.
      BITFIELD GET is a READ ONLY command.
      BITFIELD SET or BITFIELD INCR are READ WRITE commands.
      
      Other changes:
      1. SET GET was added in 6.2, add the missing since in set.json
      2. Added tests to cover the changes in acl-v2.tcl
      3. Fix some typos in server.h and cleanups in acl-v2.tcl
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      d6169258
    • Oran Agra's avatar
      Solve race in a BGSAVE test (#10190) · 795ea011
      Oran Agra authored
      This PR attempts to solve two problems that happen sometime in valgrind:
      `ERR Background save already in progress`
      and
      `not bgsave not aborted`
      
      the test used to populate the database with DEBUG, which didn't
      increment the dirty counter, so couldn't trigger an automatic bgsave.
      then it used a manual bgsave, and aborted it (when it got aborted it
      populated the dirty counter), and then it tried to do another bgsave.
      that other bgsave could have failed if the automatic one already
      started.
      795ea011
    • Oran Agra's avatar
      solve race in expiration test (#10192) · da48a812
      Oran Agra authored
      Failed on a non-valgrind run. on this line:
      ```
      assert_equal 0 [$slave exists k]
      ```
      the condition in `keyIsExpired` is `now > when`.
      so if the test is really fast, maybe it can get to EXISTS exactly 1000 milliseconds after the
      expiration was set, and the key isn't yet gone)
      da48a812
    • guybe7's avatar
      c79389f0
    • Binbin's avatar
      Fix unused variable warning in subcommand.c (#10184) · 7fdcada6
      Binbin authored
      Forgot to handle it in #10135.
      7fdcada6
    • Madelyn Olson's avatar
      Change expression to look for at least one limit exceeded (#10173) · f6b76e50
      Madelyn Olson authored
      This is an attempt to fix some of the issues with the cluster mode tests we are seeing in the daily run.
      
      The test is trying to incrementally adds a bunch of publish messages, expecting that eventually one
      of them will overflow. The tests stops one of the processes, so it expects that just that one Redis node
      will overflow. I think the test is flaky because under certain circumstances multiple links are getting
      disconnected, not just the one that is stalled.
      f6b76e50
  6. 25 Jan, 2022 6 commits
    • Viktor Söderqvist's avatar
      redis-cli: Aligned RESP3 maps with multiline value in TTY (#10170) · 4491ee18
      Viktor Söderqvist authored
      Before:
      
      ```
      127.0.0.1:6379> command info get
      1)  1) "get"
          2) (integer) 2
          3) 1~ readonly
             2~ fast
          4) (integer) 1
          5) (integer) 1
          6) (integer) 1
          7) 1~ @read
             2~ @string
             3~ @fast
          8) (empty set)
          9) 1~ 1# "flags" => 1~ RO
                   2~ access
                2# "begin_search" => 1# "type" => "index"
                   2# "spec" => 1# "index" => (integer) 1
                3# "find_keys" => 1# "type" => "range"
                   2# "spec" => 1# "lastkey" => (integer) 0
                      2# "keystep" => (integer) 1
                      3# "limit" => (integer) 0
         10) (empty set)
      ```
      
      After:
      
      ```
      127.0.0.1:6379> command info get
      1)  1) "get"
          2) (integer) 2
          3) 1~ readonly
             2~ fast
          4) (integer) 1
          5) (integer) 1
          6) (integer) 1
          7) 1~ @read
             2~ @string
             3~ @fast
          8) (empty set)
          9) 1~ 1# "flags" =>
                   1~ RO
                   2~ access
                2# "begin_search" =>
                   1# "type" => "index"
                   2# "spec" => 1# "index" => (integer) 1
                3# "find_keys" =>
                   1# "type" => "range"
                   2# "spec" =>
                      1# "lastkey" => (integer) 0
                      2# "keystep" => (integer) 1
                      3# "limit" => (integer) 0
         10) (empty set)
      ```
      4491ee18
    • Meir Shpilraien (Spielrein)'s avatar
      Added engine stats to FUNCTION STATS command. (#10179) · 5a38ccc2
      Meir Shpilraien (Spielrein) authored
      Added the following statistics (per engine) to FUNCTION STATS command:
      * number of functions
      * number of libraries
      
      Output example:
      ```
      > FUNCTION stats
      1) "running_script"
      2) (nil)
      3) "engines"
      4) 1) "LUA"
         2) 1) "libraries_count"
            2) (integer) 1
            3) "functions_count"
            4) (integer) 1
      ```
      
      To collect the stats, added a new dictionary to libraries_ctx that contains
      for each engine, the engine statistics representing the current libraries_ctx.
      Update the stats on:
      1. Link library to libraries_ctx
      2. Unlink library from libraries_ctx
      3. Flushing libraries_ctx
      5a38ccc2
    • yoav-steinberg's avatar
      Fix commands.c build issue on merge (#10172) · d26453a3
      yoav-steinberg authored
      Set commands.c's merge driver to binary so when it conflicts during a merge git will leave the local version unmodified.
      This way our Makefile will rebuild it based on src/commands/*.json before trying to compile it.
      Otherwise the file gets modified with merge conflict markers and gets the same timestamp as the *.json files,
      so the Makefile doesn't attempt to rebuild it before compiling and we get a compilation error.
      d26453a3
    • chenyang8094's avatar
    • Madelyn Olson's avatar
      Improve testing and update flags around commands without ACL keyspec flags (#10167) · 823da543
      Madelyn Olson authored
      This PR aims to improve the flags associated with some commands and adds various tests around
      these cases. Specifically, it's concerned with commands which declare keys but have no ACL
      flags (think `EXISTS`), the user needs either read or write permission to access this type of key.
      
      This change is primarily concerned around commands in three categories:
      
      # General keyspace commands
      These commands are agnostic to the underlying data outside of side channel attacks, so they are not
      marked as ACCESS.
      * TOUCH
      * EXISTS
      * TYPE
      * OBJECT 'all subcommands'
      
      Note that TOUCH is not a write command, it could be a side effect of either a read or a write command.
      
      # Length and cardinality commands
      These commands are marked as NOT marked as ACCESS since they don't return actual user strings,
      just metadata.
      * LLEN
      * STRLEN
      * SCARD
      * HSTRLEN
      
      # Container has member commands
      These commands return information about the existence or metadata about the key. These commands
      are NOT marked as ACCESS since the check of membership is used widely in write commands
      e.g. the response of HSET. 
      * SISMEMBER
      * HEXISTS
      
      # Intersection cardinality commands
      These commands are marked as ACCESS since they process data to compute the result.
      * PFCOUNT
      * ZCOUNT
      * ZINTERCARD
      * SINTERCARD
      823da543
    • Madelyn Olson's avatar
      Correctly handle minimum arity checks in scripts (#10171) · c275010f
      Madelyn Olson authored
      Correctly handle variable arity checks in scripts
      c275010f
  7. 24 Jan, 2022 7 commits
    • Oran Agra's avatar
      re-fix EVAL timeout test (#10169) · 0343fe9f
      Oran Agra authored
      The edit i made to #10098 before merging was broken.
      (INFO is forbidden in LOADING)
      0343fe9f
    • chenyang8094's avatar
      Fix EVAL timeout test failed on freebsd (#10098) · fa600496
      chenyang8094 authored
      
      
      * Refactor EVAL timeout test
      * since the test used r config set appendonly yes which generates a rewrite, it missed it's purpose
      * Fix the bug that start_server returns before redis starts ready, which affects when multiple tests share the same dir.
      * Elapsed time tracking no loner needed
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      fa600496
    • Binbin's avatar
      Fix outdated save key word in redis.conf (#10166) · 495ac8b7
      Binbin authored
      For some complex data types, server.dirty actually counts
      the number of elements that have been changed.
      And in FLUSHDB or FLUSHALL, we count the number of keys.
      
      So the word "key" is not strictly correct and is outdated.
      Some discussion can be seen at #8140.
      495ac8b7
    • yoav-steinberg's avatar
      Support function flags in script EVAL via shebang header (#10126) · 7eadc5ee
      yoav-steinberg authored
      In #10025 we added a mechanism for flagging certain properties for Redis Functions.
      This lead us to think we'd like to "port" this mechanism to Redis Scripts (`EVAL`) as well. 
      
      One good reason for this, other than the added functionality is because it addresses the
      poor behavior we currently have in `EVAL` in case the script performs a (non DENY_OOM) write operation
      during OOM state. See #8478 (And a previous attempt to handle it via #10093) for details.
      Note that in Redis Functions **all** write operations (including DEL) will return an error during OOM state
      unless the function is flagged as `allow-oom` in which case no OOM checking is performed at all.
      
      This PR:
      - Enables setting `EVAL` (and `SCRIPT LOAD`) script flags as defined in #10025.
      - Provides a syntactical framework via [shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) for
        additional script annotations and even engine selection (instead of just lua) for scripts.
      - Provides backwards compatibility so scripts without the new annotations will behave as they did before.
      - Appropriate tests.
      - Changes `EVAL[SHA]/_RO` to be flagged as `STALE` commands. This makes it possible to flag individual
        scripts as `allow-stale` or not flag them as such. In backwards compatibility mode these commands will
        return the `MASTERDOWN` error as before.
      - Changes `SCRIPT LOAD` to be flagged as a `STALE` command. This is mainly to make it logically
        compatible with the change to `EVAL` in the previous point. It enables loading a script on a stale server
        which is technically okay it doesn't relate directly to the server's dataset. Running the script does, but that
        won't work unless the script is explicitly marked as `allow-stale`.
      
      Note that even though the LUA syntax doesn't support hash tag comments `.lua` files do support a shebang
      tag on the top so they can be executed on Unix systems like any shell script. LUA's `luaL_loadfile` handles
      this as part of the LUA library. In the case of `luaL_loadbuffer`, which is what Redis uses, I needed to fix the
      input script in case of a shebang manually. I did this the same way `luaL_loadfile` does, by replacing the
      first line with a single line feed character.
      7eadc5ee
    • Viktor Söderqvist's avatar
      Disable keyspec module API in 7.0 RC1 (#10135) · 857dc5ba
      Viktor Söderqvist authored
      
      
      The keyspec API is not yet released and there is a plan to change it
      in #10108, which is going to be included in RC2. Therefore, we hide
      it in RC1 to avoid introducing a breaking change in RC2.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      857dc5ba
    • chenyang8094's avatar
    • ny0312's avatar
      Fix flaky cluster tests in 24-links.tcl (#10157) · b40a9ba5
      ny0312 authored
      
      
      * Fix flaky cluster test "Disconnect link when send buffer limit reached"
      * Fix flaky cluster test "Each node has two links with each peer"
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      b40a9ba5
  8. 23 Jan, 2022 2 commits
    • Binbin's avatar
      Fix timing issue in sentinel CKQUORUM test (#10036) · 7e5ded2a
      Binbin authored
      A test failure was reported in Daily CI (test-centos7-tls).
      `CKQUORUM detects failover authorization cannot be reached`.
      
      ```
      CKQUORUM detects failover authorization cannot be reached: FAILED:
      Expected 'invalid command name "OK 4 usable Sentinels. Quorum and failover authorization can be reached"' to match '*NOQUORUM*'
      ```
      
      It seems that current sentinel does not confirm that the other
      sentinels are actually `down`, and then check the quorum.
      It at least take 3 seconds on my machine, and we can see there
      will be a timing issue with the hard code `after 5000`.
      
      In this commit, we check the response of `SENTINEL SENTINELS mymaster`
      to ensure that other sentinels are actually `down` in the view the
      current sentinel. Solve the timing issue due to sentinel monitor mechanism.
      7e5ded2a
    • Binbin's avatar
      sub-command support for ACL CAT and COMMAND LIST. redisCommand always stores fullname (#10127) · 23325c13
      Binbin authored
      
      
      Summary of changes:
      1. Rename `redisCommand->name` to `redisCommand->declared_name`, it is a
        const char * for native commands and SDS for module commands.
      2. Store the [sub]command fullname in `redisCommand->fullname` (sds).
      3. List subcommands in `ACL CAT`
      4. List subcommands in `COMMAND LIST`
      5. `moduleUnregisterCommands` now will also free the module subcommands.
      6. RM_GetCurrentCommandName returns full command name
      
      Other changes:
      1. Add `addReplyErrorArity` and `addReplyErrorExpireTime`
      2. Remove `getFullCommandName` function that now is useless.
      3. Some cleanups about `fullname` since now it is SDS.
      4. Delete `populateSingleCommand` function from server.h that is useless.
      5. Added tests to cover this change.
      6. Add some module unload tests and fix the leaks
      7. Make error messages uniform, make sure they always contain the full command
        name and that it's quoted.
      7. Fixes some typos
      
      see the history in #9504, fixes #10124
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarguybe7 <guy.benoish@redislabs.com>
      23325c13
  9. 22 Jan, 2022 1 commit
    • guybe7's avatar
      Improved handling of subcommands (don't allow ACL on first-arg of a sub-command) (#10147) · a6fd2a46
      guybe7 authored
      Recently we added extensive support for sub-commands in for redis 7.0,
      this meant that the old ACL mechanism for
      sub-commands wasn't needed, or actually was improved (to handle both include
      and exclude control, like for commands), but only for real sub-commands.
      The old mechanism in ACL was renamed to first-arg, and was able to match the
      first argument of any command (including sub-commands).
      We now realized that we might wanna completely delete that first-arg feature some
      day, so the first step was not to give it new capabilities in 7.0 and it didn't have before.
      
      Changes:
      1. ACL: Block the first-arg mechanism on subcommands (we keep if in non-subcommands
        for backward compatibility)
      2. COMMAND: When looking up a command, insist the command name doesn't contain
        extra words. Example: When a user issues `GET key` we want `lookupCommand` to return
        `getCommand` but when if COMMAND calls `lookupCommand` with `get|key` we want it to fail.
      
      Other changes:
      1. ACLSetUser: prevent a redundant command lookup
      a6fd2a46
  10. 20 Jan, 2022 2 commits
    • Madelyn Olson's avatar
      ACL V2 - Selectors and key based permissions (#9974) · 55c81f2c
      Madelyn Olson authored
      
      
      * Implemented selectors which provide multiple different sets of permissions to users
      * Implemented key based permissions 
      * Added a new ACL dry-run command to test permissions before execution
      * Updated module APIs to support checking key based permissions
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      55c81f2c
    • guybe7's avatar
      Add command tips to COMMAND DOCS (#10104) · 10bbeb68
      guybe7 authored
      Adding command tips (see https://redis.io/topics/command-tips) to commands.
      
      Breaking changes:
      1. Removed the "random" and "sort_for_script" flags. They are now command tips.
      (this isn't affecting redis behavior since #9812, but could affect some client applications
      that's relying on COMMAND command flags)
      
      Summary of changes:
      1. add BLOCKING flag (new flag) for all commands that could block. The ACL category with
        the same name is now implicit.
      2. move RANDOM flag to a `nondeterministic_output` tip
      3. move SORT_FOR_SCRIPT flag to `nondeterministic_output_order` tip
      3. add REQUEST_POLICY and RESPONSE_POLICY where appropriate as documented in the tips
      4. deprecate (ignore) the `random` flag for RM_CreateCommand
      
      Other notes:
      1. Proxies need to send `RANDOMKEY` to all shards and then select one key randomly.
        The other option is to pick a random shard and transfer `RANDOMKEY `to it, but that scheme
        fails if this specific shard is empty
      2. Remove CMD_RANDOM from `XACK` (i.e. XACK does not have RANDOM_OUTPUT)
         It was added in 9e4fb96c, I guess by mistake.
         Also from `(P)EXPIRETIME` (new command, was flagged "random" by mistake).
      3. Add `nondeterministic_output` to `OBJECT ENCODING` (for the same reason `XTRIM` has it:
         the reply may differ depending on the internal representation in memory)
      4. RANDOM on `HGETALL` was wrong (there due to a limitation of the old script sorting logic), now
        it's `nondeterministic_output_order`
      5. Unrelated: Hide CMD_PROTECTED from COMMAND
      10bbeb68