1. 03 Jun, 2022 1 commit
  2. 02 Jun, 2022 3 commits
    • zhaozhao.zz's avatar
      rewrite alias config to original name (#10811) · a18c91d6
      zhaozhao.zz authored
      
      
      Redis 7 adds some new alias config like `hash-max-listpack-entries` alias `hash-max-ziplist-entries`.
      
      If a config file contains both real name and alias like this:
      ```
      hash-max-listpack-entries 20
      hash-max-ziplist-entries 20
      ```
      
      after set `hash-max-listpack-entries` to 100 and `config rewrite`, the config file becomes to:
      ```
      hash-max-listpack-entries 100
      hash-max-ziplist-entries 20
      ```
      
      we can see that the alias config is not modified, and users will get wrong config after restart.
      
      6.0 and 6.2 doesn't have this bug, since they only have the `slave` word alias.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      a18c91d6
    • chenyang8094's avatar
      Only print final log when aof is loaded successfully (#10808) · aae0ec25
      chenyang8094 authored
      Skip the print on AOF_NOT_EXIST status.
      aae0ec25
    • zhugezy's avatar
      Fix bugs in CONFIG REWRITE, omitting rename-command and include lines, and... · cf3323db
      zhugezy authored
      
      Fix bugs in CONFIG REWRITE, omitting rename-command and include lines, and inserting comments around module and acl configs (#10761)
      
      A regression from #10285 (redis 7.0).
      CONFIG REWRITE would put lines with: `include`, `rename-command`,
      `user`,  `loadmodule`, and any module specific config in a comment.
      
      For ACL `user`, `loadmodule` and module specific configs would be
      re-inserted at the end (instead of updating existing lines), so the only
      implication is a messy config file full of comments.
      
      But for `rename-command` and `include`, the implication would be that
      they're now missing, so a server restart would lose them.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      cf3323db
  3. 01 Jun, 2022 3 commits
    • Oran Agra's avatar
      Expose script flags to processCommand for better handling (#10744) · df558618
      Oran Agra authored
      The important part is that read-only scripts (not just EVAL_RO
      and FCALL_RO, but also ones with `no-writes` executed by normal EVAL or
      FCALL), will now be permitted to run during CLIENT PAUSE WRITE (unlike
      before where only the _RO commands would be processed).
      
      Other than that, some errors like OOM, READONLY, MASTERDOWN are now
      handled by processCommand, rather than the command itself affects the
      error string (and even error code in some cases), and command stats.
      
      Besides that, now the `may-replicate` commands, PFCOUNT and PUBLISH, will
      be considered `write` commands in scripts and will be blocked in all
      read-only scripts just like other write commands.
      They'll also be blocked in EVAL_RO (i.e. even for scripts without the
      `no-writes` shebang flag.
      
      This commit also hides the `may_replicate` flag from the COMMAND command
      output. this is a **breaking change**.
      
      background about may_replicate:
      We don't want to expose a no-may-replicate flag or alike to scripts, since we
      consider the may-replicate thing an internal concern of redis, that we may
      some day get rid of.
      In fact, the may-replicate flag was initially introduced to flag EVAL: since
      we didn't know what it's gonna do ahead of execution, before function-flags
      existed). PUBLISH and PFCOUNT, both of which because they have side effects
      which may some day be fixed differently.
      
      code changes:
      The changes in eval.c are mostly code re-ordering:
      - evalCalcFunctionName is extracted out of evalGenericCommand
      - evalExtractShebangFlags is extracted luaCreateFunction
      - evalGetCommandFlags is new code
      df558618
    • Oran Agra's avatar
      Fix broken protocol in MISCONF error, RM_Yield bugs, RM_Call(EVAL) OOM check... · b2061de2
      Oran Agra authored
      Fix broken protocol in MISCONF error, RM_Yield bugs, RM_Call(EVAL) OOM check bug, and new RM_Call checks. (#10786)
      
      * Fix broken protocol when redis can't persist to RDB (general commands, not
        modules), excessive newline. regression of #10372 (7.0 RC3)
      * Fix broken protocol when Redis can't persist to AOF (modules and
        scripts), missing newline.
      * Fix bug in OOM check of EVAL scripts called from RM_Call.
        set the cached OOM state for scripts before executing module commands too,
        so that it can serve scripts that are executed by modules.
        i.e. in the past EVAL executed by RM_Call could have either falsely
        fail or falsely succeeded because of a wrong cached OOM state flag.
      * Fix bugs with RM_Yield:
        1. SHUTDOWN should only accept the NOSAVE mode
        2. Avoid eviction during yield command processing.
        3. Avoid processing master client commands while yielding from another client
      * Add new two more checks to RM_Call script mode.
        1. READONLY You can't write against a read only replica
        2. MASTERDOWN Link with MASTER is down and `replica-serve-stale-data` is set to `no`
      * Add new RM_Call flag to let redis automatically refuse `deny-oom` commands
        while over the memory limit. 
      * Add tests to cover various errors from Scripts, Modules, Modules
        calling scripts, and Modules calling commands in script mode.
      
      Add tests:
      * Looks like the MISCONF error was completely uncovered by the tests,
        add tests for it, including from scripts, and modules
      * Add tests for NOREPLICAS from scripts
      * Add tests for the various errors in module RM_Call, including RM_Call that
        calls EVAL, and RM_call in "eval mode". that includes:
        NOREPLICAS, READONLY, MASTERDOWN, MISCONF
      b2061de2
    • filipe oliveira's avatar
      Moving client flags to a more cache friendly position within client struct (#10697) · 6a6e911f
      filipe oliveira authored
      Move the client flags to a more cache friendly position within the client struct
      we regain the lost 2% of CPU cycles since v6.2 ( from 630532.57 to 647449.80 ops/sec ).
      These are due to higher rate of calls to getClientType due to changes in #9166 and #10020 
      6a6e911f
  4. 31 May, 2022 5 commits
    • menwen's avatar
      Add module API RM_MallocUsableSize (#10795) · 42fbf064
      menwen authored
      This allows the module to know the usable size of an allocation
      it made, rather than the consumed size.
      42fbf064
    • zhaozhao.zz's avatar
      rename channel to shardchannel in sharded pubsub commands (#10738) · 805191c7
      zhaozhao.zz authored
      since the sharded pubsub is different with pubsub, it's better to give users a hint to make it more clear.
      805191c7
    • DarrenJiang13's avatar
      Adds isolated netstats for replication. (#10062) · bb1de082
      DarrenJiang13 authored
      
      
      The amount of `server.stat_net_output_bytes/server.stat_net_input_bytes`
      is actually the sum of replication flow and users' data flow. 
      It may cause confusions like this:
      "Why does my server get such a large output_bytes while I am doing nothing? ". 
      
      After discussions and revisions, now here is the change about what this
      PR brings (final version before merge):
      - 2 server variables to count the network bytes during replication,
           including fullsync and propagate bytes.
           - `server.stat_net_repl_output_bytes`/`server.stat_net_repl_input_bytes`
      - 3 info fields to print the input and output of repl bytes and instantaneous
           value of total repl bytes.
           - `total_net_repl_input_bytes` / `total_net_repl_output_bytes`
           - `instantaneous_repl_total_kbps`
      - 1 new API `rioCheckType()` to check the type of rio. So we can use this
           to distinguish between diskless and diskbased replication
      - 2 new counting items to keep network statistics consistent between master
           and slave
          - rdb portion during diskless replica. in `rdbLoadProgressCallback()`
          - first line of the full sync payload. in `readSyncBulkPayload()`
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      bb1de082
    • Harkrishn Patro's avatar
      Sharded pubsub publish messagebulk as smessage (#10792) · 4065b4f2
      Harkrishn Patro authored
      To easily distinguish between sharded channel message and a global
      channel message, introducing `smessage` (instead of `message`) as
      message bulk for sharded channel publish message.
      
      This is gonna be a breaking change in 7.0.1!
      
      Background:
      Sharded pubsub introduced in redis 7.0, but after the release we quickly
      realized that the fact that it's problematic that the client can't distinguish
      between normal (global) pubsub messages and sharded ones.
      This is important because the same connection can subscribe to both,
      but messages sent to one pubsub system are not propagated to the
      other (they're completely separate), so if one connection is used to
      subscribe to both, we need to assist the client library to know which
      message it got so it can forward it to the correct callback.
      4065b4f2
    • Wen Hui's avatar
      Fix streamParseAddOrTrimArgsOrReply function minor comment issue (#10783) · d7ae8587
      Wen Hui authored
      
      
      When I read the source codes, I have no idea where the option "age" come from.
      Co-authored-by: default avatarUbuntu <lucas.guang.yang1@huawei.com>
      Co-authored-by: default avatarguybe7 <guy.benoish@redislabs.com>
      d7ae8587
  5. 30 May, 2022 1 commit
  6. 29 May, 2022 3 commits
  7. 27 May, 2022 2 commits
    • Vitaly's avatar
      Fix ZRANGESTORE crash when zset_max_listpack_entries is 0 (#10767) · 6461f09f
      Vitaly authored
      When `zrangestore` is called container destination object is created. 
      Before this PR we used to create a listpack based object even if `zset-max-ziplist-entries`
      or equivalent`zset-max-listpack-entries` was set to 0.
      This triggered immediate conversion of the listpack into a skiplist in `zrangestore`, which hits
      an assertion resulting in an engine crash.
      
      Added a TCL test that reproduces this issue.
      6461f09f
    • Binbin's avatar
      Fix some commands key spec in json files (#10779) · 2a099d49
      Binbin authored
      There are some commands that has the wrong key specs.
      This PR adds a key-spec related check in generate-command-code.py.
      Check if the index is valid, or if there is an unused index.
      
      The check result will look like:
      ```
      [root]# python utils/generate-command-code.py
      Processing json files...
      Linking container command to subcommands...
      Checking all commands...
      command: RESTORE_ASKING may have unused key_spec
      command: RENAME may have unused key_spec
      command: PFDEBUG may have unused key_spec
      command: WATCH key_specs missing flags
      command: LCS arg: key2 key_spec_index error
      command: RENAMENX may have unused key_spec
      Error: There are errors in the commands check, please check the above logs.
      ```
      
      The following commands have been fixed according to the check results:
      - RESTORE ASKING: add missing arguments section (and history section)
      - RENAME: newkey's key_spec_index should be 1
      - PFDEBUG: add missing arguments (and change the arity from -3 to 3)
      - WATCH: add missing key_specs flags: RO, like EXIST (it allow you to know the key exists, or is modified, but doesn't "leak" the data)
      - LCS: key2 key_spec_index error, there is only one key-spec
      - RENAMENX: newkey's key_spec_index should be 1
      2a099d49
  8. 26 May, 2022 3 commits
  9. 23 May, 2022 1 commit
    • Binbin's avatar
      Fix BZMPOP gets unblocked by non-key args and returns them (#10764) · 450c88f3
      Binbin authored
      This bug was introduced in #9484 (7.0.0).
      It result that BZMPOP blocked on non-key arguments.
      
      Like `bzmpop 0 1 myzset min count 10`, this command will additionally
      block in these keys (except for the first and the last argument) and can return their values:
      - 0: timeout value
      - 1: numkeys value
      - min: min/max token
      - count: count token
      450c88f3
  10. 22 May, 2022 5 commits
    • yoav-steinberg's avatar
      Add warning for suspected slow system clocksource setting (#10636) · 843a4cdc
      yoav-steinberg authored
      This PR does 2 main things:
      1) Add warning for suspected slow system clocksource setting. This is Linux specific.
      2) Add a `--check-system` argument to redis which runs all system checks and prints a report.
      
      ## System checks
      Add a command line option `--check-system` which runs all known system checks and provides
      a report to stdout of which systems checks have failed with details on how to reconfigure the
      system for optimized redis performance.
      The `--system-check` mode exists with an appropriate error code after running all the checks.
      
      ## Slow clocksource details
      We check the system's clocksource performance by running `clock_gettime()` in a loop and then
      checking how much time was spent in a system call (via `getrusage()`). If we spend more than
      10% of the time in the kernel then we print a warning. I verified that using the slow clock sources:
      `acpi_pm` (~90% in the kernel on my laptop) and `xen` (~30% in the kernel on ...
      843a4cdc
    • Oran Agra's avatar
      Scripts that declare the `no-writes` flag are implicitly `allow-oom` too. (#10699) · b0e18f80
      Oran Agra authored
      Scripts that have the `no-writes` flag, cannot execute write commands,
      and since all `deny-oom` commands are write commands, we now act
      as if the `allow-oom` flag is implicitly set for scripts that set the `no-writes` flag.
      this also implicitly means that the EVAL*_RO and FCALL_RO commands can
      never fails with OOM error.
      
      Note about a bug that's no longer relevant:
      There was an issue with EVAL*_RO using shebang not being blocked correctly
      in OOM state:
      When an EVAL script declares a shebang, it was by default not allowed to run in
      OOM state.
      but this depends on a flag that is updated before the command is executed, which
      was not updated in case of the `_RO` variants.
      the result is that if the previous cached state was outdated (either true or false),
      the script will either unjustly fail with OOM, or unjustly allowed to run despite
      the OOM state.
      It doesn't affect scripts without a shebang since these depend on the actual
      commands they run, and since these are only read commands, they don't care
      for that cached oom state flag.
      it did affect scripts with shebang and no allow-oom flag, bug after the change in
      this PR, scripts that are run with eval_ro would implicitly have that flag so again
      the cached state doesn't matter.
      
      p.s. this isn't a breaking change since all it does is allow scripts to run when they
      should / could rather than blocking them.
      b0e18f80
    • Binbin's avatar
      Remove ziplist dead code in object.c (#10751) · 18cb4a7d
      Binbin authored
      Remove some dead code in object.c, ziplist is no longer used in 7.0
      
      Some backgrounds:
      zipmap - hash: replaced by ziplist in #285
      ziplist - hash: replaced by listpack in #8887
      ziplist - zset: replaced by listpack in #9366
      ziplist - list: replaced by quicklist (listpack) in #2143 / #9740
      
      Moved the location of ziplist.h in the server.c
      18cb4a7d
    • Yuuoniy's avatar
      Fix memory leak in streamGetEdgeID (#10753) · 4a7a4e42
      Yuuoniy authored
      si is initialized by streamIteratorStart(), we should call
      streamIteratorStop() on it when done.
      
      regression introduced in #9127 (redis 7.0)
      4a7a4e42
    • Ofir Luzon's avatar
      Add SIGINT handler to redis-cli --bigkeys, --memkeys, --hotkeys, --scan (#10736) · 00a9d6b3
      Ofir Luzon authored
      Finish current loop and display the scanned keys summery on SIGINT (Ctrl-C) signal.
      It will also prepend the current scanned percentage to the scanned keys summery 1st line.
      
      In this commit I've renamed and relocated `intrinsicLatencyModeStop` function as I'm using the exact same logic.
      00a9d6b3
  11. 18 May, 2022 1 commit
  12. 16 May, 2022 2 commits
  13. 15 May, 2022 2 commits
  14. 13 May, 2022 2 commits
  15. 12 May, 2022 1 commit
    • yoav-steinberg's avatar
      Fix possible regression around TLS config changes. Add VOLATILE_CONFIG flag... · b16d1c27
      yoav-steinberg authored
      Fix possible regression around TLS config changes. Add VOLATILE_CONFIG flag for volatile configurations. (#10713)
      
      This fixes a possible regression in Redis 7.0.0, in which doing CONFIG SET
      on a TLS config would not reload the configuration in case the new config is
      the same file as before.
      
      A volatile configuration is a configuration value which is a reference to the
      configuration data and not the configuration data itself. In such a case Redis
      doesn't know if the config data changed under the hood and can't assume a
      change happens only when the config value changes. Therefore it needs to
      be applied even when setting a config value to the same value as it was before.
      b16d1c27
  16. 11 May, 2022 3 commits
    • Yossi Gottlieb's avatar
      Fix Makefile.dep generation with ICC. (#10708) · 8bdd2d5d
      Yossi Gottlieb authored
      Before this commit, all source files including those that are not going
      to be compiled were used. Some of these files are platform specific and
      won't even pre-process on another platform. With GCC/Clang, that's not
      an issue and they'll simply ignore them, but ICC aborts in this case.
      
      This commit only attempts to generate Makefile.dep from the actual set
      of C source files that will be compiled.
      8bdd2d5d
    • Binbin's avatar
      redis-server command line arguments support take one bulk string with spaces... · bfbb15f7
      Binbin authored
      redis-server command line arguments support take one bulk string with spaces for MULTI_ARG configs parsing. And allow options value to use the -- prefix (#10660)
      
      ## Take one bulk string with spaces for MULTI_ARG configs parsing
      Currently redis-server looks for arguments that start with `--`,
      and anything in between them is considered arguments for the config.
      like: `src/redis-server --shutdown-on-sigint nosave force now --port 6380`
      
      MULTI_ARG configs behave differently for CONFIG command, vs the command
      line argument for redis-server.
      i.e. CONFIG command takes one bulk string with spaces in it, while the
      command line takes an argv array with multiple values.
      
      In this PR, in config.c, if `argc > 1` we can take them as is,
      and if the config is a `MULTI_ARG` and `argc == 1`, we will split it by spaces.
      
      So both of these will be the same:
      ```
      redis-server --shutdown-on-sigint nosave force now --shutdown-on-sigterm nosave force
      redis-server --shutdown-on-sigint nosave "force now" --shutdown-on-sigterm nosave force
      redis-server --shutdown-on-sigint nosave "force now" --shutdown-on-sigterm "nosave force"
      ```
      
      ## Allow options value to use the `--` prefix
      Currently it decides to switch to the next config, as soon as it sees `--`, 
      even if there was not a single value provided yet to the last config,
      this makes it impossible to define a config value that has `--` prefix in it.
      
      For instance, if we want to set the logfile to `--my--log--file`,
      like `redis-server --logfile --my--log--file --loglevel verbose`,
      current code will handle that incorrectly.
      
      In this PR, now we allow a config value that has `--` prefix in it.
      **But note that** something like `redis-server --some-config --config-value1 --config-value2 --loglevel debug`
      would not work, because if you want to pass a value to a config starting with `--`, it can only be a single value.
      like: `redis-server --some-config "--config-value1 --config-value2" --loglevel debug`
      
      An example (using `--` prefix config value):
      ```
      redis-server --logfile --my--log--file --loglevel verbose
      redis-cli config get logfile loglevel
      1) "loglevel"
      2) "verbose"
      3) "logfile"
      4) "--my--log--file"
      ```
      
      ### Potentially breaking change
      `redis-server --save --loglevel verbose` used to work the same as `redis-server --save "" --loglevel verbose`
      now, it'll error!
      bfbb15f7
    • Binbin's avatar
      FLUSHDB and FLUSHALL add call forceCommandPropagation / FLUSHALL reset dirty... · 783b210d
      Binbin authored
      FLUSHDB and FLUSHALL add call forceCommandPropagation / FLUSHALL reset dirty counter to 0 if we enable save (#10691)
      
      ## FLUSHALL
      We used to restore the dirty counter after `rdbSave` zeroed it if we enable save.
      Otherwise FLUSHALL will not be replicated nor put into the AOF.
      
      And then we do increment it again below.
      Without that extra dirty++, when db was already empty, FLUSHALL
      will not be replicated nor put into the AOF.
      
      We now gonna replace all that dirty counter magic with a call
      to forceCommandPropagation (REPL and AOF), instead of all the
      messing around with the dirty counter.
      Added tests to cover three part (dirty counter, REPL, AOF).
      
      One benefit other than cleaner code is that the `rdb_changes_since_last_save` is correct in this case.
      
      ## FLUSHDB
      FLUSHDB was not replicated nor put into the AOF when db was already empty.
      Unlike DEL on a non-existing key, FLUSHDB always does something, and that's to call the module hook. 
      So basically FLUSHDB is never a NOP, and thus it should always be propagated.
      Not doing that, could mean that if a module does something in that hook, and wants to
      avoid issues of that hook being missing on the replica if the db is empty, it'll need to do complicated things.
      
      So now FLUSHDB add call forceCommandPropagation, we will always propagate FLUSHDB.
      Always propagating FLUSHDB seems like a safe approach that shouldn't have any drawbacks (other than looking odd)
      
      This was mentioned in #8972
      
      ## Test section:
      We actually found it while solving a race condition in the BGSAVE test (other.tcl).
      It was found in extra_ci Daily Arm64 (test-libc-malloc).
      ```
      [exception]: Executing test client: ERR Background save already in progress.
      ERR Background save already in progress
      ```
      
      It look like `r flushdb` trigger (schedule) a bgsave right after `waitForBgsave r` and before `r save`.
      Changing flushdb to flushall, FLUSHALL will do a foreground save and then set the dirty counter to 0.
      783b210d
  17. 10 May, 2022 2 commits
    • guybe7's avatar
      Dediacted member to hold RedisModuleCommand (#10681) · 815a6f84
      guybe7 authored
      Fix #10552
      
      We no longer piggyback getkeys_proc to hold the RedisModuleCommand struct, when exists
      
      Others:
      Use `doesCommandHaveKeys` in `RM_GetCommandKeysWithFlags` and `getKeysSubcommandImpl`.
      It causes a very minor behavioral change in commands that don't have actual keys, but have a spec
      with `CMD_KEY_NOT_KEY`.
      For example, before this command `COMMAND GETKEYS SPUBLISH` would return
      `Invalid arguments specified for command` but not it returns `The command has no key arguments`
      815a6f84
    • Mariya Markova's avatar
      Replace float zero comparison to FP_ZERO comparison (#10675) · c2d8d4e6
      Mariya Markova authored
      I suggest to use "[fpclassify](https://en.cppreference.com/w/cpp/numeric/math/fpclassify)" for float
      comparison with zero, because of expression "value == 0" with value very close to zero can be
      considered as true with some performance compiler optimizations.
      
      Note: this code was introduced by 9d520a7f to accept zset scores that get ERANGE in conversion
      due to precision loss near 0.
      But with Intel compilers, ICC and ICX, where optimizations for 0 check are more aggressive, "==0" is
      true for mentioned functions, however should not be. Behavior is seen starting from O2.
      This leads to a failure in the ZSCAN test in scan.tcl
      c2d8d4e6