1. 12 Dec, 2022 3 commits
    • Binbin's avatar
      Fix command line startup --sentinel problem (#11591) · b322a77e
      Binbin authored
      There is a issue with --sentinel:
      ```
      [root]# src/redis-server sentinel.conf --sentinel --loglevel verbose
      
      *** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
      Reading the configuration file, at line 352
      >>> 'sentinel "--loglevel" "verbose"'
      Unrecognized sentinel configuration statement
      ```
      
      This is because in #10660 (Redis 7.0.1), `--` prefix change break it.
      In this PR, we will handle `--sentinel` the same as we did for `--save`
      in #10866. i.e. it's a pseudo config option with no value.
      
      (cherry picked from commit 8f13ac10)
      b322a77e
    • Oran Agra's avatar
      diskless master, avoid bgsave child hung when fork parent crashes (#11463) · 7c956d5c
      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).
      
      (cherry picked from commit ccaef5c9)
      7c956d5c
    • Steffen Moser's avatar
      Fixing compilation by removing flock() when compiling on Solaris (#11327) · 929ab58a
      Steffen Moser authored
      SunOS/Solaris and its relatives don't support the flock() function.
      While "redis" has been excluding setting up the lock using flock() on the cluster
      configuration file when compiling under Solaris, it was still using flock() in the
      unlock call while shutting down.
      
      This pull request eliminates the flock() call also in the unlocking stage
      for Oracle Solaris and its relatives.
      
      Fix compilation regression from #10912
      
      (cherry picked from commit 6aab4cb7)
      929ab58a
  2. 21 Sep, 2022 2 commits
    • Shaya Potter's avatar
      Improve cmd_flags for script/functions in RM_Call (#11159) · a1ec0cae
      Shaya Potter authored
      When RM_Call was used with `M` (reject OOM), `W` (reject writes),
      as well as `S` (rejecting stale or write commands in "Script mode"),
      it would have only checked the command flags, but not the declared
      script flag in case it's a command that runs a script.
      
      Refactoring: extracts out similar code in server.c's processCommand
      to be usable in RM_Call as well.
      
      (cherry picked from commit bed6d759)
      a1ec0cae
    • Shay Fadida's avatar
      Fix missing sections for INFO ALL with module (#11291) · c9eabbf9
      Shay Fadida authored
      
      
      When using `INFO ALL <section>`, when `section` is a specific module section. 
      Redis will not print the additional section(s).
      
      The fix in this case, will search the modules info sections if the user provided additional sections to `ALL`.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit eedb8b17)
      c9eabbf9
  3. 11 Jul, 2022 1 commit
  4. 07 Jul, 2022 1 commit
  5. 06 Jul, 2022 1 commit
  6. 04 Jul, 2022 1 commit
    • Qu Chen's avatar
      Unlock cluster config file upon server shutdown. (#10912) · 33b7ff38
      Qu Chen authored
      Currently in cluster mode, Redis process locks the cluster config file when
      starting up and holds the lock for the entire lifetime of the process.
      When the server shuts down, it doesn't explicitly release the lock on the
      cluster config file. We noticed a problem with restart testing that if you shut down
      a very large redis-server process (i.e. with several hundred GB of data stored),
      it takes the OS a while to free the resources and unlock the cluster config file.
      So if we immediately try to restart the redis server process, it might fail to acquire
      the lock on the cluster config file and fail to come up.
      
      This fix explicitly releases the lock on the cluster config file upon a shutdown rather
      than relying on the OS to release the lock, which is a cleaner and safer approach to
      free up resources acquired. 
      33b7ff38
  7. 30 Jun, 2022 1 commit
    • Binbin's avatar
      Add SENTINEL command flag to CLIENT/COMMANDS subcommands (#10904) · 35e836c2
      Binbin authored
      This was harmless because we marked the parent command
      with SENTINEL flag. So the populateCommandTable was ok.
      And we also don't show the flag (SENTINEL and ONLY-SENTNEL)
      in COMMAND INFO.
      
      In this PR, we also add the same CMD_SENTINEL and CMD_ONLY_SENTINEL
      flags check when populating the sub-commands.
      so that in the future it'll be possible to add some sub-commands to sentinel or sentinel-only but not others.
      35e836c2
  8. 26 Jun, 2022 1 commit
    • Binbin's avatar
      redis-server command line arguments allow passing config name and value in the same arg (#10866) · d443e312
      Binbin authored
      This commit has two topics.
      
      ## Passing config name and value in the same arg
      In #10660 (Redis 7.0.1), when we supported the config values that can start with `--` prefix (one of the two topics of that PR),
      we broke another pattern: `redis-server redis.config "name value"`, passing both config name
      and it's value in the same arg, see #10865
      
      This wasn't a intended change (i.e we didn't realize this pattern used to work).
      Although this is a wrong usage, we still like to fix it.
      
      Now we support something like:
      ```
      src/redis-server redis.conf "--maxmemory '700mb'" "--maxmemory-policy volatile-lru" --proc-title-template --my--title--template --loglevel verbose
      ```
      
      ## Changes around --save
      Also in this PR, we undo the breaking change we made in #10660 on purpose.
      1. `redis-server redis.conf --save --loglevel verbose` (missing `save` argument before anotehr argument).
          In 7.0.1, it was throwing an wrong arg error.
          Now it will work and reset the save, similar to how it used to be in 7.0.0 and 6.2.x.
      3. `redis-server redis.conf --loglevel verbose --save` (missing `save` argument as last argument).
          In 6.2, it did not reset the save, which was a bug (inconsistent with the previous bullet).
          Now we will make it work and reset the save as well (a bug fix).
      d443e312
  9. 23 Jun, 2022 1 commit
  10. 12 Jun, 2022 2 commits
    • XiongDa's avatar
      Fix 3 comments in server.c (#10844) · abb2ea7e
      XiongDa authored
      abb2ea7e
    • Binbin's avatar
      Fixed SET and BITFIELD commands being wrongly marked movablekeys (#10837) · 92fb4f4f
      Binbin authored
      
      
      The SET and BITFIELD command were added `get_keys_function` in #10148, causing
      them to be wrongly marked movablekeys in `populateCommandMovableKeys`.
      
      This was an unintended side effect introduced in #10148 (7.0 RC1)
      which could cause some clients an extra round trip for these commands in cluster mode.
      
      Since we define movablekeys as a way to determine if the legacy range [first, last, step]
      doesn't find all keys, then we need a completely different approach.
      
      The right approach should be to check if the legacy range covers all key-specs,
      and if none of the key-specs have the INCOMPLETE flag. 
      This way, we don't need to look at getkeys_proc of VARIABLE_FLAG at all.
      Probably with the exception of modules, who may still not be using key-specs.
      
      In this PR, we removed `populateCommandMovableKeys` and put its logic in
      `populateCommandLegacyRangeSpec`.
      In order to properly serve both old and new modules, we must probably keep relying
      CMD_MODULE_GETKEYS, but do that only for modules that don't declare key-specs. 
      For ones that do, we need to take the same approach we take with native redis commands.
      
      This approach was proposed by Oran. Fixes #10833
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      92fb4f4f
  11. 11 Jun, 2022 1 commit
    • Binbin's avatar
      Fix crash when overcommit_memory is inaccessible (#10848) · 62ac1ab0
      Binbin authored
      When `/proc/sys/vm/overcommit_memory` is inaccessible, fp is NULL.
      `checkOvercommit` will return -1 without setting err_msg, and then
      the err_msg is used to print the log, crash the server.
      Set the err_msg variables to Null when declaring it, seems safer.
      
      And the overcommit_memory error log will print two "WARNING",
      like `WARNING WARNING overcommit_memory is set to 0!`, this PR
      also removes the second WARNING in `checkOvercommit`.
      
      Reported in #10846. Fixes #10846. Introduced in #10636 (7.0.1)
      62ac1ab0
  12. 06 Jun, 2022 1 commit
    • DarrenJiang13's avatar
      Split instantaneous_repl_total_kbps to instantaneous_input_repl_kbps and... · f5585834
      DarrenJiang13 authored
      Split instantaneous_repl_total_kbps to instantaneous_input_repl_kbps and instantaneous_output_repl_kbps. (#10810)
      
      A supplement to https://github.com/redis/redis/pull/10062
      Split `instantaneous_repl_total_kbps` to `instantaneous_input_repl_kbps` and `instantaneous_output_repl_kbps`. 
      ## Work:
      This PR:
      - delete 1 info field:
          - `instantaneous_repl_total_kbps`
      - add 2 info fields:
          - `instantaneous_input_repl_kbps / instantaneous_output_repl_kbps`
      ## Result:
      - master
      ```
      total_net_input_bytes:26633673
      total_net_output_bytes:21716596
      total_net_repl_input_bytes:0
      total_net_repl_output_bytes:18433052
      instantaneous_input_kbps:0.02
      instantaneous_output_kbps:0.00
      instantaneous_input_repl_kbps:0.00
      instantaneous_output_repl_kbps:0.00
      ```
      - slave
      ```
      total_net_input_bytes:18433212
      total_net_output_bytes:94790
      total_net_repl_input_bytes:18433052
      total_net_repl_output_bytes:0
      instantaneous_input_kbps:0.00
      instantaneous_output_kbps:0.05
      instantaneous_input_repl_kbps:0.00
      instantaneous_output_repl_kbps:0.00
      ```
      f5585834
  13. 02 Jun, 2022 1 commit
  14. 01 Jun, 2022 2 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
  15. 31 May, 2022 2 commits
    • 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
  16. 30 May, 2022 1 commit
  17. 26 May, 2022 1 commit
  18. 22 May, 2022 2 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 an ec2 `m4.large`)
      we get this warning.
      
      The check runs 5 system ticks so we can detect time spent in kernel at 20% jumps (0%,20%,40%...).
      Anything more accurate will require the test to run longer. Typically 5 ticks are 50ms. This means
      running the test on startup will delay startup by 50ms. To avoid this we make sure the test is only
      executed in the `--check-system` mode.
      
      For a quick startup check, we specifically warn if the we see the system is using the `xen` clocksource
      which we know has bad performance and isn't recommended (at least on ec2). In such a case the
      user should manually rung redis with `--check-system` to force the slower clocksource test described
      above.
      
      ## Other changes in the PR
      
      * All the system checks are now implemented as functions in _syscheck.c_.
        They are implemented using a standard interface (see details in _syscheck.c_).
        To do this I moved the checking functions `linuxOvercommitMemoryValue()`,
        `THPIsEnabled()`, `linuxMadvFreeForkBugCheck()` out of _server.c_ and _latency.c_
        and into the new _syscheck.c_. When moving these functions I made sure they don't
        depend on other functionality provided in _server.c_ and made them use a standard
        "check functions" interface. Specifically:
        * I removed all logging out of `linuxMadvFreeForkBugCheck()`. In case there's some
          unexpected error during the check aborts as before, but without any logging.
          It returns an error code 0 meaning the check didn't not complete.
        * All these functions now return 1 on success, -1 on failure, 0 in case the check itself
          cannot be completed.
        * The `linuxMadvFreeForkBugCheck()` function now internally calls `exit()` and not
          `exitFromChild()` because the latter is only available in _server.c_ and I wanted to
          remove that dependency. This isn't an because we don't need to worry about the
          child process created by the test doing anything related to the rdb/aof files which
          is why `exitFromChild()` was created.
      
      * This also fixes parsing of other /proc/\<pid\>/stat fields to correctly handle spaces
        in the process name and be more robust in general. Not that before this fix the rss
        info in `INFO memory` was corrupt in case of spaces in the process name. To
        recreate just rename `redis-server` to `redis server`, start it, and run `INFO memory`.
      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
  19. 15 May, 2022 1 commit
  20. 13 May, 2022 1 commit
    • Wen Hui's avatar
      Update comments on command args, and a misleading error reply (#10645) · 135998ed
      Wen Hui authored
      Updated the comments for:
      info command
      lmpopCommand and blmpopCommand
      sinterGenericCommand 
      
      Fix the missing "key" words in the srandmemberCommand function
      For LPOS command, when rank is 0, prompt user that rank could be
      positive number or negative number, and add a test for it
      135998ed
  21. 11 May, 2022 1 commit
    • 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
  22. 10 May, 2022 1 commit
    • 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
  23. 28 Apr, 2022 1 commit
  24. 26 Apr, 2022 4 commits
    • chenyang8094's avatar
      Fix bug when AOF enabled after startup. put the new incr file in the manifest... · 46ec6ad9
      chenyang8094 authored
      Fix bug when AOF enabled after startup. put the new incr file in the manifest only when AOFRW is done. (#10616)
      
      Changes:
      
      - When AOF is enabled **after** startup, the data accumulated during `AOF_WAIT_REWRITE`
        will only be stored in a temp INCR AOF file. Only after the first AOFRW is successful, we will
        add it to manifest file.
        Before this fix, the manifest referred to the temp file which could cause a restart during that
        time to load it without it's base.
      - Add `aof_rewrites_consecutive_failures` info field for  aofrw limiting implementation.
      
      Now we can guarantee that these behaviors of MP-AOF are the same as before (past redis releases):
      - When AOF is enabled after startup, the data accumulated during `AOF_WAIT_REWRITE` will only
        be stored in a visible place. Only after the first AOFRW is successful, we will add it to manifest file.
      - When disable AOF, we did not delete the AOF file in the past so there's no need to change that
        behavior now (yet).
      - When toggling AOF off and then on (could be as part of a full-sync), a crash or restart before the
        first rewrite is completed, would result with the previous version being loaded (might not be right thing,
        but that's what we always had).
      46ec6ad9
    • Eduardo Semprebon's avatar
      Allow configuring signaled shutdown flags (#10594) · 3a1d1425
      Eduardo Semprebon authored
      
      
      The SHUTDOWN command has various flags to change it's default behavior,
      but in some cases establishing a connection to redis is complicated and it's easier
      for the management software to use signals. however, so far the signals could only
      trigger the default shutdown behavior.
      Here we introduce the option to control shutdown arguments for SIGTERM and SIGINT.
      
      New config options:
      `shutdown-on-sigint [nosave | save] [now] [force]` 
      `shutdown-on-sigterm [nosave | save] [now] [force]`
      
      Implementation:
      Support MULTI_ARG_CONFIG on createEnumConfig to support multiple enums to be applied as bit flags.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      3a1d1425
    • Madelyn Olson's avatar
      Set replicas to panic on disk errors, and optionally panic on replication errors (#10504) · 6fa8e4f7
      Madelyn Olson authored
      * Till now, replicas that were unable to persist, would still execute the commands
        they got from the master, now they'll panic by default, and we add a new
        `replica-ignore-disk-errors` config to change that.
      * Till now, when a command failed on a replica or AOF-loading, it only logged a
        warning and a stat, we add a new `propagation-error-behavior` config to allow
        panicking in that state (may become the default one day)
      
      Note that commands that fail on the replica can either indicate a bug that could
      cause data inconsistency between the replica and the master, or they could be
      in some cases (specifically in previous versions), a result of a command (e.g. EVAL)
      that failed on the master, but still had to be propagated to fail on the replica as well.
      6fa8e4f7
    • Madelyn Olson's avatar
      By default prevent cross slot operations in functions and scripts with # (#10615) · efcd1bf3
      Madelyn Olson authored
      Adds the `allow-cross-slot-keys` flag to Eval scripts and Functions to allow
      scripts to access keys from multiple slots.
      The default behavior is now that they are not allowed to do that (unlike before).
      This is a breaking change for 7.0 release candidates (to be part of 7.0.0), but
      not for previous redis releases since EVAL without shebang isn't doing this check.
      
      Note that the check is done on both the keys declared by the EVAL / FCALL command
      arguments, and also the ones used by the script when making a `redis.call`.
      
      A note about the implementation, there seems to have been some confusion
      about allowing access to non local keys. I thought I missed something in our
      wider conversation, but Redis scripts do block access to non-local keys.
      So the issue was just about cross slots being accessed.
      efcd1bf3
  25. 25 Apr, 2022 3 commits
  26. 20 Apr, 2022 3 commits
    • filipe oliveira's avatar
      Optimization: Use either monotonic or wall-clock to measure command execution... · 3cd8baf6
      filipe oliveira authored
      
      Optimization: Use either monotonic or wall-clock to measure command execution time, to regain up to 4% execution time (#10502)
      
      In #7491 (part of redis 6.2), we started using the monotonic timer instead of mstime to measure
      command execution time for stats, apparently this meant sampling the clock 3 times per command
      rather than two (wince we also need the wall-clock time).
      In some cases this causes a significant overhead.
      
      This PR fixes that by avoiding the use of monotonic timer, except for the cases were we know it
      should be extremely fast.
      This PR also adds a new INFO field called `monotonic_clock` that shows which clock redis is using.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      3cd8baf6
    • Oran Agra's avatar
      Fixes around clients that must be obeyed. Replica report disk errors in PING. (#10603) · ee220599
      Oran Agra authored
      This PR unifies all the places that test if the current client is the
      master client or AOF client, and uses a method to test that on all of
      these.
      
      Other than some refactoring, these are the actual implications:
      - Replicas **don't** ignore disk error when processing commands not
        coming from their master.
        **This is important for PING to be used for health check of replicas**
      - SETRANGE, APPEND, SETBIT, BITFIELD don't do proto_max_bulk_len check for AOF
      - RM_Call in SCRIPT_MODE ignores disk error when coming from master /
        AOF
      - RM_Call in cluster mode ignores slot check when processing AOF
      - Scripts ignore disk error when processing AOF
      - Scripts **don't** ignore disk error on a replica, if the command comes
        from clients other than the master
      - SCRIPT KILL won't kill script coming from AOF
      - Scripts **don't** skip OOM check on replica if the command comes from
        clients other than the master
      
      Note that Script, AOF, and module clients don't reach processCommand,
      which is why some of the changes don't actually have any implications.
      
      Note, reverting the change done to processCommand in 2f4240b9
      should be dead code due to the above mentioned fact.
      ee220599
    • yoav-steinberg's avatar
      Optimized `hdr_value_at_percentile` (#10606) · 5075e743
      yoav-steinberg authored
      `hdr_value_at_percentile()` is part of the Hdr_Histogram library
      used when generating `latencystats` report. 
      
      There's a pending optimization for this function which greatly
      affects the performance of `info latencystats`.
      https://github.com/HdrHistogram/HdrHistogram_c/pull/107
      
      
      
      This PR:
      1. Upgrades the sources in _deps/hdr_histogram_ to the latest Hdr_Histogram
        version 0.11.5
      2. Applies the referenced optimization.
      3. Adds minor documentation about the hdr_histogram dependency which was
        missing under _deps/README.md_.
      
      benchmark on my machine:
      running: `redis-benchmark -n 100000 info latencystats` on a clean build with no data.
      
      | benchmark | RPS |
      | ---- | ---- |
      | before upgrade to v0.11.05  | 7,681 |
      | before optimization | 12,474 |
      | after optimization | 52,606 |
      Co-authored-by: default avatarfilipe oliveira <filipecosta.90@gmail.com>
      5075e743