1. 30 Mar, 2021 1 commit
    • Oran Agra's avatar
      solve race conditions in psync2-pingoff test (#8720) · cd81dcf1
      Oran Agra authored
      Another test race condition in the macos tests.
      the test was waiting for PINGs to be generated and put on the replication stream,
      but waiting for 1 or 2 seconds doesn't really guarantee that.
      then the test that expected 6 full syncs, found only 4
      cd81dcf1
  2. 29 Mar, 2021 3 commits
  3. 26 Mar, 2021 1 commit
    • Huang Zhw's avatar
      make processCommand check publish channel permissions. (#8534) · e138698e
      Huang Zhw authored
      Add publish channel permissions check in processCommand.
      
      processCommand didn't check publish channel permissions, so we can
      queue a publish command in a transaction. But when exec the transaction,
      it will fail with -NOPERM.
      
      We also union keys/commands/channels permissions check togegher in
      ACLCheckAllPerm. Remove pubsubCheckACLPermissionsOrReply in 
      publishCommand/subscribeCommand/psubscribeCommand. Always 
      check permissions in processCommand/execCommand/
      luaRedisGenericCommand.
      e138698e
  4. 25 Mar, 2021 1 commit
    • Oran Agra's avatar
      Fix SLOWLOG for blocked commands (#8632) · 497351ad
      Oran Agra authored
      * SLOWLOG didn't record anything for blocked commands because the client
        was reset and argv was already empty. there was a fix for this issue
        specifically for modules, now it works for all blocked clients.
      * The original command argv (before being re-written) was also reset
        before adding the slowlog on behalf of the blocked command.
      * Latency monitor is now updated regardless of the slowlog flags of the
        command or its execution (their purpose is to hide sensitive info from
        the slowlog, not hide the fact the latency happened).
      * Latency monitor now uses real_cmd rather than c->cmd (which may be
        different if the command got re-written, e.g. GEOADD)
      
      Changes:
      * Unify shared code between slowlog insertion in call() and
        updateStatsOnUnblock(), hopefully prevent future bugs from happening
        due to the later being overlooked.
      * Reset CLIENT_PREVENT_LOGGING in resetClient rather than after command
        processing.
      * Add a test for SLOWLOG and BLPOP
      
      Notes:
      - real_cmd == c->lastcmd, except inside MULTI and Lua.
      - blocked commands never happen in these cases (MULTI / Lua)
      - real_cmd == c->cmd, except for when the command is rewritten (e.g.
        GEOADD)
      - blocked commands (currently) are never rewritten
      - other than the command's CLIENT_PREVENT_LOGGING, and the
        execution flag CLIENT_PREVENT_LOGGING, other cases that we want to
        avoid slowlog are on AOF loading (specifically CMD_CALL_SLOWLOG will
        be off when executed from execCommand that runs from an AOF)
      497351ad
  5. 24 Mar, 2021 3 commits
  6. 22 Mar, 2021 3 commits
  7. 17 Mar, 2021 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Fix script kill to work also on scripts that use pcall (#8661) · 9ae4f5c7
      Meir Shpilraien (Spielrein) authored
      pcall function runs another LUA function in protected mode, this means
      that any error will be caught by this function and will not stop the LUA
      execution. The script kill mechanism uses error to stop the running script.
      Scripts that uses pcall can catch the error raise by the script kill mechanism,
      this will cause a script like this to be unkillable:
      
      local f = function()
              while 1 do
                      redis.call('ping')
              end
      end
      while 1 do
              pcall(f)
      end
      
      The fix is, when we want to kill the script, we set the hook function to be invoked 
      after each line. This will promise that the execution will get another
      error before it is able to enter the pcall function again.
      9ae4f5c7
  8. 16 Mar, 2021 2 commits
    • Huang Zhw's avatar
      When tests exit normally, some processes may still be alive (#8647) · a19c4058
      Huang Zhw authored
      In certain scenario start_server may think it failed to start a redis server
      although it started successfully. in these cases, it'll not terminate it, and
      it'll remain running when the test is over.
      
      In start_server if config doesn't have bind (the minimal.conf in introspection.tcl),
      it will try to bind ipv4 and ipv6. One may success while other fails. It will
      output "Could not create server TCP listening socket".
      wait_server_started uses this message to check whether instance started
      successfully. So it will consider that it failed even though redis started successfully.
      
      Additionally, in some cases it wasn't clear to users why the server exited,
      since the warning message printed to the log, could in some cases be harmless,
      and in some cases fatal.
      
      This PR adds makes a clear distinction between a warning log message and
      a fatal one, and changes the test suite to look for the fatal message.
      a19c4058
    • Madelyn Olson's avatar
      Redact slowlog entries for config with sensitive data. (#8584) · e1d98bca
      Madelyn Olson authored
      Redact config set requirepass/masterauth/masteruser from slowlog in addition to showing ACL commands without sensitive values. 
      e1d98bca
  9. 15 Mar, 2021 1 commit
    • guybe7's avatar
      Missing EXEC on modules propagation after failed EVAL execution (#8654) · dba33a94
      guybe7 authored
      1. moduleReplicateMultiIfNeeded should use server.in_eval like
         moduleHandlePropagationAfterCommandCallback
      2. server.in_eval could have been set to 1 and not reset back
         to 0 (a lot of missed early-exits after in_eval is already 1)
      
      Note: The new assertions in processCommand cover (2) and I added
      two module tests to cover (1)
      
      Implications:
      If an EVAL that failed (and thus left server.in_eval=1) runs before a module
      command that replicates, the replication stream will contain MULTI (because
      moduleReplicateMultiIfNeeded used to check server.lua_caller which is NULL
      at this point) but not EXEC (because server.in_eval==1)
      This only affects modules as module.c the only user of server.in_eval.
      
      Affects versions 6.2.0, 6.2.1
      dba33a94
  10. 12 Mar, 2021 1 commit
  11. 11 Mar, 2021 2 commits
  12. 10 Mar, 2021 1 commit
    • guybe7's avatar
      Fix some issues with modules and MULTI/EXEC (#8617) · 3d0b427c
      guybe7 authored
      Bug 1:
      When a module ctx is freed moduleHandlePropagationAfterCommandCallback
      is called and handles propagation. We want to prevent it from propagating
      commands that were not replicated by the same context. Example:
      1. module1.foo does: RM_Replicate(cmd1); RM_Call(cmd2); RM_Replicate(cmd3)
      2. RM_Replicate(cmd1) propagates MULTI and adds cmd1 to also_propagagte
      3. RM_Call(cmd2) create a new ctx, calls call() and destroys the ctx.
      4. moduleHandlePropagationAfterCommandCallback is called, calling
         alsoPropagates EXEC (Note: EXEC is still not written to socket),
         setting server.in_trnsaction = 0
      5. RM_Replicate(cmd3) is called, propagagting yet another MULTI (now
         we have nested MULTI calls, which is no good) and then cmd3
      
      We must prevent RM_Call(cmd2) from resetting server.in_transaction.
      REDISMODULE_CTX_MULTI_EMITTED was revived for that purpose.
      
      Bug 2:
      Fix issues with nested RM_Call where some have '!' and some don't.
      Example:
      1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!')
      2. module2.bar internally calls RM_Call of INCR with '!'
      3. at the end of module1.foo we call RM_ReplicateVerbatim
      
      We want the replica/AOF to see only module1.foo and not the INCR from module2.bar
      
      Introduced a global replication_allowed flag inside RM_Call to determine
      whether we need to replicate or not (even if '!' was specified)
      
      Other changes:
      Split beforePropagateMultiOrExec to beforePropagateMulti afterPropagateExec
      just for better readability
      3d0b427c
  13. 08 Mar, 2021 2 commits
  14. 04 Mar, 2021 1 commit
    • Yossi Gottlieb's avatar
      Improve redis-cli non-binary safe string handling. (#8566) · 3c7d6a18
      Yossi Gottlieb authored
      * The `redis-cli --scan` output should honor output mode (set explicitly or implicitly), and quote key names when not in raw mode.
        * Technically this is a breaking change, but it should be very minor since raw mode is by default on for non-tty output.
        * It should only affect  TTY output (human users) or non-tty output if `--no-raw` is specified.
      
      * Added `--quoted-input` option to treat all arguments as potentially quoted strings.
      * Added `--quoted-pattern` option to accept a potentially quoted pattern.
      
      Unquoting is applied to potentially quoted input only if single or double quotes are used. 
      
      Fixes #8561, #8563
      3c7d6a18
  15. 02 Mar, 2021 1 commit
  16. 01 Mar, 2021 4 commits
  17. 28 Feb, 2021 1 commit
    • Viktor Söderqvist's avatar
      Shared reusable client for RM_Call() (#8516) · 6122f1c4
      Viktor Söderqvist authored
      A single client pointer is added in the server struct. This is
      initialized by the first RM_Call() and reused for every subsequent
      RM_Call() except if it's already in use, which means that it's not
      used for (recursive) module calls to modules. For these, a new
      "fake" client is created each time.
      
      Other changes:
      * Avoid allocating a dict iterator in pubsubUnsubscribeAllChannels
        when not needed
      6122f1c4
  18. 24 Feb, 2021 2 commits
  19. 23 Feb, 2021 2 commits
    • guybe7's avatar
      Fix race in CONFIG REWRITE sanity (#8536) · f745c018
      guybe7 authored
      server may still be LOADING the RDB when receiving the ping
      f745c018
    • Yossi Gottlieb's avatar
      Fix failed tests on Linux Alpine and add a CI job. (#8532) · 95ea7454
      Yossi Gottlieb authored
      * Remove linux/version.h dependency.
      
      This introduces unnecessary dependencies, and generally not a good idea
      as the platform we build on may be different than the platform we run
      on.
      
      To determine if sync_file_range exists we can simply rely on header file
      hints.
      
      * Fix setproctitle() on libmusl.
      
      The previous ifdef checks were a bit too strict for no apparent
      reason.
      
      * Fix tests failure on Linux with no backtrace.
      
      * Add alpine daily CI job.
      95ea7454
  20. 22 Feb, 2021 1 commit
  21. 21 Feb, 2021 2 commits
    • Huang Zw's avatar
      Client tracking tracking-redir-broken push len is 2 not 3 (#8456) · f687ac0c
      Huang Zw authored
      When redis responds with tracking-redir-broken push message (RESP3),
      it was responding with a broken protocol: an array of 3 elements, but only
      pushes 2 elements.
      
      Some bugs in the test make this pass. Read the push reply
      will consume an extra reply, because the reply length is 3, but there
      are only two elements, so the next reply will be treated as third
      element. So the test is corrected too.
      
      Other changes:
      * checkPrefixCollisionsOrReply success should return 1 instead of -1,
        this bug didn't have any implications.
      * improve client tracking tests to validate more of the response it reads.
      f687ac0c
    • Gnanesh's avatar
      EXPIRE, EXPIREAT, SETEX, GETEX: Return error when expire time overflows (#8287) · 0772098b
      Gnanesh authored
      
      
      Respond with error if expire time overflows from positive to negative of vice versa.
      
      * `SETEX`, `SET EX`, `GETEX` etc would have already error on negative value,
      but now they would also error on overflows (i.e. when the input was positive but
      after the manipulation it becomes negative, which would have passed before)
      * `EXPIRE` and `EXPIREAT` was ok taking negative values (would implicitly delete
      the key), we keep that, but we do error if the user provided a value that changes
      sign when manipulated (except the case of changing sign when `basetime` is added)
      Signed-off-by: default avatarGnanesh <gnaneshkunal@outlook.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      0772098b
  22. 19 Feb, 2021 1 commit
  23. 16 Feb, 2021 1 commit
    • uriyage's avatar
      Adds INFO fields to track fork child progress (#8414) · fd052d2a
      uriyage authored
      * Adding current_save_keys_total and current_save_keys_processed info fields.
        Present in replication, BGSAVE and AOFRW.
      * Changing RM_SendChildCOWInfo() to RM_SendChildHeartbeat(double progress)
      * Adding new info field current_fork_perc. Present in Replication, BGSAVE, AOFRW,
        and module forks.
      fd052d2a
  24. 15 Feb, 2021 2 commits
    • Oran Agra's avatar
      minor test suite cleanup, revive old test (#8497) · fb3457d1
      Oran Agra authored
      There are two tests in other.tcl that were dependant of the sha1 package
      import which meant that they didn't usually run.
      The reason it was like that was that prior to the creation of DEBUG
      DIGEST, the test suite used to have an equivalent function, but that's
      no longer the case and this dependency isn't needed.
      
      The other change is to revert config changes done by the test before the
      test suite continues. can be useful if using `--host` to run multiple
      units against the same server
      fb3457d1
    • Yossi Gottlieb's avatar
      Escape unsafe field name characters in INFO. (#8492) · 141ac8df
      Yossi Gottlieb authored
      Fixes #8489
      141ac8df