1. 28 Mar, 2022 1 commit
  2. 25 Mar, 2022 1 commit
    • zhaozhao.zz's avatar
      optimize(remove) usage of client's pending_querybuf (#10413) · 78bef6e1
      zhaozhao.zz authored
      To remove `pending_querybuf`, the key point is reusing `querybuf`, it means master client's `querybuf` is not only used to parse command, but also proxy to sub-replicas.
      
      1. add a new variable `repl_applied` for master client to record how many data applied (propagated via `replicationFeedStreamFromMasterStream()`) but not trimmed in `querybuf`.
      
      2. don't sdsrange `querybuf` in `commandProcessed()`, we trim it to `repl_applied` after the whole replication pipeline processed to avoid fragmented `sdsrange`. And here are some scenarios we cannot trim to `qb_pos`:
          * we don't receive complete command from master
          * master client blocked because of client pause
          * IO threads operate read, master client flagged with CLIENT_PENDING_COMMAND
      
          In these scenarios, `qb_pos` points to the part of the current command or the beginning of next command, and the current command is not applied yet, so the `repl_applied` is not equal to `qb_pos`.
      
      Some other notes:
      * Do not do big arg optimization on master client, since we can only sdsrange `querybuf` after data sent to replicas.
      * Set `qb_pos` and `repl_applied` to 0 when `freeClient` in `replicationCacheMaster`.
      * Rewrite `processPendingCommandsAndResetClient` to `processPendingCommandAndInputBuffer`, let `processInputBuffer` to be called successively after `processCommandAndResetClient`.
      78bef6e1
  3. 22 Mar, 2022 4 commits
    • Oran Agra's avatar
      crash log, print killer pid only when si_code is SI_USER (#10454) · 6761d10c
      Oran Agra authored
      Avoid printing "Killed by PID" when si_code != SI_USER.
      Apparently SI_USER isn't always set to 0. e.g. on Mac it's 0x10001 and the check that did <= was wrong.
      6761d10c
    • Meir Shpilraien (Spielrein)'s avatar
      Add new RM_Call flags for script mode, no writes, and error replies. (#10372) · f3855a09
      Meir Shpilraien (Spielrein) authored
      The PR extends RM_Call with 3 new capabilities using new flags that
      are given to RM_Call as part of the `fmt` argument.
      It aims to assist modules that are getting a list of commands to be
      executed from the user (not hard coded as part of the module logic),
      think of a module that implements a new scripting language...
      
      * `S` - Run the command in a script mode, this means that it will raise an
        error if a command which are not allowed inside a script (flaged with the
        `deny-script` flag) is invoked (like SHUTDOWN). In addition, on script mode,
        write commands are not allowed if there is not enough good replicas (as
        configured with `min-replicas-to-write`) and/or a disk error happened.
      
      * `W` - no writes mode, Redis will reject any command that is marked with `write`
        flag. Again can be useful to modules that implement a new scripting language
        and wants to prevent any write commands.
      
      * `E` - Return errors as RedisModuleCallReply. Today the errors that happened
        before the command was invoked (like unknown commands or acl error) return
        a NULL reply and set errno. This might be missing important information about
        the failure and it is also impossible to just pass the error to the user using
        RM_ReplyWithCallReply. This new flag allows you to get a RedisModuleCallReply
        object with the relevant error message and treat it as if it was an error that was
        raised by the command invocation.
      
      Tests were added to verify the new code paths.
      
      In addition small refactoring was done to share some code between modules,
      scripts, and `processCommand` function:
      1. `getAclErrorMessage` was added to `acl.c` to unified to log message extraction
        from the acl result
      2. `checkGoodReplicasStatus` was added to `replication.c` to check the status of
        good replicas. It is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and
        `processCommand`.
      3. `writeCommandsGetDiskErrorMessage` was added to `server.c` to get the error
        message on persistence failure. Again it is used on `scriptVerifyWriteCommandAllow`,
        `RM_Call`, and `processCommand`.
      f3855a09
    • yiyuaner's avatar
      Fix an off by one error in zzlStrtod (#10465) · 08aed7e7
      yiyuaner authored
      When vlen = sizeof(buf), the statement buf[vlen] = '\0' accessing the buffer buf is an off by one error.
      08aed7e7
    • zhaozhao.zz's avatar
      config rewrite enhancement, in case of line longer than 1024 (#8009) · 79db037a
      zhaozhao.zz authored
      When rewrite the config file, we need read the old config file first,
      but the CONFIG_MAX_LEN is 1024, so if some lines are longer than it,
      it will generate a wrong config file, and redis cannot reboot from
      the new config file.
      
      Rename CONFIG_MAX_LINE to CONFIG_READ_LEN
      79db037a
  4. 21 Mar, 2022 4 commits
  5. 20 Mar, 2022 1 commit
    • 郭伟光's avatar
      unblockClient: avoid to reset client when the client was shutdown-blocked (#10440) · fae5b1a1
      郭伟光 authored
      fix #10439. see https://github.com/redis/redis/pull/9872
      When executing SHUTDOWN we pause the client so we can un-pause it
      if the shutdown fails.
      this could happen during the timeout, if the shutdown is aborted, but could
      also happen from withing the initial `call()` to shutdown, if the rdb save fails.
      in that case when we return to `call()`, we'll crash if `c->cmd` has been set to NULL.
      
      The call stack is:
      ```
      unblockClient(c)
      replyToClientsBlockedOnShutdown()
      cancelShutdown()
      finishShutdown()
      prepareForShutdown()
      shutdownCommand()
      ```
      
      what's special about SHUTDOWN in that respect is that it can be paused,
      and then un-paused before the original `call()` returns.
      tests where added for both failed shutdown, and a followup successful one.
      fae5b1a1
  6. 17 Mar, 2022 1 commit
  7. 16 Mar, 2022 4 commits
  8. 15 Mar, 2022 3 commits
    • Wen Hui's avatar
      Sentinel: update command json files (#10374) · c30de707
      Wen Hui authored
      c30de707
    • ranshid's avatar
      make sort/ro commands validate external keys access patterns (#10106) (#10340) · 1078e30c
      ranshid authored
      Currently the sort and sort_ro can access external keys via `GET` and `BY`
      in order to make sure the user cannot violate the authorization ACL
      rules, the decision is to reject external keys access patterns unless ACL allows
      SORT full access to all keys.
      I.e. for backwards compatibility, SORT with GET/BY keeps working, but
      if ACL has restrictions to certain keys, these features get permission denied.
      
      ### Implemented solution
      We have discussed several potential solutions and decided to only allow the GET and BY
      arguments when the user has all key permissions with the SORT command. The reasons
      being that SORT with GET or BY is problematic anyway, for instance it is not supported in
      cluster mode since it doesn't declare keys, and we're not sure the combination of that feature
      with ACL key restriction is really required.
      **HOWEVER** If in the fullness of time we will identify a real need for fine grain access
      support ...
      1078e30c
    • yoav-steinberg's avatar
      Optimization: remove `updateClientMemUsage` from i/o threads. (#10401) · cf6dcb7b
      yoav-steinberg authored
      In a benchmark we noticed we spend a relatively long time updating the client
      memory usage leading to performance degradation.
      Before #8687 this was performed in the client's cron and didn't affect performance.
      But since introducing client eviction we need to perform this after filling the input
      buffers and after processing commands. This also lead me to write this code to be
      thread safe and perform it in the i/o threads.
      
      It turns out that the main performance issue here is related to atomic operations
      being performed while updating the total clients memory usage stats used for client
      eviction (`server.stat_clients_type_memory[]`). This update needed to be atomic
      because `updateClientMemUsage()` was called from the IO threads.
      
      In this commit I make sure to call `updateClientMemUsage()` only from the main thread.
      In case of threaded IO I call it for each client during the "fan-in" phase of the read/write
      operation. This also means I could chuck the `updateClientMemUsageBucket()` function
      which was called during this phase and embed it into `updateClientMemUsage()`.
      
      Profiling shows this makes `updateClientMemUsage()` (on my x86_64 linux) roughly x4 faster.
      cf6dcb7b
  9. 14 Mar, 2022 1 commit
  10. 13 Mar, 2022 3 commits
  11. 10 Mar, 2022 4 commits
    • Binbin's avatar
      Initialize help when using redis-cli help or redis-cli ? (#10382) · 1797330e
      Binbin authored
      The following usage will output an empty newline:
      ```
      > redis-cli help set
      empty line
      ```
      
      The reason is that in interactive mode, we have called
      `cliInitHelp`, which initializes help.
      
      When using `redis-cli help xxx` or `redis-cli help ? xxx`,
      we can't match the command due to empty `helpEntries`,
      so we output an empty newline.
      
      In this commit, we will call `cliInitHelp` to init the help.
      Note that in this case, we need to call `cliInitHelp` (COMMAND DOCS)
      every time, which i think is acceptable.
      
      So now the output will look like:
      ```
      [redis]# src/redis-cli help get
      
        GET key
        summary: Get the value of a key
        since: 1.0.0
        group: string
      
      [redis]#
      ```
      
      Fixes #10378
      
      This PR also fix a redis-cli crash when using `--ldb --eval`:
      ```
      [root]# src/redis-cli --ldb --eval test.lua test 1
      Lua debugging session started, please use:
      quit    -- End the session.
      restart -- Restart the script in debug mode again.
      help    -- Show Lua script debugging commands.
      
      * Stopped at 1, stop reason = step over
      -> 1   local num = redis.call('GET', KEYS[1]);
      redis-cli: redis-cli.c:718: cliCountCommands: Assertion
      `commandTable->element[i]->type == 1' failed.
      Aborted
      ```
      Because in ldb mode, `COMMAND DOCS` or `COMMAND` will
      return an array, only with one element, and the type
      is `REDIS_REPLY_STATUS`, the result is `<error> Unknown
      Redis Lua debugger command or wrong number of arguments`.
      
      So if we are in the ldb mode, and init the Redis HELP, we
      will get the wrong response and crash the redis-cli.
      In ldb mode we don't initialize HELP, help is only initialized
      after the lua debugging session ends.
      
      It was broken in #10043
      1797330e
    • ranshid's avatar
      ACL DRYRUN does not validate the verified command args. (#10405) · 11b071a2
      ranshid authored
      As a result we segfault when parsing and matching the command keys.
      11b071a2
    • zhugezy's avatar
      set "disable-thp" config immutable (#10409) · a26cab9d
      zhugezy authored
      It's confusing for this config to be modifiable since it only takes effect on startup
      a26cab9d
    • rangerzhang's avatar
      Fix outdated comments on updateSlavesWaitingBgsave (#10394) · 4e012dae
      rangerzhang authored
      * fix-replication-comments
      
      The described capacity
       `and to schedule a new BGSAVE if there are slaves that attached while a BGSAVE was in progress`
      was moved to `checkChildrenDone()`  named by `replicationStartPendingFork`
      
      But the comment was not changed, may misleading others.
      
      * remove-misleading-comments
      
      The described capacity
       `to schedule a new BGSAVE if there are slaves that attached while a BGSAVE was in progress` 
      and 
      `or when the replication RDB transfer strategy is modified from disk to socket or the other way around` 
      were not correct now.
      4e012dae
  12. 09 Mar, 2022 4 commits
  13. 08 Mar, 2022 5 commits
    • guybe7's avatar
      XREADGROUP: Unblock client if stream is deleted (#10306) · 2a295408
      guybe7 authored
      Deleting a stream while a client is blocked XREADGROUP should unblock the client.
      
      The idea is that if a client is blocked via XREADGROUP is different from
      any other blocking type in the sense that it depends on the existence of both
      the key and the group. Even if the key is deleted and then revived with XADD
      it won't help any clients blocked on XREADGROUP because the group no longer
      exist, so they would fail with -NOGROUP anyway.
      The conclusion is that it's better to unblock these clients (with error) upon
      the deletion of the key, rather than waiting for the first XADD. 
      
      Other changes:
      1. Slightly optimize all `serveClientsBlockedOn*` functions by checking `server.blocked_clients_by_type`
      2. All `serveClientsBlockedOn*` functions now use a list iterator rather than looking at `listFirst`, relying
        on `unblockClient` to delete the head of the list. Before this commit, only `serveClientsBlockedOnStreams`
        used to work like that.
      3. bugfix: CLIENT UNBLOCK ERROR should work even if the command doesn't have a timeout_callback
        (only relevant to module commands)
      2a295408
    • zhaozhao.zz's avatar
      script should not allow may-replicate commands when client pause write (#10364) · 728e6252
      zhaozhao.zz authored
      In some special commands like eval_ro / fcall_ro we allow no-writes commands.
      But may-replicate commands are no-writes too, that leads crash when client pause write:
      728e6252
    • Oran Agra's avatar
      dismiss COW of client output buffer now that it's dynamic (#10371) · b3fe4f31
      Oran Agra authored
      since #9822, the static reply buffer is no longer part of the client structure, so we need to dismiss it.
      b3fe4f31
    • zhugezy's avatar
      remove a piece of redundant comment (#10392) · 4f19b4d0
      zhugezy authored
      introduced in #10147 since we blocked the first-arg mechanism on subcommands
      4f19b4d0
    • Yossi Gottlieb's avatar
      Fix redis-benchmark --cluster with IPv6. (#10393) · 38052fd7
      Yossi Gottlieb authored
      Currently, CLUSTER NODES is parsed and was not done correctly for IPv6
      addresses.
      38052fd7
  14. 07 Mar, 2022 2 commits
  15. 05 Mar, 2022 1 commit
    • Yuta Hongo's avatar
      redis-cli: Better --json Unicode support and --quoted-json (#10286) · e3ef73dc
      Yuta Hongo authored
      Normally, `redis-cli` escapes non-printable data received from Redis, using a custom scheme (which is also used to handle quoted input). When using `--json` this is not desired as it is not compatible with RFC 7159, which specifies JSON strings are assumed to be Unicode and how they should be escaped.
      
      This commit changes `--json` to follow RFC 7159, which means that properly encoded Unicode strings in Redis will result with a valid Unicode JSON.
      
      However, this introduces a new problem with `--json` and data that is not valid Unicode (e.g., random binary data, text that follows other encoding, etc.). To address this, we add `--quoted-json` which produces JSON strings that follow the original redis-cli quoting scheme.
      
      For example, a value that consists of only null (0x00) bytes will show up as:
      * `"\u0000\u0000\u0000"` when using `--json`
      * `"\\x00\\x00\\x00"` when using `--quoted-json`
      e3ef73dc
  16. 03 Mar, 2022 1 commit