1. 26 Apr, 2022 7 commits
    • meir's avatar
      Added support for Lua readonly tables. · 8b33d813
      meir authored
      The new feature can be turned off and on using the new `lua_enablereadonlytable` Lua API.
      8b33d813
    • Oran Agra's avatar
      Add module API flag for using enum configs as bit flags (#10643) · 81926254
      Oran Agra authored
      Enables registration of an enum config that'll let the user pass multiple keywords that
      will be combined with `|` as flags into the integer config value.
      
      ```
          const char *enum_vals[] = {"none", "one", "two", "three"};
          const int int_vals[] = {0, 1, 2, 4};
      
          if (RedisModule_RegisterEnumConfig(ctx, "flags", 3, REDISMODULE_CONFIG_DEFAULT | REDISMODULE_CONFIG_BITFLAGS, enum_vals, int_vals, 4, getFlagsConfigCommand, setFlagsConfigCommand, NULL, NULL) == REDISMODULE_ERR) {
              return REDISMODULE_ERR;
          }
      ```
      doing:
      `config set moduleconfigs.flags "two three"` will result in 6 being passed to`setFlagsConfigCommand`.
      81926254
    • chenyang8094's avatar
      Fix bug when AOF enabled after startup. put the new incr file in the manifest... · 46ec6ad9
      chenyang8094 authored
      Fix bug when AOF enabled after startup. put the new incr file in the manifest only when AOFRW is done. (#10616)
      
      Changes:
      
      - When AOF is enabled **after** startup, the data accumulated during `AOF_WAIT_REWRITE`
        will only be stored in a temp INCR AOF file. Only after the first AOFRW is successful, we will
        add it to manifest file.
        Before this fix, the manifest referred to the temp file which could cause a restart during that
        time to load it without it's base.
      - Add `aof_rewrites_consecutive_failures` info field for  aofrw limiting implementation.
      
      Now we can guarantee that these behaviors of MP-AOF are the same as before (past redis releases):
      - When AOF is enabled after startup, the data accumulated during `AOF_WAIT_REWRITE` will only
        be stored in a visible place. Only after the first AOFRW is successful, we will add it to manifest file.
      - When disable AOF, we did not delete the AOF file in the past so there's no need to change that
        behavior now (yet).
      - When toggling AOF off and then on (could be as part of a full-sync), a crash or restart before the
        first rewrite is completed, would result with the previous version being loaded (might not be right thing,
        but that's what we always had).
      46ec6ad9
    • Eduardo Semprebon's avatar
      Allow configuring signaled shutdown flags (#10594) · 3a1d1425
      Eduardo Semprebon authored
      
      
      The SHUTDOWN command has various flags to change it's default behavior,
      but in some cases establishing a connection to redis is complicated and it's easier
      for the management software to use signals. however, so far the signals could only
      trigger the default shutdown behavior.
      Here we introduce the option to control shutdown arguments for SIGTERM and SIGINT.
      
      New config options:
      `shutdown-on-sigint [nosave | save] [now] [force]` 
      `shutdown-on-sigterm [nosave | save] [now] [force]`
      
      Implementation:
      Support MULTI_ARG_CONFIG on createEnumConfig to support multiple enums to be applied as bit flags.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      3a1d1425
    • Binbin's avatar
      Fix syntax error in replicationErrorBehavior enum (#10642) · 156836bf
      Binbin authored
      Missing a typeof, we will get errors like this:
      - multiple definition of `replicationErrorBehavior'
      - ld: error: duplicate symbol: replicationErrorBehavior
      
      Introduced in #10504
      156836bf
    • Madelyn Olson's avatar
      Set replicas to panic on disk errors, and optionally panic on replication errors (#10504) · 6fa8e4f7
      Madelyn Olson authored
      * Till now, replicas that were unable to persist, would still execute the commands
        they got from the master, now they'll panic by default, and we add a new
        `replica-ignore-disk-errors` config to change that.
      * Till now, when a command failed on a replica or AOF-loading, it only logged a
        warning and a stat, we add a new `propagation-error-behavior` config to allow
        panicking in that state (may become the default one day)
      
      Note that commands that fail on the replica can either indicate a bug that could
      cause data inconsistency between the replica and the master, or they could be
      in some cases (specifically in previous versions), a result of a command (e.g. EVAL)
      that failed on the master, but still had to be propagated to fail on the replica as well.
      6fa8e4f7
    • Madelyn Olson's avatar
      By default prevent cross slot operations in functions and scripts with # (#10615) · efcd1bf3
      Madelyn Olson authored
      Adds the `allow-cross-slot-keys` flag to Eval scripts and Functions to allow
      scripts to access keys from multiple slots.
      The default behavior is now that they are not allowed to do that (unlike before).
      This is a breaking change for 7.0 release candidates (to be part of 7.0.0), but
      not for previous redis releases since EVAL without shebang isn't doing this check.
      
      Note that the check is done on both the keys declared by the EVAL / FCALL command
      arguments, and also the ones used by the script when making a `redis.call`.
      
      A note about the implementation, there seems to have been some confusion
      about allowing access to non local keys. I thought I missed something in our
      wider conversation, but Redis scripts do block access to non-local keys.
      So the issue was just about cross slots being accessed.
      efcd1bf3
  2. 25 Apr, 2022 5 commits
    • Oran Agra's avatar
      fix broken protocol regression from #10612 (#10639) · 79ffc352
      Oran Agra authored
      A change in #10612 introduced a regression.
      when replying with garbage bytes to the caller, we must make sure it
      doesn't include any newlines.
      
      in the past it called rejectCommandFormat which did that trick.
      but now it calls rejectCommandSds, which doesn't, so we need to make sure
      to sanitize the sds.
      79ffc352
    • Binbin's avatar
      Fix typos and limit unknown command error message (#10634) · 119ec91a
      Binbin authored
      minor cleanup for recent changes.
      119ec91a
    • Lu JJ's avatar
      fix an unclear comment and add a comment to 'zi' in 'quicklist.h' (#10633) · 0c35f54f
      Lu JJ authored
      fix an unclear comment quicklist container formats to quicklist node container formats
      Add a comment to 'zi' in quicklistIter (Where it first appeared)
      Why do I add a comment to zi:
      Because it is not a variable name with a clear meaning, and its name seems to be from the deprecated ziplist.
      0c35f54f
    • guybe7's avatar
      Fix regression not aborting transaction on error, and re-edit some error responses (#10612) · df787764
      guybe7 authored
      1. Disk error and slave count checks didn't flag the transactions or counted correctly in command stats (regression from #10372  , 7.0 RC3)
      2. RM_Call will reply the same way Redis does, in case of non-exisitng command or arity error
      3. RM_WrongArtiy will consider the full command name
      4. Use lowercase 'u' in "unknonw subcommand" (to align with "unknown command")
      
      Followup work of #10127
      df787764
    • guybe7's avatar
      Test: RM_Call from within "expired" notification (#10613) · 21e39ec4
      guybe7 authored
      This case is interesting because it originates from cron,
      rather than from another command.
      
      The idea came from looking at #9890 and #10573, and I was wondering if RM_Call
      would work properly when `server.current_client == NULL`
      21e39ec4
  3. 24 Apr, 2022 5 commits
    • Yossi Gottlieb's avatar
      Run large-memory tests as solo. (#10626) · bd823c7f
      Yossi Gottlieb authored
      This avoids random memory spikes and enables --large-memory tests to run
      on moderately sized systems.
      bd823c7f
    • guybe7's avatar
      isSafeToPerformEvictions: Remove redundant condition (#10610) · 8ad0cfa5
      guybe7 authored
      If was first added in #9890 to solve the problem of
      CONFIG SET maxmemory causing eviction inside MULTI/EXEC,
      but that problem is already fixed (CONFIG SET doesn't evict
      directly, it just schedules a later eviction)
      
      Keep that condition may hide bugs (i.e. performEvictions
      should always expect to have an empty server.also_propagate)
      8ad0cfa5
    • Binbin's avatar
      Fix timing issue in slowlog redact test (#10614) · a6b3ce28
      Binbin authored
      * Fix timing issue in slowlog redact test
      
      This test failed once in my daily CI (test-sanitizer-address (clang))
      ```
      *** [err]: SLOWLOG - Some commands can redact sensitive fields in tests/unit/slowlog.tcl
      Expected 'migrate 127.0.0.1 25649 key 9 5000 AUTH2 (redacted) (redacted)' to match '* key 9 5000 AUTH (redacted)' (context: type eval line 12 cmd {assert_match {* key 9 5000 AUTH (redacted)} [lindex [lindex [r slowlog get] 1] 3]} proc ::test)
      ```
      
      The reason is that with slowlog-log-slower-than 10000,
      slowlog get will have a chance to exceed 10ms.
      
      Change slowlog-log-slower-than from 10000 to -1, disable it.
      Also handles a same potentially problematic test above.
      This is actually the same timing issue as #10432.
      
      But also avoid repeated calls to `SLOWLOG GET`
      a6b3ce28
    • Lu JJ's avatar
    • Lu JJ's avatar
      fix typo in "lcsCommand" doc comment (#10622) · 719b7665
      Lu JJ authored
      fix typo. `LCS[j+(blen+1)*j]` -> `LCS[j+(blen+1)*i]`
      719b7665
  4. 20 Apr, 2022 5 commits
    • filipe oliveira's avatar
      Optimization: Use either monotonic or wall-clock to measure command execution... · 3cd8baf6
      filipe oliveira authored
      
      Optimization: Use either monotonic or wall-clock to measure command execution time, to regain up to 4% execution time (#10502)
      
      In #7491 (part of redis 6.2), we started using the monotonic timer instead of mstime to measure
      command execution time for stats, apparently this meant sampling the clock 3 times per command
      rather than two (wince we also need the wall-clock time).
      In some cases this causes a significant overhead.
      
      This PR fixes that by avoiding the use of monotonic timer, except for the cases were we know it
      should be extremely fast.
      This PR also adds a new INFO field called `monotonic_clock` that shows which clock redis is using.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      3cd8baf6
    • Oran Agra's avatar
      Fixes around clients that must be obeyed. Replica report disk errors in PING. (#10603) · ee220599
      Oran Agra authored
      This PR unifies all the places that test if the current client is the
      master client or AOF client, and uses a method to test that on all of
      these.
      
      Other than some refactoring, these are the actual implications:
      - Replicas **don't** ignore disk error when processing commands not
        coming from their master.
        **This is important for PING to be used for health check of replicas**
      - SETRANGE, APPEND, SETBIT, BITFIELD don't do proto_max_bulk_len check for AOF
      - RM_Call in SCRIPT_MODE ignores disk error when coming from master /
        AOF
      - RM_Call in cluster mode ignores slot check when processing AOF
      - Scripts ignore disk error when processing AOF
      - Scripts **don't** ignore disk error on a replica, if the command comes
        from clients other than the master
      - SCRIPT KILL won't kill script coming from AOF
      - Scripts **don't** skip OOM check on replica if the command comes from
        clients other than the master
      
      Note that Script, AOF, and module clients don't reach processCommand,
      which is why some of the changes don't actually have any implications.
      
      Note, reverting the change done to processCommand in 2f4240b9
      should be dead code due to the above mentioned fact.
      ee220599
    • menwen's avatar
      Stop RDB child before flushing and parsing the RDB in Diskless replication too (#10602) · 046654a7
      menwen authored
      
      
      We should stop RDB child in advance before flushing to reduce COW in diskless replication too.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      046654a7
    • yoav-steinberg's avatar
      Optimized `hdr_value_at_percentile` (#10606) · 5075e743
      yoav-steinberg authored
      `hdr_value_at_percentile()` is part of the Hdr_Histogram library
      used when generating `latencystats` report. 
      
      There's a pending optimization for this function which greatly
      affects the performance of `info latencystats`.
      https://github.com/HdrHistogram/HdrHistogram_c/pull/107
      
      
      
      This PR:
      1. Upgrades the sources in _deps/hdr_histogram_ to the latest Hdr_Histogram
        version 0.11.5
      2. Applies the referenced optimization.
      3. Adds minor documentation about the hdr_histogram dependency which was
        missing under _deps/README.md_.
      
      benchmark on my machine:
      running: `redis-benchmark -n 100000 info latencystats` on a clean build with no data.
      
      | benchmark | RPS |
      | ---- | ---- |
      | before upgrade to v0.11.05  | 7,681 |
      | before optimization | 12,474 |
      | after optimization | 52,606 |
      Co-authored-by: default avatarfilipe oliveira <filipecosta.90@gmail.com>
      5075e743
    • David CARLIER's avatar
      Add socket-mark-id support for marking sockets. (#10349) · aba2865c
      David CARLIER authored
      Add a configuration option to attach an operating system-specific identifier to Redis sockets, supporting advanced network configurations using iptables (Linux) or ipfw (FreeBSD).
      aba2865c
  5. 19 Apr, 2022 5 commits
  6. 18 Apr, 2022 2 commits
    • Oran Agra's avatar
      Fix RM_Yield bug processing future commands of the current client. (#10573) · 7d1ad6ca
      Oran Agra authored
      RM_Yield was missing a call to protectClient to prevent redis from
      processing future commands of the yielding client.
      
      Adding tests that fail without this fix.
      
      This would be complicated to solve since nested calls to RM_Call used to
      replace the current_client variable with the module temp client.
      
      It looks like it's no longer necessary to do that, since it was added
      back in #9890 to solve two issues, both already gone:
      1. call to CONFIG SET maxmemory could trigger a module hook calling
         RM_Call. although this specific issue is gone, arguably other hooks
         like keyspace notification, can do the same.
      2. an assertion in lookupKey that checks the current command of the
         current client, introduced in #9572 and removed in #10248
      7d1ad6ca
    • Binbin's avatar
      Fix long long to double implicit conversion warning (#10595) · fe4b4806
      Binbin authored
      
      
      There is a implicit conversion warning in clang:
      ```
      util.c:574:23: error: implicit conversion from 'long long' to 'double'
      changes value from -4611686018427387903 to -4611686018427387904
      [-Werror,-Wimplicit-const-int-float-conversion]
          if (d < -LLONG_MAX/2 || d > LLONG_MAX/2)
      ```
      
      introduced in #10486
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      fe4b4806
  7. 17 Apr, 2022 5 commits
    • Oran Agra's avatar
      Optimize integer zset scores in listpack (converting to string and back) (#10486) · 0c4733c8
      Oran Agra authored
      When the score doesn't have fractional part, and can be stored as an integer,
      we use the integer capabilities of listpack to store it, rather than convert it to string.
      This already existed before this PR (lpInsert dose that conversion implicitly).
      
      But to do that, we would have first converted the score from double to string (calling `d2string`),
      then pass the string to `lpAppend` which identified it as being an integer and convert it back to an int.
      Now, instead of converting it to a string, we store it using lpAppendInteger`.
      
      Unrelated:
      ---
      * Fix the double2ll range check (negative and positive ranges, and also the comparison operands
        were slightly off. but also, the range could be made much larger, see comment).
      * Unify the double to string conversion code in rdb.c with the one in util.c
      * Small optimization in lpStringToInt64, don't attempt to convert strings that are obviously too long.
      
      Benchmark;
      ---
      Up to 20% improvement in certain tight loops doing zzlInsert with large integers.
      (if listpack is pre-allocated to avoid realloc, and insertion is sorted from largest to smaller)
      0c4733c8
    • guybe7's avatar
      Add RM_PublishMessageShard (#10543) · f49ff156
      guybe7 authored
      since PUBLISH and SPUBLISH use different dictionaries for channels and clients,
      and we already have an API for PUBLISH, it only makes sense to have one for SPUBLISH
      
      Add test coverage and unifying some test infrastructure.
      f49ff156
    • Meir Shpilraien (Spielrein)'s avatar
      Added test to verify loading Lua binary payload is not possible (#10583) · 789c94fe
      Meir Shpilraien (Spielrein) authored
      The tests verify that loading a binary payload to the Lua interpreter raises an error.
      The Lua code modification was done here: fdf9d455
      which force the Lau interpreter to always use the text parser.
      789c94fe
    • Wang Yuan's avatar
      Optimize the call of prepareReplicasToWrite (#10588) · a9d5cfa9
      Wang Yuan authored
      From #9166, we call several times of prepareReplicasToWrite when propagating
      one write command to replication stream (once per argument, same as we do for
      normal clients), that is not necessary. Now we only call it one time per command
      at the begin of feeding replication stream.
      
      This results in reducing CPU consumption and slightly better performance,
      specifically when there are many replicas.
      a9d5cfa9
    • guybe7's avatar
      Add RM_MallocSizeString, RM_MallocSizeDict (#10542) · fe1c096b
      guybe7 authored
      Add APIs to allow modules to compute the memory consumption of opaque objects owned by redis.
      Without these, the mem_usage callbacks of module data types are useless in many cases.
      
      Other changes:
      Fix streamRadixTreeMemoryUsage to include the size of the rax structure itself
      fe1c096b
  8. 14 Apr, 2022 2 commits
    • Madelyn Olson's avatar
      Fix incorrect error code for eval scripts and fix test error checking (#10575) · effa707e
      Madelyn Olson authored
      By the convention of errors, there is supposed to be a space between the code and the name.
      While looking at some lua stuff I noticed that interpreter errors were not adding the space,
      so some clients will try to map the detailed error message into the error.
      
      We have tests that hit this condition, but they were just checking that the string "starts" with ERR.
      I updated some other tests with similar incorrect string checking. This isn't complete though, as
      there are other ways we check for ERR I didn't fix.
      
      Produces some fun output like:
      ```
      # Errorstats
      errorstat_ERR:count=1
      errorstat_ERRuser_script_1_:count=1
      ```
      effa707e
    • Oran Agra's avatar
      solve corrupt dump fuzzer crash in streams (#10579) · 95050f26
      Oran Agra authored
      we had a panic in streamLastValidID when the stream metadata
      said it's not empty, but the rax is empty.
      95050f26
  9. 13 Apr, 2022 4 commits
    • Wen Hui's avatar
      Fix several document error and function comments (#10580) · ca913a5d
      Wen Hui authored
      
      
      This PR fix the following minor errors before Redis 7 release:
      
      ZRANGEBYLEX command in deprecated in 6.2.0, and could be replaced by ZRANGE with the
      BYLEX argument, but in the document, the words is written incorrect in " by ZRANGE with the BYSCORE argument"
      
      Fix function zpopmaxCommand incorrect comment
      
      The comments of function zmpopCommand and bzmpopCommand are not consistent with document description, fix them
      Co-authored-by: default avatarUbuntu <lucas.guang.yang1@huawei.com>
      ca913a5d
    • filipe oliveira's avatar
      Optimize stream id sds creation on XADD key * (~20% saved cpu cycles) (#10574) · a642947e
      filipe oliveira authored
      
      
      we can observe that when adding to a stream without ID there is a duplicate work
      on sds creation/freeing/sdslen that costs ~11% of the CPU cycles. 
      
      This PR avoids it by not freeing the sds after the first reply. 
      The expected reduction in CPU cycles is around 9-10% 
      
      Additionally, we now pre-allocate the sds to the right size, to avoid realloc.
      this brought another ~10% improvement
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      a642947e
    • Luke Palmer's avatar
      Keyspace event for new keys (#10512) · bb7891f0
      Luke Palmer authored
      Add an optional keyspace event when new keys are added to the db.
      
      This is useful for applications where clients need to be aware of the redis keyspace.
      Such an application can SCAN once at startup and then listen for "new" events (plus
      others associated with DEL, RENAME, etc).
      bb7891f0
    • guybe7's avatar
      Add the deprecated_since field in command args of COMMAND DOCS (#10545) · e875ff89
      guybe7 authored
      Apparently, some modules can afford deprecating command arguments
      (something that was never done in Redis, AFAIK), so in order to represent
      this piece of information, we added the `deprecated_since` field to redisCommandArg
      (in symmetry to the already existing `since` field).
      
      This commit adds `const char *deprecated_since` to `RedisModuleCommandArg`,
      which is technically a breaking change, but since 7.0 was not released yet, we decided to let it slide
      e875ff89