1. 27 Dec, 2021 3 commits
    • Itamar Haber's avatar
      Adds utils/gen-commands-json.py (#9958) · f810510b
      Itamar Haber authored
      Following #9656, this script generates a "commands.json" file from the output
      of the new COMMAND. The output of this script is used in redis/redis-doc#1714
      and by redis/redis-io#259. This also converts a couple of rogue dashes (in 
      'key-specs' and 'multiple-token' flags) to underscores (continues #9959).
      f810510b
    • chenyang8094's avatar
      Fix failing test due to recent change in transaction propagation (#10006) · 317464a3
      chenyang8094 authored
      
      
      PR #9890 may have introduced a problem.
      There are tests that use MULTI-EXEC to make sure two BGSAVE / BGREWRITEAOF are executed together.
      But now it's not valid to run run commands that create a snapshot inside a transaction (gonna be blocked soon)
      This PR modifies the test not to rely on MULTI-EXEC.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      317464a3
    • guybe7's avatar
      Fix race in propagation test (#10012) · 0f15e025
      guybe7 authored
      There's a race between testing DBSIZE and the thread starting.
      If the thread hadn't started by the time we checked DBISZE, no
      keys will have been evicted.
      The correct way is to check the evicted_keys stat.
      0f15e025
  2. 26 Dec, 2021 3 commits
    • Binbin's avatar
      santize dump payload: fix carsh when zset with NAN score (#10002) · e84ccc3f
      Binbin authored
      `zslInsert` with a NAN score will crash the server.
      This one found by the `corrupt-dump-fuzzer`.
      e84ccc3f
    • Meir Shpilraien (Spielrein)'s avatar
      Add FUNCTION DUMP and RESTORE. (#9938) · 365cbf46
      Meir Shpilraien (Spielrein) authored
      Follow the conclusions to support Functions in redis cluster (#9899)
      
      Added 2 new FUNCTION sub-commands:
      1. `FUNCTION DUMP` - dump a binary payload representation of all the functions.
      2. `FUNCTION RESTORE <PAYLOAD> [FLUSH|APPEND|REPLACE]` - give the binary payload extracted
         using `FUNCTION DUMP`, restore all the functions on the given payload. Restore policy can be given to
         control how to handle existing functions (default is APPEND):
         * FLUSH: delete all existing functions.
         * APPEND: appends the restored functions to the existing functions. On collision, abort.
         * REPLACE: appends the restored functions to the existing functions. On collision,
           replace the old function with the new function.
      
      Modify `redis-cli --cluster add-node` to use `FUNCTION DUMP` to get existing functions from
      one of the nodes in the cluster, and `FUNCTION RESTORE` to load the same set of functions
      to the new node. `redis-cli` will execute this step before sending the `CLUSTER MEET` command
      to the new node. If `FUNCTION DUMP` returns an error, assume the current Redis version do not
      support functions and skip `FUNCTION RESTORE`. If `FUNCTION RESTORE` fails, abort and do not send
      the `CLUSTER MEET` command. If the new node already contains functions (before the `FUNCTION RESTORE`
      is sent), abort and do not add the node to the cluster. Test was added to verify
      `redis-cli --cluster add-node` works as expected. 
      365cbf46
    • Meir Shpilraien (Spielrein)'s avatar
      Changed fuction name to be case insensitive. (#9984) · 08ff606b
      Meir Shpilraien (Spielrein) authored
      Use case insensitive string comparison for function names (like we do for commands and configs)
      In addition, add verification that the functions only use the following characters: [a-zA-Z0-9_]
      08ff606b
  3. 22 Dec, 2021 5 commits
    • guybe7's avatar
      Sort out mess around propagation and MULTI/EXEC (#9890) · 7ac21307
      guybe7 authored
      The mess:
      Some parts use alsoPropagate for late propagation, others using an immediate one (propagate()),
      causing edge cases, ugly/hacky code, and the tendency for bugs
      
      The basic idea is that all commands are propagated via alsoPropagate (i.e. added to a list) and the
      top-most call() is responsible for going over that list and actually propagating them (and wrapping
      them in MULTI/EXEC if there's more than one command). This is done in the new function,
      propagatePendingCommands.
      
      Callers to propagatePendingCommands:
      1. top-most call() (we want all nested call()s to add to the also_propagate array and just the top-most
         one to propagate them) - via `afterCommand`
      2. handleClientsBlockedOnKeys: it is out of call() context and it may propagate stuff - via `afterCommand`. 
      3. handleClientsBlockedOnKeys edge case: if the looked-up key is already expired, we will propagate the
         expire but will not unblock any client so `afterCommand` isn't called. in that case, we have to propagate
         the deletion explicitly.
      4. cron stuff: active-expire and eviction may also propagate stuff
      5. modules: the module API allows to propagate stuff from just about anywhere (timers, keyspace notifications,
         threads). I could have tried to catch all the out-of-call-context places but it seemed easier to handle it in one
         place: when we free the context. in the spirit of what was done in call(), only the top-most freeing of a module
         context may cause propagation.
      6. modules: when using a thread-safe ctx it's not clear when/if the ctx will be freed. we do know that the module
         must lock the GIL before calling RM_Replicate/RM_Call so we propagate the pending commands when
         releasing the GIL.
      
      A "known limitation", which were actually a bug, was fixed because of this commit (see propagate.tcl):
         When using a mix of RM_Call with `!` and RM_Replicate, the command would propagate out-of-order:
         first all the commands from RM_Call, and then the ones from RM_Replicate
      
      Another thing worth mentioning is that if, in the past, a client would issue a MULTI/EXEC with just one
      write command the server would blindly propagate the MULTI/EXEC too, even though it's redundant.
      not anymore.
      
      This commit renames propagate() to propagateNow() in order to cause conflicts in pending PRs.
      propagatePendingCommands is the only caller of propagateNow, which is now a static, internal helper function.
      
      Optimizations:
      1. alsoPropagate will not add stuff to also_propagate if there's no AOF and replicas
      2. alsoPropagate reallocs also_propagagte exponentially, to save calls to memmove
      
      Bugfixes:
      1. CONFIG SET can create evictions, sending notifications which can cause to dirty++ with modules.
         we need to prevent it from propagating to AOF/replicas
      2. We need to set current_client in RM_Call. buggy scenario:
         - CONFIG SET maxmemory, eviction notifications, module hook calls RM_Call
         - assertion in lookupKey crashes, because current_client has CONFIG SET, which isn't CMD_WRITE
      3. minor: in eviction, call propagateDeletion after notification, like active-expire and all commands
         (we always send a notification before propagating the command)
      7ac21307
    • Oran Agra's avatar
      resolve replication test timing sensitivity - 2nd attempt (#9988) · b7567394
      Oran Agra authored
      issue started failing after #9878 was merged (made an exiting test more sensitive)
      looks like #9982 didn't help, tested this one and it seems to work better.
      
      this commit does two things:
      1. reduce the extra delay i added earlier and instead add more keys, the effect no duration
         of replication is the same, but the intervals in which the server is responsive to the tcl client is higher.
      2. improve the test infra to print context when assert_error fails.
      b7567394
    • Oran Agra's avatar
      resolve replication test timing sensitivity (#9982) · e33e0295
      Oran Agra authored
      issue started failing after #9878 was merged (made an exiting test more sensitive)
      e33e0295
    • Oran Agra's avatar
      Allow most CONFIG SET during loading, block some commands in async-loading (#9878) · 41e6e05d
      Oran Agra authored
      ## background
      Till now CONFIG SET was blocked during loading.
      (In the not so distant past, GET was disallowed too)
      
      We recently (not released yet) added an async-loading mode, see #9323,
      and during that time it'll serve CONFIG SET and any other command.
      And now we realized (#9770) that some configs, and commands are dangerous
      during async-loading.
      
      ## changes
      * Allow most CONFIG SET during loading (both on async-loading and normal loading)
      * Allow CONFIG REWRITE and CONFIG RESETSTAT during loading
      * Block a few config during loading (`appendonly`, `repl-diskless-load`, and `dir`)
      * Block a few commands during loading (list below)
      
      ## the blocked commands:
      * SAVE - obviously we don't wanna start a foregreound save during loading 8-)
      * BGSAVE - we don't mind to schedule one, but we don't wanna fork now
      * BGREWRITEAOF - we don't mind to schedule one, but we don't wanna fork now
      * MODULE - we obviously don't wanna unload a module during replication / rdb loading
        (MODULE HELP and MODULE LIST are not blocked)
      * SYNC / PSYNC - we're in the middle of RDB loading from master, must not allow sync
        requests now.
      * REPLICAOF / SLAVEOF - we're in the middle of replicating, maybe it makes sense to let
        the user abort it, but he couldn't do that so far, i don't wanna take any risk of bugs due to odd state.
      * CLUSTER - only allow [HELP, SLOTS, NODES, INFO, MYID, LINKS, KEYSLOT, COUNTKEYSINSLOT,
        GETKEYSINSLOT, RESET, REPLICAS, COUNT_FAILURE_REPORTS], for others, preserve the status quo
      
      ## other fixes
      * processEventsWhileBlocked had an issue when being nested, this could happen with a busy script
        during async loading (new), but also in a busy script during AOF loading (old). this lead to a crash in
        the scenario described in #6988
      41e6e05d
    • zhugezy's avatar
      Shorten timeouts of CLIENT PAUSE to avoid hanging when tests fail. (#9975) · ad55fbaa
      zhugezy authored
      If a test fails at `wait_for_blocked_clients_count` after the `PAUSE` command,
      It won't send `UNPAUSE` to server, leading to the server hanging until timeout,
      which is bad and hard to debug sometimes when developing.
      This PR tries to fix this.
      
      Timeout in `CLIENT PAUSE` shortened from 1e5 seconds(extremely long) to 50~100 seconds.
      ad55fbaa
  4. 21 Dec, 2021 2 commits
    • Meir Shpilraien (Spielrein)'s avatar
      Change FUNCTION CREATE, DELETE and FLUSH to be WRITE commands instead of MAY_REPLICATE. (#9953) · 3bcf1084
      Meir Shpilraien (Spielrein) authored
      The issue with MAY_REPLICATE is that all automatic mechanisms to handle
      write commands will not work. This require have a special treatment for:
      * Not allow those commands to be executed on RO replica.
      * Allow those commands to be executed on RO replica from primary connection.
      * Allow those commands to be executed on the RO replica from AOF.
      
      By setting those commands as WRITE commands we are getting all those properties from Redis.
      Test was added to verify that those properties work as expected.
      
      In addition, rearrange when and where functions are flushed. Before this PR functions were
      flushed manually on `rdbLoadRio` and cleaned manually on failure. This contradicts the
      assumptions that functions are data and need to be created/deleted alongside with the
      data. A side effect of this, for example, `debug reload noflush` did not flush the data but
      did flush the functions, `debug loadaof` flush the data but not the functions.
      This PR move functions deletion into `emptyDb`. `emptyDb` (renamed to `emptyData`) will
      now accept an additional flag, `NOFUNCTIONS` which specifically indicate that we do not
      want to flush the functions (on all other cases, functions will be flushed). Used the new flag
      on FLUSHALL and FLUSHDB only! Tests were added to `debug reload` and `debug loadaof`
      to verify that functions behave the same as the data.
      
      Notice that because now functions will be deleted along side with the data we can not allow
      `CLUSTER RESET` to be called from within a function (it will cause the function to be released
      while running), this PR adds `NO_SCRIPT` flag to `CLUSTER RESET`  so it will not be possible
      to be called from within a function. The other cluster commands are allowed from within a
      function (there are use-cases that uses `GETKEYSINSLOT` to iterate over all the keys on a
      given slot). Tests was added to verify `CLUSTER RESET` is denied from within a script.
      
      Another small change on this PR is that `RDBFLAGS_ALLOW_DUP` is also applicable on functions.
      When loading functions, if this flag is set, we will replace old functions with new ones on collisions. 
      3bcf1084
    • zhugezy's avatar
      Remove EVAL script verbatim replication, propagation, and deterministic execution logic (#9812) · 1b0968df
      zhugezy authored
      
      
      # Background
      
      The main goal of this PR is to remove relevant logics on Lua script verbatim replication,
      only keeping effects replication logic, which has been set as default since Redis 5.0.
      As a result, Lua in Redis 7.0 would be acting the same as Redis 6.0 with default
      configuration from users' point of view.
      
      There are lots of reasons to remove verbatim replication.
      Antirez has listed some of the benefits in Issue #5292:
      
      >1. No longer need to explain to users side effects into scripts.
          They can do whatever they want.
      >2. No need for a cache about scripts that we sent or not to the slaves.
      >3. No need to sort the output of certain commands inside scripts
          (SMEMBERS and others): this both simplifies and gains speed.
      >4. No need to store scripts inside the RDB file in order to startup correctly.
      >5. No problems about evicting keys during the script execution.
      
      When looking back at Redis 5.0, antirez and core team decided to set the config
      `lua-replicate-commands yes` by default instead of removing verbatim replication
      directly, in case some bad situations happened. 3 years later now before Redis 7.0,
      it's time to remove it formally.
      
      # Changes
      
      - configuration for lua-replicate-commands removed
        - created config file stub for backward compatibility
      - Replication script cache removed
        - this is useless under script effects replication
        - relevant statistics also removed
      - script persistence in RDB files is also removed
      - Propagation of SCRIPT LOAD and SCRIPT FLUSH to replica / AOF removed
      - Deterministic execution logic in scripts removed (i.e. don't run write commands
        after random ones, and sorting output of commands with random order)
        - the flags indicating which commands have non-deterministic results are kept as hints to clients.
      - `redis.replicate_commands()` & `redis.set_repl()` changed
        - now `redis.replicate_commands()` does nothing and return an 1
        - ...and then `redis.set_repl()` can be issued before `redis.replicate_commands()` now
      - Relevant TCL cases adjusted
      - DEBUG lua-always-replicate-commands removed
      
      # Other changes
      - Fix a recent bug comparing CLIENT_ID_AOF to original_client->flags instead of id. (introduced in #9780)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      1b0968df
  5. 20 Dec, 2021 1 commit
    • Binbin's avatar
      Fix recent daily CI test failures (#9966) · febc3f63
      Binbin authored
      Recent PRs have introduced some failures, this commit
      try to fix these CI failures. Here are the changes:
      
      1. Enable debug-command in sentinel test.
      ```
      Master reboot in very short time: ERR DEBUG command not allowed. If the
      enable-debug-command option is set to "local", you can run it from a
      local connection, otherwise you need to set this option in the
      configuration file, and then restart the server.
      ```
      
      2. Enable protected-config in sentinel test.
      ```
      SDOWN is triggered by misconfigured instance replying with errors: ERR
      CONFIG SET failed (possibly related to argument 'dir') - can't set
      protected config
      ```
      
      3. Enable debug-command in cluster test.
      ```
      Verify slaves consistency: ERR DEBUG command not allowed. If the
      enable-debug-command option is set to "local", you can run it from a
      local connection, otherwise you need to set this option in the
      configuration file, and then restart the server.
      ```
      
      4. quicklist fill should be signed int.
      The reason for the modification is to eliminate the warning.
      Modify `int fill: QL_FILL_BITS` to `signed int fill: QL_FILL_BITS`
      
      The first three were introduced at #9920 (same issue).
      And the last one was introduced at #9962.
      febc3f63
  6. 19 Dec, 2021 2 commits
    • Oran Agra's avatar
      Add external test that runs without debug command (#9964) · 6add1b72
      Oran Agra authored
      - add needs:debug flag for some tests
      - disable "save" in external tests (speedup?)
      - use debug_digest proc instead of debug command directly so it can be skipped
      - use OBJECT ENCODING instead of DEBUG OBJECT to get encoding
      - add a proc for OBJECT REFCOUNT so it can be skipped
      - move a bunch of tests in latency_monitor tests to happen later so that latency monitor has some values in it
      - add missing close_replication_stream calls
      - make sure to close the temp client if DEBUG LOG fails
      6add1b72
    • YaacovHazan's avatar
      Protected configs and sensitive commands (#9920) · ae2f5b7b
      YaacovHazan authored
      Block sensitive configs and commands by default.
      
      * `enable-protected-configs` - block modification of configs with the new `PROTECTED_CONFIG` flag.
         Currently we add this flag to `dbfilename`, and `dir` configs,
         all of which are non-mutable configs that can set a file redis will write to.
      * `enable-debug-command` - block the `DEBUG` command
      * `enable-module-command` - block the `MODULE` command
      
      These have a default value set to `no`, so that these features are not
      exposed by default to client connections, and can only be set by modifying the config file.
      
      Users can change each of these to either `yes` (allow all access), or `local` (allow access from
      local TCP connections and unix domain connections)
      
      Note that this is a **breaking change** (specifically the part about MODULE command being disabled by default).
      I.e. we don't consider DEBUG command being blocked as an issue (people shouldn't have been using it),
      and the few configs we protected are unlikely to have been set at runtime anyway.
      On the other hand, it's likely to assume some users who use modules, load them from the config file anyway.
      Note that's the whole point of this PR, for redis to be more secure by default and reduce the attack surface on
      innocent users, so secure defaults will necessarily mean a breaking change.
      ae2f5b7b
  7. 18 Dec, 2021 1 commit
  8. 17 Dec, 2021 1 commit
    • ny0312's avatar
      Introduce memory management on cluster link buffers (#9774) · 792afb44
      ny0312 authored
      Introduce memory management on cluster link buffers:
       * Introduce a new `cluster-link-sendbuf-limit` config that caps memory usage of cluster bus link send buffers.
       * Introduce a new `CLUSTER LINKS` command that displays current TCP links to/from peers.
       * Introduce a new `mem_cluster_links` field under `INFO` command output, which displays the overall memory usage by all current cluster links.
       * Introduce a new `total_cluster_links_buffer_limit_exceeded` field under `CLUSTER INFO` command output, which displays the accumulated count of cluster links freed due to `cluster-link-sendbuf-limit`.
      792afb44
  9. 16 Dec, 2021 2 commits
    • Meir Shpilraien (Spielrein)'s avatar
      Add FUNCTION FLUSH command to flush all functions (#9936) · 687210f1
      Meir Shpilraien (Spielrein) authored
      Added `FUNCTION FLUSH` command. The new sub-command allows delete all the functions.
      An optional `[SYNC|ASYNC]` argument can be given to control whether or not to flush the
      functions synchronously or asynchronously. if not given the default flush mode is chosen by
      `lazyfree-lazy-user-flush` configuration values.
      
      Add the missing `functions.tcl` test to the list of tests that are executed in test_helper.tcl,
      and call FUNCTION FLUSH in between servers in external mode
      687210f1
    • yoav-steinberg's avatar
      Multiparam config get. (#9914) · 70ff26b4
      yoav-steinberg authored
      Support doing `CONFIG GET <x> <y> <z>`, each of them can also be
      a pattern with wildcards.
      
      This avoids duplicates in the result by looping over the configs and for
      each once checking all the patterns, once a match is found for a pattern
      we move on to the next config.
      70ff26b4
  10. 15 Dec, 2021 2 commits
    • guybe7's avatar
      Auto-generate the command table from JSON files (#9656) · 86781600
      guybe7 authored
      Delete the hardcoded command table and replace it with an auto-generated table, based
      on a JSON file that describes the commands (each command must have a JSON file).
      
      These JSON files are the SSOT of everything there is to know about Redis commands,
      and it is reflected fully in COMMAND INFO.
      
      These JSON files are used to generate commands.c (using a python script), which is then
      committed to the repo and compiled.
      
      The purpose is:
      * Clients and proxies will be able to get much more info from redis, instead of relying on hard coded logic.
      * drop the dependency between Redis-user and the commands.json in redis-doc.
      * delete help.h and have redis-cli learn everything it needs to know just by issuing COMMAND (will be
        done in a separate PR)
      * redis.io should stop using commands.json and learn everything from Redis (ultimately one of the release
        artifacts should be a large JSON, containing all the information about all of the commands, which will be
        generated from COMMAND's reply)
      * the byproduct of this is:
        * module commands will be able to provide that info and possibly be more of a first-class citizens
        * in theory, one may be able to generate a redis client library for a strictly typed language, by using this info.
      
      ### Interface changes
      
      #### COMMAND INFO's reply change (and arg-less COMMAND)
      
      Before this commit the reply at index 7 contained the key-specs list
      and reply at index 8 contained the sub-commands list (Both unreleased).
      Now, reply at index 7 is a map of:
      - summary - short command description
      - since - debut version
      - group - command group
      - complexity - complexity string
      - doc-flags - flags used for documentation (e.g. "deprecated")
      - deprecated-since - if deprecated, from which version?
      - replaced-by - if deprecated, which command replaced it?
      - history - a list of (version, what-changed) tuples
      - hints - a list of strings, meant to provide hints for clients/proxies. see https://github.com/redis/redis/issues/9876
      - arguments - an array of arguments. each element is a map, with the possibility of nesting (sub-arguments)
      - key-specs - an array of keys specs (already in unstable, just changed location)
      - subcommands - a list of sub-commands (already in unstable, just changed location)
      - reply-schema - will be added in the future (see https://github.com/redis/redis/issues/9845)
      
      more details on these can be found in https://github.com/redis/redis-doc/pull/1697
      
      only the first three fields are mandatory 
      
      #### API changes (unreleased API obviously)
      
      now they take RedisModuleCommand opaque pointer instead of looking up the command by name
      
      - RM_CreateSubcommand
      - RM_AddCommandKeySpec
      - RM_SetCommandKeySpecBeginSearchIndex
      - RM_SetCommandKeySpecBeginSearchKeyword
      - RM_SetCommandKeySpecFindKeysRange
      - RM_SetCommandKeySpecFindKeysKeynum
      
      Currently, we did not add module API to provide additional information about their commands because
      we couldn't agree on how the API should look like, see https://github.com/redis/redis/issues/9944
      
      .
      
      ### Somehow related changes
      1. Literals should be in uppercase while placeholder in lowercase. Now all the GEO* command
         will be documented with M|KM|FT|MI and can take both lowercase and uppercase
      
      ### Unrelated changes
      1. Bugfix: no_madaory_keys was absent in COMMAND's reply
      2. expose CMD_MODULE as "module" via COMMAND
      3. have a dedicated uint64 for ACL categories (instead of having them in the same uint64 as command flags)
      Co-authored-by: default avatarItamar Haber <itamar@garantiadata.com>
      86781600
    • Wen Hui's avatar
      Error message improvement for CONFIG SET command (#9924) · a09bc504
      Wen Hui authored
      When CONFIG SET fails, print the name of the config that failed.
      This is helpful since config set is now variadic.
      
      however, there are cases where several configs have the same apply
      function, and we can't be sure which one of them caused the failure.
      a09bc504
  11. 13 Dec, 2021 2 commits
    • yoav-steinberg's avatar
      Fix possible int overflow when hashing an sds. (#9916) · c7dc17fc
      yoav-steinberg authored
      This caused a crash when adding elements larger than 2GB to a set (same goes for hash keys). See #8455.
      
      Details:
      * The fix makes the dict hash functions receive a `size_t` instead of an `int`. In practice the dict hash functions
        call siphash which receives a `size_t` and the callers to the hash function pass a `size_t` to it so the fix is trivial.
      * The issue was recreated by attempting to add a >2gb value to a set. Appropriate tests were added where I create
        a set with large elements and check basic functionality on it (SADD, SCARD, SPOP, etc...).
      * When I added the tests I also refactored a bit all the tests code which is run under the `--large-memory` flag.
        This removed code duplication for the test framework's `write_big_bulk` and `write_big_bulk` code and also takes
        care of not allocating the test frameworks helper huge string used by these tests when not run under `--large-memory`.
      * I also added the _violoations.tcl_ unit tests to be part of the entire test suite and leaned up non relevant list related
        tests that were in there. This was done in this PR because most of the _violations_ tests are "large memory" tests.
      c7dc17fc
    • Madelyn Olson's avatar
  12. 10 Dec, 2021 1 commit
    • Binbin's avatar
      Fix timing issue in strem blocking tests (#9927) · b93ccee4
      Binbin authored
      A test failure was reported in Daily CI (FreeBSD).
      `XREAD: XADD + DEL should not awake client`
      
      ```
      *** [err]: XREAD: XADD + DEL should not awake client in tests/unit/type/stream.tcl
      Expected [lindex  0 0] eq {s1} (context: type eval line 11 cmd {assert {[lindex $res 0 0] eq {s1}}} proc ::test)
      ```
      
      It seems that `r` is executed before `rd` enters the blocking
      state. And ended up getting a empty reply by timeout.
      
      We use `wait_for_blocked_clients_count` to wait for the
      blocking client to be ready and avoid this situation.
      Also fixed other test cases that may have the same issue.
      b93ccee4
  13. 08 Dec, 2021 3 commits
  14. 07 Dec, 2021 2 commits
    • yoav-steinberg's avatar
      Don't write oom score adj to proc unless we're managing it. (#9904) · 1736fa4d
      yoav-steinberg authored
      When disabling redis oom-score-adj managment we restore the
      base value read before enabling oom-score-adj management.
      
      This fixes an issue introduced in #9748 where updating
      `oom-score-adj-values` while `oom-score-adj` was set to `no`
      would write the base oom score adj value read on startup to `/proc`.
      This is a bug since while `oom-score-adj` is disabled we should
      never write to proc and let external processes manage it.
      
      Added appropriate tests.
      1736fa4d
    • Binbin's avatar
      Fix timing issue in logging.tcl with FreeBSD (#9910) · b947049f
      Binbin authored
      A test failure was reported in Daily CI.
      `Crash report generated on SIGABRT` with FreeBSD.
      
      ```
      *** [err]: Crash report generated on SIGABRT in tests/integration/logging.tcl
      Expected [string match *crashed by signal* ### Starting...(logs) in tests/integration/logging.tcl]
      ```
      
      It look like `tail -1000` was executed too early, before it
      printed out all the crash logs. We can give it a few more
      chances by using `wait_for_log_messages`.
      
      Other changes:
      1. In `Server is able to generate a stack trace on selected systems`,
      use `wait_for_log_messages`to reduce the lines of code. And if it
      fails, there are more detailed logs that can be printed.
      
      2. In `Crash report generated on DEBUG SEGFAULT`, we also use
      `wait_for_log_messages` to avoid possible timing issues.
      b947049f
  15. 04 Dec, 2021 1 commit
  16. 02 Dec, 2021 2 commits
    • meir@redislabs.com's avatar
      Redis Functions - Added redis function unit and Lua engine · cbd46317
      meir@redislabs.com authored
      Redis function unit is located inside functions.c
      and contains Redis Function implementation:
      1. FUNCTION commands:
        * FUNCTION CREATE
        * FCALL
        * FCALL_RO
        * FUNCTION DELETE
        * FUNCTION KILL
        * FUNCTION INFO
      2. Register engine
      
      In addition, this commit introduce the first engine
      that uses the Redis Function capabilities, the
      Lua engine.
      cbd46317
    • Binbin's avatar
      Fix CONFIG SET test failures in MacOS/FreeBSD (#9881) · e57a4db5
      Binbin authored
      After the introduction of `Multiparam config set` in #9748,
      there are two tests cases failed.
      
      ```
      [exception]: Executing test client: ERR Config set failed - Failed to set current oom_score_adj. Check server logs..
      ERR Config set failed - Failed to set current oom_score_adj. Check server logs.
      ```
      
      `CONFIG sanity` test failed on the `config set oom-score-adj-values`
      which is a "special" config that does not catch no-op changes.
      And then it will update `oom-score-adj` which not supported in
      MacOs. We solve it by adding `oom-score*` to the `skip_configs` list.
      
      ```
      *** [err]: CONFIG SET rollback on apply error in tests/unit/introspection.tcl
      Expected an error but nothing was caught
      ```
      
      `CONFIG SET rollback on apply error` test failed on the
      `config set port $used_port`. In theory, it should throw the
      error `Unable to listen on this port*`. But it failed on MacOs.
      We solve it by adding `-myaddr 127.0.0.1` to the socket call.
      e57a4db5
  17. 01 Dec, 2021 2 commits
    • meir@redislabs.com's avatar
      Redis Functions - Introduce script unit. · fc731bc6
      meir@redislabs.com authored
      Script unit is a new unit located on script.c.
      Its purpose is to provides an API for functions (and eval)
      to interact with Redis. Interaction includes mostly
      executing commands, but also functionalities like calling
      Redis back on long scripts or check if the script was killed.
      
      The interaction is done using a scriptRunCtx object that
      need to be created by the user and initialized using scriptPrepareForRun.
      
      Detailed list of functionalities expose by the unit:
      1. Calling commands (including all the validation checks such as
         acl, cluster, read only run, ...)
      2. Set Resp
      3. Set Replication method (AOF/REPLICATION/NONE)
      4. Call Redis back to on long running scripts to allow Redis reply
         to clients and perform script kill
      
      The commit introduce the new unit and uses it on eval commands to
      interact with Redis.
      fc731bc6
    • yoav-steinberg's avatar
      Multiparam config set (#9748) · 0e5b813e
      yoav-steinberg authored
      We can now do: `config set maxmemory 10m repl-backlog-size 5m`
      
      ## Basic algorithm to support "transaction like" config sets:
      
      1. Backup all relevant current values (via get).
      2. Run "verify" and "set" on everything, if we fail run "restore".
      3. Run "apply" on everything (optional optimization: skip functions already run). If we fail run "restore".
      4. Return success.
      
      ### restore
      1. Run set on everything in backup. If we fail log it and continue (this puts us in an undefined
         state but we decided it's better than the alternative of panicking). This indicates either a bug
         or some unsupported external state.
      2. Run apply on everything in backup (optimization: skip functions already run). If we fail log
         it (see comment above).
      3. Return error.
      
      ## Implementation/design changes:
      * Apply function are idempotent (have no effect if they are run more than once for the same config).
      * No indication in set functions if we're reading the config or running from the `CONFIG SET` command
         (removed `update` argument).
      * Set function should set some config variable and assume an (optional) apply function will use that
         later to apply. If we know this setting can be safely applied immediately and can always be reverted
         and doesn't depend on any other configuration we can apply immediately from within the set function
         (and not store the setting anywhere). This is the case of this `dir` config, for example, which has no
         apply function. No apply function is need also in the case that setting the variable in the `server` struct
         is all that needs to be done to make the configuration take effect. Note that the original concept of `update_fn`,
         which received the old and new values was removed and replaced by the optional apply function.
      * Apply functions use settings written to the `server` struct and don't receive any inputs.
      * I take care that for the generic (non-special) configs if there's no change I avoid calling the setter (possible
         optimization: avoid calling the apply function as well).
      * Passing the same config parameter more than once to `config set` will fail. You can't do `config set my-setting
         value1 my-setting value2`.
      
      Note that getting `save` in the context of the conf file parsing to work here as before was a pain.
      The conf file supports an aggregate `save` definition, where each `save` line is added to the server's
      save params. This is unlike any other line in the config file where each line overwrites any previous
      configuration. Since we now support passing multiple save params in a single line (see top comments
      about `save` in https://github.com/redis/redis/pull/9644) we should deprecate the aggregate nature of
      this config line and perhaps reduce this ugly code in the future.
      0e5b813e
  18. 30 Nov, 2021 3 commits
    • Itamar Haber's avatar
      Adds auto-seq-only-generation via `XADD ... <ms>-*` (#9217) · 21aa1d4b
      Itamar Haber authored
      Adds the ability to autogenerate the sequence part of the millisecond-only explicit ID specified for `XADD`. This is useful in case added entries have an externally-provided timestamp without sub-millisecond resolution.
      21aa1d4b
    • Wen Hui's avatar
      Sentinel master reboot fix (#9438) · 2afa41f6
      Wen Hui authored
      Add master-reboot-down-after-period as a configurable parameter, to make it possible to trigger a failover from a master that is responding with `-LOADING` for a long time after being restarted.
      2afa41f6
    • Meir Shpilraien (Spielrein)'s avatar
      Swap '\r\n' with spaces when returning a big number reply from Lua script. (#9870) · b8e82d20
      Meir Shpilraien (Spielrein) authored
      The issue can only happened with a bad Lua script that claims to return
      a big number while actually return data which is not a big number (contains
      chars that are not digits). Such thing will not cause an issue unless the big
      number value contains `\r\n` and then it messes the resp3 structure. The fix
      changes all the appearances of '\r\n' with spaces.
      
      Such an issue can also happened on simple string or error replies but those
      already handle it the same way this PR does (replace `\r\n` with spaces).
      
      Other replies type are not vulnerable to this issue because they are not
      counting on free text that is terminated with `\r\n` (either it contains the
      bulk length like string reply or they are typed reply that can not inject free
      text like boolean or number).
      
      The issue only exists on unstable branch, big number reply on Lua script
      was not yet added to any official release.
      b8e82d20
  19. 29 Nov, 2021 2 commits
    • Binbin's avatar
      Fix CLIENT KILL kill all clients with id 0 (#9853) · 3119a3ae
      Binbin authored
      
      
      * Fix CLIENT KILL kill all clients with id 0 or with skipme
      CLIENT KILL with ID argument should only kill the client with the provided ID. In old code, 
      CLIENT KILL with id 0 will kill all the connected clients.
      Co-authored-by: default avatarOfir Luzon <ofirluzon@gmail.com>
      3119a3ae
    • leishiao's avatar
      improvement of a blocking xread test (#9859) · d56ded89
      leishiao authored
      
      
      This test relies on that `XREAD BLOCK 20000 STREAMS s1{t} s2{t} s3{t} $ $ $`
      is executed by redis before `XADD s2{t} * new abcd1234`. A ` wait_for_blocked_client`
      is needed between the two to ensure the order, otherwise `XADD s2{t} * new abcd1234`
      might be executed first due to network delay causing a test failure.
      Co-authored-by: default avatarxiaolei <xiaolei@91jkys.com>
      d56ded89