1. 30 Mar, 2022 4 commits
    • dependabot[bot]'s avatar
      Bump actions/checkout from 2 to 3 (#10390) · 4e55d557
      dependabot[bot] authored
      Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
      - [Release notes](https://github.com/actions/checkout/releases)
      - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
      - [Commits](https://github.com/actions/checkout/compare/v2...v3
      
      )
      
      ---
      updated-dependencies:
      - dependency-name: actions/checkout
        dependency-type: direct:production
        update-type: version-update:semver-major
      ...
      Signed-off-by: default avatardependabot[bot] <support@github.com>
      Co-authored-by: default avatardependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
      4e55d557
    • dependabot[bot]'s avatar
      Bump actions/cache from 2 to 3 (#10463) · 8df37363
      dependabot[bot] authored
      Bumps [actions/cache](https://github.com/actions/cache) from 2 to 3.
      - [Release notes](https://github.com/actions/cache/releases)
      - [Commits](https://github.com/actions/cache/compare/v2...v3
      
      )
      
      ---
      updated-dependencies:
      - dependency-name: actions/cache
        dependency-type: direct:production
        update-type: version-update:semver-major
      ...
      Signed-off-by: default avatardependabot[bot] <support@github.com>
      Co-authored-by: default avatardependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
      8df37363
    • 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
    • Binbin's avatar
      command json files cleanups (#10473) · e2fa6aa1
      Binbin authored
      This PR do some command json files cleanups:
      
      1. Add COMMAND TIPS to some commands
      - command-docs: add `NONDETERMINISTIC_OUTPUT_ORDER`
      - command-info: add `NONDETERMINISTIC_OUTPUT_ORDER`
      - command-list: add `NONDETERMINISTIC_OUTPUT_ORDER`
      - command: change `NONDETERMINISTIC_OUTPUT` to `NONDETERMINISTIC_OUTPUT_ORDER`
      - function-list: add `NONDETERMINISTIC_OUTPUT_ORDER`
      - latency-doctor: add `NONDETERMINISTIC_OUTPUT`, `REQUEST_POLICY:ALL_NODES` and `RESPONSE_POLICY:SPECIAL`
      - latency-graph: add `NONDETERMINISTIC_OUTPUT`, `REQUEST_POLICY:ALL_NODES` and `RESPONSE_POLICY:SPECIAL`
      - memory-doctor: add `REQUEST_POLICY:ALL_SHARDS` and `RESPONSE_POLICY:SPECIAL`
      - memory-malloc-stats: add `REQUEST_POLICY:ALL_SHARDS` and `RESPONSE_POLICY:SPECIAL`
      - memory-purge: add `REQUEST_POLICY:ALL_SHARDS` and `RESPONSE_POLICY:ALL_SUCCEEDED`
      - module-list: add `NONDETERMINISTIC_OUTPUT_ORDER`
      - msetnx: add `REQUEST_POLICY:MULTI_SHARD` and `RESPONSE_POLICY:AGG_MIN`
      - object-refcount: add `NONDETERMINISTIC_OUTPUT`
      3. Only (mostly) indentation and formatting changes:
      - cluster-shards
      - latency-history
      - pubsub-shardchannels
      - pubsub-shardnumsub
      - spublish
      - ssubscribe
      - sunsubscribe
      4. add doc_flags (DEPRECATED) to cluster-slots,  replaced_by `CLUSTER SHARDS` in 7.0
      5. command-getkeysandflags: a better summary (the old one is copy from command-getkeys)
      6. adjustment of command parameter types
      - `port` is integer, not string (`MIGRATE`, `REPLICAOF`, `SLAVEOF`)
      - `replicationid` is string, not integer (`PSYNC`)
      - `pattern` is pattern, not string (`PUBSUB CHANNELS`, `SENTINEL RESET`, `SORT`, `SORT_RO`)
      e2fa6aa1
  2. 29 Mar, 2022 4 commits
    • Oran Agra's avatar
      fix daily.yaml skip filters (#10490) · 16d206ee
      Oran Agra authored
      * missing parenthesis meant that the ubuntu and centos jobs were not
        skipped
      * the recently divided freebsd, macos, and valgrind jobs, which are now
        split into distict jobs for redis, modules, sentinel, cluster. were
        all executed, producing a build, but not running anything.
        now they're filtered at the job level
      * iothreads was missing from the skip list defaults, so was not skipped
      16d206ee
    • Viktor Söderqvist's avatar
      redis-cli: Do DNS lookup before sending CLUSTER MEET (#10436) · 35bb0212
      Viktor Söderqvist authored
      Affects `--cluster create` and `--cluster add-node`.
      35bb0212
    • zhaozhao.zz's avatar
      show cluster.links in MEMORY STATS (#10302) · 001e1925
      zhaozhao.zz authored
      001e1925
    • Oran Agra's avatar
      improve malloc efficiency for cluster slots_info_pairs (#10488) · 3b1e65a3
      Oran Agra authored
      This commit improve malloc efficiency of the slots_info_pairs mechanism in cluster.c
      by changing adlist into an array being realloced with greedy growth mechanism
      
      Recently the cluster tests are consistently failing when executed with ASAN in the CI.
      I tried to track down the commit that started it, and it appears to be #10293.
      Looking at the commit, i realize it didn't affect this test / flow, other than the
      replacement of the slots_info_pairs from sds to list.
      
      I concluded that what could be happening is that the slot range is very fragmented,
      and that results in many allocations.
      with sds, it results in one allocation and also, we have a greedy growth mechanism,
      but with adlist, we just have many many small allocations.
      this probably causes stress on ASAN, and causes it to be slow at termination.
      3b1e65a3
  3. 28 Mar, 2022 4 commits
  4. 27 Mar, 2022 3 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
    • Binbin's avatar
      Cleanups in redis.conf (#10452) · f25e688e
      Binbin authored
      Did some cleanups:
      1. local local typo
      2. replace the only slave word in the file
      3. add FUNCTION FLUSH to `lazyfree-lazy-user-flush` description
      4. thought it would be better to use these, there are actually "four" options
      5. the the typo
      6. remove a extra space
      7. change comment next to `activedefrag no` to match the default value
      f25e688e
  5. 26 Mar, 2022 1 commit
  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 5 commits
    • Oran Agra's avatar
      Split daily CI into smaller chunks (#10469) · 1a57af62
      Oran Agra authored
      this should aid find the CI issues with freebsd and macos runs, and also
      get faster results from valgrind and tls
      1a57af62
    • 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
  8. 21 Mar, 2022 5 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. 17 Mar, 2022 1 commit
  12. 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
  13. 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 for SORT, we would implement the complete solution which is the alternative
      described below.
      
      ### Alternative (Completion solution):
      Check sort ACL rules after executing it and before committing output (either via store or
      to COB). it would require making several changes to the sort command itself. and would
      potentially cause performance degradation since we will have to collect all the get keys
      instead of just applying them to a temp array and then scan the access keys against the
      ACL selectors. This solution can include an optimization to avoid the overheads of collecting
      the key names, in case the ACL rules grant SORT full key-access, or if the ACL key pattern
      literal matches the one used in GET/BY. It would also mean that authorization would be
      O(nlogn) since we will have to complete most of the command execution before we can
      perform verification
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      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
  14. 14 Mar, 2022 2 commits