1. 26 Apr, 2022 2 commits
    • 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 4 commits
  3. 24 Apr, 2022 3 commits
  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 4 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 4 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
    • 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
  10. 12 Apr, 2022 2 commits
  11. 11 Apr, 2022 3 commits
    • Ernesto Rodriguez Reina's avatar
      Extending the use of hashTypeGetValue. (#10567) · 4c6d9bbd
      Ernesto Rodriguez Reina authored
      
      
      * Extending the use of hashTypeGetValue.
      
      Functions hashTypeExists, hashTypeGetValueLength and addHashFieldToReply
      have a similar pattern on calling hashTypeGetFromHashTable or
      hashTypeGetFromZipList depending on the underlying data structure. What
      does functions are duing is exactly what hashTypeGetValue does. Those
      functions were changed to use existing function hashTypeGetValue making
      the code more consistent.
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      4c6d9bbd
    • zhaozhao.zz's avatar
      Durability enhancement for appendfsync=always policy (#9678) · 1a7765cb
      zhaozhao.zz authored
      Durability of database is a big and old topic, in this regard Redis use AOF to
      support it, and `appendfsync=alwasys` policy is the most strict level, guarantee
      all data is both written and synced on disk before reply success to client.
      
      But there are some cases have been overlooked, and could lead to durability broken.
      
      1. The most clear one is about threaded-io mode
         we should also set client's write handler with `ae_barrier` in
         `handleClientsWithPendingWritesUsingThreads`, or the write handler would be
         called after read handler in the next event loop, it means the write command result
         could be replied to client before flush to AOF.
      2. About blocked client (mostly by module)
         in `beforeSleep()`, `handleClientsBlockedOnKeys()` should be called before
         `flushAppendOnlyFile()`, in case the unblocked clients modify data without persistence
         but send reply.
      3. When handling `ProcessingEventsWhileBlocked`
         normally it takes place when lua/function/module timeout, and we give a chance to users
         to kill the slow operation, but we should call `flushAppendOnlyFile()` before
         `handleClientsWithPendingWrites()`, in case the other clients in the last event loop get
         acknowledge before data persistence.
         for a instance:
         ```
         in the same event loop
         client A executes set foo bar
         client B executes eval "for var=1,10000000,1 do end" 0
         ```
         after the script timeout, client A will get `OK` but lose data after restart (kill redis when
         timeout) if we don't flush the write command to AOF.
      4. A more complex case about `ProcessingEventsWhileBlocked`
         it is lua timeout in transaction, for example
         `MULTI; set foo bar; eval "for var=1,10000000,1 do end" 0; EXEC`, then client will get set
         command's result before the whole transaction done, that breaks atomicity too.
         fortunately, it's already fixed by #5428 (although it's not the original purpose just a side
         effect : )), but module timeout should be fixed too.
      
      case 1, 2, 3 are fixed in this commit, the module issue in case 4 needs a followup PR.
      1a7765cb
    • Evan's avatar
      modules: add RedisModuleKey* return type to RM_OpenKey (#3719) · 574ed6b0
      Evan authored
      Change `RM_OpenKey` to return `RedisModuleKey*` instead of `void*`.
      Which is the input type of other APIs that take the value from RM_OpenKey.
      574ed6b0
  12. 10 Apr, 2022 5 commits