1. 02 Apr, 2022 1 commit
    • Viktor Söderqvist's avatar
      Turn into replica on SETSLOT (#10489) · b53c7f2c
      Viktor Söderqvist authored
      * Fix race condition where node loses its last slot and turns into replica
      
      When a node has lost its last slot and finds out from the SETSLOT command
      before the cluster bus PONG from the new owner arrives. In this case, the
      node didn't turn itself into a replica of the new slot owner.
      
      This commit adds the same logic to the SETSLOT command as already exists
      for the cluster bus PONG processing.
      
      * Revert "Fix new / failing cluster slot migration test (#10482)"
      
      This reverts commit 0b21ef8d.
      
      In this test, the old slot owner finds out that it has lost its last
      slot in a nondeterministic way. Either the cluster bus PONG from the
      new slot owner and sometimes in a SETSLOT command from redis-cli. In
      both cases, the result should be the same and the old owner should
      turn itself into a replica of the new slot owner.
      b53c7f2c
  2. 31 Mar, 2022 3 commits
    • sundb's avatar
      Fix failing moduleconfigs tests and memory leak (#10501) · b8eb2a73
      sundb authored
      Fix global `strval` not reset to NULL after being freed, causing a crash on alpine
      (most likely because the dynamic library loader doesn't init globals on reload)
      By the way, fix the memory leak of using `RedisModule_Free` to free `RedisModuleString`,
      and add a corresponding test.
      b8eb2a73
    • Madelyn Olson's avatar
      Prevent replica failover during manual takeover test (#10499) · e81bd15e
      Madelyn Olson authored
      During 11-manual-takeover.tcl, if the killing of the instances happens
      too slowly, one of the replicas might be able to promote itself.
      I'm not sure why it was slow, but it was observed taking 6 seconds
      which is enough time to do an election.
      I was able to verify the error locally by adding a small delay (1 second)
      during ASAN CI. A fix is just to disable automated failover until all the
      nodes are confirmed dead.
      e81bd15e
    • Binbin's avatar
      Fix cluster slot migration test (#10495) · a3075ca4
      Binbin authored
      Fix three timing issues in the test
      a3075ca4
  3. 30 Mar, 2022 2 commits
    • Binbin's avatar
      Move restart_killed_instances and verify_sentinel_auto_discovery to utils (#10497) · 6075f506
      Binbin authored
      Create a utils.tcl in sentinel/tests/includes, and move two procs to it.
      Allow sentinel test 08-hostname-conf run on its own.
      6075f506
    • Nick Chun's avatar
      Module Configurations (#10285) · bda9d74d
      Nick Chun authored
      
      
      This feature adds the ability to add four different types (Bool, Numeric,
      String, Enum) of configurations to a module to be accessed via the redis
      config file, and the CONFIG command.
      
      **Configuration Names**:
      
      We impose a restriction that a module configuration always starts with the
      module name and contains a '.' followed by the config name. If a module passes
      "config1" as the name to a register function, it will be registered as MODULENAME.config1.
      
      **Configuration Persistence**:
      
      Module Configurations exist only as long as a module is loaded. If a module is
      unloaded, the configurations are removed.
      There is now also a minimal core API for removal of standardConfig objects
      from configs by name.
      
      **Get and Set Callbacks**:
      
      Storage of config values is owned by the module that registers them, and provides
      callbacks for Redis to access and manipulate the values.
      This is exposed through a GET and SET callback.
      
      The get callback returns a typed value of the config to redis. The callback takes
      the name of the configuration, and also a privdata pointer. Note that these only
      take the CONFIGNAME portion of the config, not the entire MODULENAME.CONFIGNAME.
      
      ```
       typedef RedisModuleString * (*RedisModuleConfigGetStringFunc)(const char *name, void *privdata);
       typedef long long (*RedisModuleConfigGetNumericFunc)(const char *name, void *privdata);
       typedef int (*RedisModuleConfigGetBoolFunc)(const char *name, void *privdata);
       typedef int (*RedisModuleConfigGetEnumFunc)(const char *name, void *privdata);
      ```
      
      Configs must also must specify a set callback, i.e. what to do on a CONFIG SET XYZ 123
      or when loading configurations from cli/.conf file matching these typedefs. *name* is
      again just the CONFIGNAME portion, *val* is the parsed value from the core,
      *privdata* is the registration time privdata pointer, and *err* is for providing errors to a client.
      
      ```
      typedef int (*RedisModuleConfigSetStringFunc)(const char *name, RedisModuleString *val, void *privdata, RedisModuleString **err);
      typedef int (*RedisModuleConfigSetNumericFunc)(const char *name, long long val, void *privdata, RedisModuleString **err);
      typedef int (*RedisModuleConfigSetBoolFunc)(const char *name, int val, void *privdata, RedisModuleString **err);
      typedef int (*RedisModuleConfigSetEnumFunc)(const char *name, int val, void *privdata, RedisModuleString **err);
      ```
      
      Modules can also specify an optional apply callback that will be called after
      value(s) have been set via CONFIG SET:
      
      ```
      typedef int (*RedisModuleConfigApplyFunc)(RedisModuleCtx *ctx, void *privdata, RedisModuleString **err);
      ```
      
      **Flags:**
      We expose 7 new flags to the module, which are used as part of the config registration.
      
      ```
      #define REDISMODULE_CONFIG_MODIFIABLE 0 /* This is the default for a module config. */
      #define REDISMODULE_CONFIG_IMMUTABLE (1ULL<<0) /* Can this value only be set at startup? */
      #define REDISMODULE_CONFIG_SENSITIVE (1ULL<<1) /* Does this value contain sensitive information */
      #define REDISMODULE_CONFIG_HIDDEN (1ULL<<4) /* This config is hidden in `config get <pattern>` (used for tests/debugging) */
      #define REDISMODULE_CONFIG_PROTECTED (1ULL<<5) /* Becomes immutable if enable-protected-configs is enabled. */
      #define REDISMODULE_CONFIG_DENY_LOADING (1ULL<<6) /* This config is forbidden during loading. */
      /* Numeric Specific Configs */
      #define REDISMODULE_CONFIG_MEMORY (1ULL<<7) /* Indicates if this value can be set as a memory value */
      ```
      
      **Module Registration APIs**:
      
      ```
      int (*RedisModule_RegisterBoolConfig)(RedisModuleCtx *ctx, char *name, int default_val, unsigned int flags, RedisModuleConfigGetBoolFunc getfn, RedisModuleConfigSetBoolFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata);
      int (*RedisModule_RegisterNumericConfig)(RedisModuleCtx *ctx, const char *name, long long default_val, unsigned int flags, long long min, long long max, RedisModuleConfigGetNumericFunc getfn, RedisModuleConfigSetNumericFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata);
      int (*RedisModule_RegisterStringConfig)(RedisModuleCtx *ctx, const char *name, const char *default_val, unsigned int flags, RedisModuleConfigGetStringFunc getfn, RedisModuleConfigSetStringFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata);
      int (*RedisModule_RegisterEnumConfig)(RedisModuleCtx *ctx, const char *name, int default_val, unsigned int flags, const char **enum_values, const int *int_values, int num_enum_vals, RedisModuleConfigGetEnumFunc getfn, RedisModuleConfigSetEnumFunc setfn, RedisModuleConfigApplyFunc applyfn, void *privdata);
      int (*RedisModule_LoadConfigs)(RedisModuleCtx *ctx);
      ```
      
      The module name will be auto appended along with a "." to the front of the name of the config.
      
      **What RM_Register[...]Config does**:
      
      A RedisModule struct now keeps a list of ModuleConfig objects which look like:
      ```
      typedef struct ModuleConfig {
          sds name; /* Name of config without the module name appended to the front */
          void *privdata; /* Optional data passed into the module config callbacks */
          union get_fn { /* The get callback specificed by the module */
              RedisModuleConfigGetStringFunc get_string;
              RedisModuleConfigGetNumericFunc get_numeric;
              RedisModuleConfigGetBoolFunc get_bool;
              RedisModuleConfigGetEnumFunc get_enum;
          } get_fn;
          union set_fn { /* The set callback specified by the module */
              RedisModuleConfigSetStringFunc set_string;
              RedisModuleConfigSetNumericFunc set_numeric;
              RedisModuleConfigSetBoolFunc set_bool;
              RedisModuleConfigSetEnumFunc set_enum;
          } set_fn;
          RedisModuleConfigApplyFunc apply_fn;
          RedisModule *module;
      } ModuleConfig;
      ```
      It also registers a standardConfig in the configs array, with a pointer to the
      ModuleConfig object associated with it.
      
      **What happens on a CONFIG GET/SET MODULENAME.MODULECONFIG:**
      
      For CONFIG SET, we do the same parsing as is done in config.c and pass that
      as the argument to the module set callback. For CONFIG GET, we call the
      module get callback and return that value to config.c to return to a client.
      
      **CONFIG REWRITE**:
      
      Starting up a server with module configurations in a .conf file but no module load
      directive will fail. The flip side is also true, specifying a module load and a bunch
      of module configurations will load those configurations in using the module defined
      set callbacks on a RM_LoadConfigs call. Configs being rewritten works the same
      way as it does for standard configs, as the module has the ability to specify a
      default value. If a module is unloaded with configurations specified in the .conf file
      those configurations will be commented out from the .conf file on the next config rewrite.
      
      **RM_LoadConfigs:**
      
      `RedisModule_LoadConfigs(RedisModuleCtx *ctx);`
      
      This last API is used to make configs available within the onLoad() after they have
      been registered. The expected usage is that a module will register all of its configs,
      then call LoadConfigs to trigger all of the set callbacks, and then can error out if any
      of them were malformed. LoadConfigs will attempt to set all configs registered to
      either a .conf file argument/loadex argument or their default value if an argument is
      not specified. **LoadConfigs is a required function if configs are registered.
      ** Also note that LoadConfigs **does not** call the apply callbacks, but a module
      can do that directly after the LoadConfigs call.
      
      **New Command: MODULE LOADEX [CONFIG NAME VALUE] [ARGS ...]:**
      
      This command provides the ability to provide startup context information to a module.
      LOADEX stands for "load extended" similar to GETEX. Note that provided config
      names need the full MODULENAME.MODULECONFIG name. Any additional
      arguments a module might want are intended to be specified after ARGS.
      Everything after ARGS is passed to onLoad as RedisModuleString **argv.
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      Co-authored-by: default avatarMadelyn Olson <matolson@amazon.com>
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      Co-authored-by: default avatarMadelyn Olson <34459052+madolson@users.noreply.github.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarYossi Gottlieb <yossigo@gmail.com>
      bda9d74d
  4. 28 Mar, 2022 2 commits
    • Oran Agra's avatar
      introduce MAX_D2STRING_CHARS instead of 128 const (#10487) · 14b19886
      Oran Agra authored
      There are a few places that use a hard coded const of 128 to allocate a buffer for d2string.
      Replace these with a clear macro.
      Note that In theory, converting double into string could take as much as nearly 400 chars,
      but since d2string uses `%g` and not `%f`, it won't pass some 40 chars.
      
      unrelated:
      restore some changes to auto generated commands.c that got accidentally reverted in #10293
      14b19886
    • Moti Cohen's avatar
      Fix sentinel test SDOWN is triggered by non-responding instance (#10484) · 63f77698
      Moti Cohen authored
      A timing issue of debug sleep master isn't long enough to ensure
      that master is down and let the test identify it. Replaced the code
      with suspend PID until verified master-is-down.
      63f77698
  5. 27 Mar, 2022 2 commits
    • Oran Agra's avatar
      Fix new / failing cluster slot migration test (#10482) · 0b21ef8d
      Oran Agra authored
      #10381 fixed an issue in `redis-cli --cluster reshard` that used to fail it (redis-cli) because
      of a race condition.
      the race condition is / was that when moving the last slot from a node, sometimes the PONG
      messages delivering the configuration change arrive to that node before the SETSLOT arrives
      to it, and it becomes a replica.
      other times the the SETSLOT arrive first, and then PONG **doesn't** demote it.
      
      **however**, the PR also added a new test that suffers from exactly the same race condition,
      and the tests started failing a lot.
      
      The fact is (if i understand it correctly), that this test (the one being deleted here), isn't related
      to the fix that PR fixed (which was to fix redis-cli).
      The race condition in the cluster code still happens, and as long as we don't solve it, there's
      no reason to test it.
      
      For now, even if my understandings are wrong, i'm gonna delete that failing test, since as far as
      i understand, #10381 didn't introduce any new risks for that matter (which are gonna be
      compromised by removing this check), this race existed since forever, and still exists, and the
      fact that redis-cli is now immune to it is still being tested.
      
      Additional work should be carried to fix it, and i live it for other PRs to handle.
      0b21ef8d
    • Moti Cohen's avatar
      Fix Sentinel reconnect test following ACL change (#10480) · 37d761ba
      Moti Cohen authored
      Replace condition with wait_for_condition On "Verify sentinel that restarted 
      failed to reconnect master after ACL change"
      
      The reason we reach it, is because the test is fast enough to modify ACL and
      test sentinel connection status with the server - before its scheduled operation
      got the chance to update connection status with the server:
      ```
      /* Perform scheduled operations for the specified Redis instance. */
      void sentinelHandleRedisInstance(sentinelRedisInstance *ri) {
          /* ========== MONITORING HALF ============ */
          /* Every kind of instance */
          sentinelReconnectInstance(ri);
      ```
      37d761ba
  6. 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
  7. 22 Mar, 2022 1 commit
    • 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
  8. 21 Mar, 2022 3 commits
  9. 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
  10. 18 Mar, 2022 1 commit
    • sundb's avatar
      Restore ::singledb after cluster test (#10441) · b9656adb
      sundb authored
      When ::singledb is 0, we will use db 9 for the test db.
      Since ::singledb is set to 1 in the cluster-related tests, but not restored, some subsequent
      tests associated with db 9 will fail.
      b9656adb
  11. 16 Mar, 2022 5 commits
    • Madelyn Olson's avatar
    • Viktor Söderqvist's avatar
      Fix redis-cli CLUSTER SETSLOT race conditions (#10381) · 69017fa2
      Viktor Söderqvist authored
      After migrating a slot, send CLUSTER SETSLOT NODE to the destination
      node first to make sure the slot isn't left without an owner in case
      the destination node crashes before it is set as new owner.
      
      When informing the source node, it can happen that the destination
      node has already informed it and if the source node has lost its
      last slot, it has already turned itself into a replica. Redis-cli
      should ignore this error in this case.
      69017fa2
    • Binbin's avatar
      Fix module redact test for valgrind (#10432) · 61b7e591
      Binbin authored
      The new module redact test will fail with valgrind:
      ```
      [err]: modules can redact arguments in tests/unit/moduleapi/auth.tcl
      Expected 'slowlog reset' to be equal to 'auth.redact 1 (redacted) 3 (redacted)' (context: type eval line 12 cmd {assert_equal {slowlog reset} [lindex [lindex [r slowlog get] 2] 3]} proc ::test)
      ```
      
      The reason is that with `slowlog-log-slower-than 10000`,
      `slowlog get` will have a chance to exceed 10ms.
      
      Made two changes to avoid failure:
      1. change `slowlog-log-slower-than` from 10000 to -1, distable it.
      2. assert to use the previous execution result.
      
      In theory, the second one can actually be left unchanged, but i
      think it will be better if it is changed.
      61b7e591
    • Harkrishn Patro's avatar
      Add new cluster shards command (#10293) · 45ccae89
      Harkrishn Patro authored
      
      
      Implement a new cluster shards command, which provides a flexible and extensible API for topology discovery.
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      45ccae89
    • Madelyn Olson's avatar
      Add module API for redacting command arguments (#10425) · 416c9ac2
      Madelyn Olson authored
      Add module API for redacting client commands
      416c9ac2
  12. 15 Mar, 2022 1 commit
    • 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
  13. 14 Mar, 2022 1 commit
    • Binbin's avatar
      Sentinel: fix reconnect test timing issue (#10424) · 871fa12f
      Binbin authored
      We need to wait for `sentinelTimer` to kick in, and then trigger the reconnect.
      
      As for another change, we should better call `server_set_password` before calling SENTINEL SET auth-pass.
      
      Fixes problem introeuced in #10400
      871fa12f
  14. 13 Mar, 2022 1 commit
    • Moti Cohen's avatar
      Sentinel: fix no reconnect after auth-pass is changed (#10400) · a6bf5098
      Moti Cohen authored
      When updating SENTINEL with master’s new password (command:
      `SENTINEL SET mymaster auth-pass some-new-password`), 
      sentinel might still keep the old connection and avoid reconnecting 
      with the new password. This is because of wrong logic that traces 
      the last ping (pong) time to servers. In fact it worked fine until 8631e647 
      changed the condition to send ping. To resolve it with minimal risk, 
      let’s disconnect master and replicas once changing password/user. 
      
      Based on earlier work of yz1509.
      a6bf5098
  15. 10 Mar, 2022 2 commits
  16. 09 Mar, 2022 1 commit
  17. 08 Mar, 2022 2 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
  18. 07 Mar, 2022 2 commits
    • Shaya Potter's avatar
      Modules: Add REDISMODULE_EVENT_CONFIG (#10311) · 23f03e79
      Shaya Potter authored
      Add a new REDISMODULE_EVENT_CONFIG event type for notifying modules when Redis configuration changes.
      23f03e79
    • Binbin's avatar
      Fix timing issue in rehash test (#10388) · 45d83fb2
      Binbin authored
      `Expected '*table size: 4096*' to match '*table size: 8192*'`
      
      This test failed once on daily macOS, the reason is because
      the bgsave has not stopped after the kill and `after 200`.
      So there is a child process and no rehash triggered.
      
      This commit use `waitForBgsave` to wait for it to finish.
      45d83fb2
  19. 06 Mar, 2022 1 commit
  20. 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
  21. 01 Mar, 2022 2 commits
    • ranshid's avatar
      Introduce debug command to disable reply buffer resizing (#10360) · 9b15dd28
      ranshid authored
      In order to resolve some flaky tests which hard rely on examine memory footprint.
      we introduce the following fixes:
      
      # Fix in client-eviction test - by @yoav-steinberg 
      Sometime the libc allocator can use different size client struct allocations.
      this may cause unexpected memory calculations to fail the test.
      
      # Introduce new DEBUG command for disabling reply buffer resizing
      In order to eliminate reply buffer resizing during specific tests.
      we introduced the ability to disable (and enable) the resizing cron job
      
      Co-authored-by: yoav-steinberg yoav@redislabs.com
      9b15dd28
    • Harkrishn Patro's avatar
  22. 28 Feb, 2022 1 commit
  23. 27 Feb, 2022 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Sort out the mess around Lua error messages and error stats (#10329) · aa856b39
      Meir Shpilraien (Spielrein) authored
      
      
      This PR fix 2 issues on Lua scripting:
      * Server error reply statistics (some errors were counted twice).
      * Error code and error strings returning from scripts (error code was missing / misplaced).
      
      ## Statistics
      a Lua script user is considered part of the user application, a sophisticated transaction,
      so we want to count an error even if handled silently by the script, but when it is
      propagated outwards from the script we don't wanna count it twice. on the other hand,
      if the script decides to throw an error on its own (using `redis.error_reply`), we wanna
      count that too.
      Besides, we do count the `calls` in command statistics for the commands the script calls,
      we we should certainly also count `failed_calls`.
      So when a simple `eval "return redis.call('set','x','y')" 0` fails, it should count the failed call
      to both SET and EVAL, but the `errorstats` and `total_error_replies` should be counted only once.
      
      The PR changes the error object that is raised on errors. Instead of raising a simple Lua
      string, Redis will raise a Lua table in the following format:
      
      ```
      {
          err='<error message (including error code)>',
          source='<User source file name>',
          line='<line where the error happned>',
          ignore_error_stats_update=true/false,
      }
      ```
      
      The `luaPushError` function was modified to construct the new error table as describe above.
      The `luaRaiseError` was renamed to `luaError` and is now simply called `lua_error` to raise
      the table on the top of the Lua stack as the error object.
      The reason is that since its functionality is changed, in case some Redis branch / fork uses it,
      it's better to have a compilation error than a bug.
      
      The `source` and `line` fields are enriched by the error handler (if possible) and the
      `ignore_error_stats_update` is optional and if its not present then the default value is `false`.
      If `ignore_error_stats_update` is true, the error will not be counted on the error stats.
      
      When parsing Redis call reply, each error is translated to a Lua table on the format describe
      above and the `ignore_error_stats_update` field is set to `true` so we will not count errors
      twice (we counted this error when we invoke the command).
      
      The changes in this PR might have been considered as a breaking change for users that used
      Lua `pcall` function. Before, the error was a string and now its a table. To keep backward
      comparability the PR override the `pcall` implementation and extract the error message from
      the error table and return it.
      
      Example of the error stats update:
      
      ```
      127.0.0.1:6379> lpush l 1
      (integer) 2
      127.0.0.1:6379> eval "return redis.call('get', 'l')" 0
      (error) WRONGTYPE Operation against a key holding the wrong kind of value. script: e471b73f1ef44774987ab00bdf51f21fd9f7974a, on @user_script:1.
      
      127.0.0.1:6379> info Errorstats
      # Errorstats
      errorstat_WRONGTYPE:count=1
      
      127.0.0.1:6379> info commandstats
      # Commandstats
      cmdstat_eval:calls=1,usec=341,usec_per_call=341.00,rejected_calls=0,failed_calls=1
      cmdstat_info:calls=1,usec=35,usec_per_call=35.00,rejected_calls=0,failed_calls=0
      cmdstat_lpush:calls=1,usec=14,usec_per_call=14.00,rejected_calls=0,failed_calls=0
      cmdstat_get:calls=1,usec=10,usec_per_call=10.00,rejected_calls=0,failed_calls=1
      ```
      
      ## error message
      We can now construct the error message (sent as a reply to the user) from the error table,
      so this solves issues where the error message was malformed and the error code appeared
      in the middle of the error message:
      
      ```diff
      127.0.0.1:6379> eval "return redis.call('set','x','y')" 0
      -(error) ERR Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479): @user_script:1: OOM command not allowed when used memory > 'maxmemory'.
      +(error) OOM command not allowed when used memory > 'maxmemory' @user_script:1. Error running script (call to 71e6319f97b0fe8bdfa1c5df3ce4489946dda479)
      ```
      
      ```diff
      127.0.0.1:6379> eval "redis.call('get', 'l')" 0
      -(error) ERR Error running script (call to f_8a705cfb9fb09515bfe57ca2bd84a5caee2cbbd1): @user_script:1: WRONGTYPE Operation against a key holding the wrong kind of value
      +(error) WRONGTYPE Operation against a key holding the wrong kind of value script: 8a705cfb9fb09515bfe57ca2bd84a5caee2cbbd1, on @user_script:1.
      ```
      
      Notica that `redis.pcall` was not change:
      ```
      127.0.0.1:6379> eval "return redis.pcall('get', 'l')" 0
      (error) WRONGTYPE Operation against a key holding the wrong kind of value
      ```
      
      
      ## other notes
      Notice that Some commands (like GEOADD) changes the cmd variable on the client stats so we
      can not count on it to update the command stats. In order to be able to update those stats correctly
      we needed to promote `realcmd` variable to be located on the client struct.
      
      Tests was added and modified to verify the changes.
      
      Related PR's: #10279, #10218, #10278, #10309
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      aa856b39
  24. 23 Feb, 2022 2 commits
    • Itamar Haber's avatar
      Add stream consumer group lag tracking and reporting (#9127) · c81c7f51
      Itamar Haber authored
      
      
      Adds the ability to track the lag of a consumer group (CG), that is, the number
      of entries yet-to-be-delivered from the stream.
      
      The proposed constant-time solution is in the spirit of "best-effort."
      
      Partially addresses #8737.
      
      ## Description of approach
      
      We add a new "entries_added" property to the stream. This starts at 0 for a new
      stream and is incremented by 1 with every `XADD`.  It is essentially an all-time
      counter of the entries added to the stream.
      
      Given the stream's length and this counter value, we can trivially find the logical
      "entries_added" counter of the first ID if and only if the stream is contiguous.
      A fragmented stream contains one or more tombstones generated by `XDEL`s.
      The new "xdel_max_id" stream property tracks the latest tombstone.
      
      The CG also tracks its last delivered ID's as an "entries_read" counter and
      increments it independently when delivering new messages, unless the this
      read counter is invalid (-1 means invalid offset). When the CG's counter is
      available, the reported lag is the difference between added and read counters.
      
      Lastly, this also adds a "first_id" field to the stream structure in order to make
      looking it up cheaper in most cases.
      
      ## Limitations
      
      There are two cases in which the mechanism isn't able to track the lag.
      In these cases, `XINFO` replies with `null` in the "lag" field.
      
      The first case is when a CG is created with an arbitrary last delivered ID,
      that isn't "0-0", nor the first or the last entries of the stream. In this case,
      it is impossible to obtain a valid read counter (short of an O(N) operation).
      The second case is when there are one or more tombstones fragmenting
      the stream's entries range.
      
      In both cases, given enough time and assuming that the consumers are
      active (reading and lacking) and advancing, the CG should be able to
      catch up with the tip of the stream and report zero lag.
      Once that's achieved, lag tracking would resume as normal (until the
      next tombstone is set).
      
      ## API changes
      
      * `XGROUP CREATE` added with the optional named argument `[ENTRIESREAD entries-read]`
        for explicitly specifying the new CG's counter.
      * `XGROUP SETID` added with an optional positional argument `[ENTRIESREAD entries-read]`
        for specifying the CG's counter.
      * `XINFO` reports the maximal tombstone ID, the recorded first entry ID, and total
        number of entries added to the stream.
      * `XINFO` reports the current lag and logical read counter of CGs.
      * `XSETID` is an internal command that's used in replication/aof. It has been added with
        the optional positional arguments `[ENTRIESADDED entries-added] [MAXDELETEDID max-deleted-entry-id]`
        for propagating the CG's offset and maximal tombstone ID of the stream.
      
      ## The generic unsolved problem
      
      The current stream implementation doesn't provide an efficient way to obtain the
      approximate/exact size of a range of entries. While it could've been nice to have
      that ability (#5813) in general, let alone specifically in the context of CGs, the risk
      and complexities involved in such implementation are in all likelihood prohibitive.
      
      ## A refactoring note
      
      The `streamGetEdgeID` has been refactored to accommodate both the existing seek
      of any entry as well as seeking non-deleted entries (the addition of the `skip_tombstones`
      argument). Furthermore, this refactoring also migrated the seek logic to use the
      `streamIterator` (rather than `raxIterator`) that was, in turn, extended with the
      `skip_tombstones` Boolean struct field to control the emission of these.
      Co-authored-by: default avatarGuy Benoish <guy.benoish@redislabs.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      c81c7f51
    • Binbin's avatar
      Fix timing issue in EXEC fail on lazy expired WATCHed key test (#10332) · 488aecb3
      Binbin authored
      The test will fail on slow machines (valgrind or FreeBsd).
      Because in #10256 when WATCH is called on a key that's already
      logically expired, we will add an `expired` flag, and we will
      skip it in `isWatchedKeyExpired` check.
      
      Apparently we need to increase the expiration time so that
      the key can not expire logically then the WATCH is called.
      Also added retries to make sure it doesn't fail. I suppose
      100ms is enough in valgrind, tested locally, no need to retry.
      488aecb3