1. 27 Apr, 2022 40 commits
    • filipe oliveira's avatar
      Optimization: Use either monotonic or wall-clock to measure command execution... · 7fddebc2
      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>
      (cherry picked from commit 3cd8baf6)
      7fddebc2
    • Oran Agra's avatar
      Bring some CI adjustments from 7.0 into 6.2 · 28d20b8f
      Oran Agra authored
      28d20b8f
    • Yossi Gottlieb's avatar
      Clean unused var compiler warning in module test. (#9289) · ba2feb30
      Yossi Gottlieb authored
      
      (cherry picked from commit 8bf433dc)
      ba2feb30
    • sundb's avatar
      Fix memory leak due to missing freeCallback in blockonbackground moduleapi test (#9499) · aa0606db
      sundb authored
      Before #9497, before redis-server was shut down, we did not manually shut down all the clients,
      which would have prevented valgrind from detecting a memory leak in the client's argc.
      
      (cherry picked from commit 1376d833)
      aa0606db
    • Ozan Tezcan's avatar
      Fix overflow check in expireGenericCommand · 49c1c96f
      Ozan Tezcan authored
      Partial cherry pick from #9601 in order for the tests in #9601 to pass
      
      (cherry picked from commit b91d8b28)
      49c1c96f
    • Oran Agra's avatar
    • Oran Agra's avatar
      crash log, print killer pid only when si_code is SI_USER (#10454) · e1c09d59
      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.
      
      (cherry picked from commit 6761d10c)
      e1c09d59
    • yiyuaner's avatar
      Fix an off by one error in zzlStrtod (#10465) · 8d9e75c7
      yiyuaner authored
      When vlen = sizeof(buf), the statement buf[vlen] = '\0' accessing the buffer buf is an off by one error.
      
      (cherry picked from commit 08aed7e7)
      8d9e75c7
    • Vitah Lin's avatar
      Fix memory leak in RM_StreamIteratorStop and moduleFreeKeyIterator (#10353) · 2643c691
      Vitah Lin authored
      * Fix memory leak in RM_StreamIteratorStop
      * Fix memory leak in moduleFreeKeyIterator
      
      (cherry picked from commit dff153ff)
      2643c691
    • filipe oliveira's avatar
      Optimization: Avoid deferred array reply on ZRANGE commands BYRANK (#10337) · ef78757e
      filipe oliveira authored
      
      
      Avoid deferred array reply on genericZrangebyrankCommand() when consumer type is client.
      I.e. any ZRANGE / ZREVRNGE (when tank is used).
      This was a performance regression introduced in #7844 (v 6.2) mainly affecting pipelined workloads.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 1dc89e2d)
      ef78757e
    • filipe oliveira's avatar
      Optimize deferred replies to use shared objects instead of sprintf (#10334) · 35e697de
      filipe oliveira authored
      
      
      Avoid sprintf/ll2string on setDeferredAggregateLen()/addReplyLongLongWithPrefix() when we can used shared objects.
      In some pipelined workloads this achieves about 10% improvement.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit b857928b)
      35e697de
    • qetu3790's avatar
      Fix geo search bounding box check causing missing results (#10018) · 159981e7
      qetu3790 authored
      
      
      Consider the following example:
      1. geoadd k1 -0.15307903289794921875 85 n1 0.3515625 85.00019260486917005437 n2.
      2. geodist k1 n1 n2 returns  "4891.9380"
      3. but GEORADIUSBYMEMBER k1 n1 4891.94 m only returns n1.
      n2 is in the  boundingbox but out of search areas.So we let  search areas contain boundingbox to get n2.
      Co-authored-by: default avatarBinbin <binloveplay1314@qq.com>
      (cherry picked from commit b2d393b9)
      159981e7
    • Yossi Gottlieb's avatar
      Fix OpenSSL 3.0.x related issues. (#10291) · ec06e933
      Yossi Gottlieb authored
      * Drop obsolete initialization calls.
      * Use decoder API for DH parameters.
      * Enable auto DH parameters if not explicitly used, which should be the
        preferred configuration going forward.
      
      (cherry picked from commit 3881f785)
      ec06e933
    • Oran Agra's avatar
      Fix and improve module error reply statistics (#10278) · d6a8e64e
      Oran Agra authored
      This PR handles several aspects
      1. Calls to RM_ReplyWithError from thread safe contexts don't violate thread safety.
      2. Errors returning from RM_Call to the module aren't counted in the statistics (they
        might be handled silently by the module)
      3. When a module propagates a reply it got from RM_Call to it's client, then the error
        statistics are counted.
      
      This is done by:
      1. When appending an error reply to the output buffer, we avoid updating the global
        error statistics, instead we cache that error in a deferred list in the client struct.
      2. When creating a RedisModuleCallReply object, the deferred error list is moved from
        the client into that object.
      3. when a module calls RM_ReplyWithCallReply we copy the deferred replies to the dest
        client (if that's a real client, then that's when the error statistics are updated to the server)
      
      Note about RM_ReplyWithCallReply: if the original reply had an array with errors, and the module
      replied with just a portion of the original reply, and not the entire reply, the errors are currently not
      propagated and the errors stats will not get propagated.
      
      Fix #10180
      
      (cherry picked from commit b099889a)
      d6a8e64e
    • Oran Agra's avatar
      Attempt to fix a rare crash in cluster tests. (#10265) · a06f10b0
      Oran Agra authored
      The theory is that a replica gets disconnected from within REPLCONF ACK,
      so when we go up the stack, we'll crash when attempting to access
      c->cmd->flags
      
      (cherry picked from commit aa9beaca)
      a06f10b0
    • ivanstosic-janea's avatar
      Fix protocol error caused by redis-benchmark (#10236) · 0a26869a
      ivanstosic-janea authored
      The protocol error was caused by the buggy `writeHandler` in `redis-benchmark.c`,
      which didn't handle one of the cases, thereby repeating data, leading to protocol errors
      when the values being sent are very long.
      
      This PR fixes #10233, issue introduced by #7959
      
      (cherry picked from commit bb875603)
      0a26869a
    • Binbin's avatar
      Fix PSYNC crash with wrong offset (#10243) · 2c529844
      Binbin authored
      `PSYNC replicationid str_offset` will crash the server.
      
      The reason is in `masterTryPartialResynchronization`,
      we will call `getLongLongFromObjectOrReply` check the
      offset. With a wrong offset, it will add a reply and
      then trigger a full SYNC and the client become a replica.
      
      So crash in `c->bufpos == 0 && listLength(c->reply) == 0`.
      In this commit, we check the psync_offset before entering
      function `masterTryPartialResynchronization`, and return.
      
      Regardless of that crash, accepting the sync, but also replying
      with an error would have corrupt the replication stream.
      
      (cherry picked from commit 344e41c9)
      2c529844
    • Moti Cohen's avatar
      Improve srand entropy (and fix Sentinel failures) (#10197) · 33f7e12b
      Moti Cohen authored
      As Sentinel relies upon consensus algorithm, all sentinel instances,
      randomize a time to initiate their next attempt to become the
      leader of the group. But time after time, all raffled the same value.
      
      The problem is in the line `srand(time(NULL)^getpid())` such that
      all spinned up containers get same time (in seconds) and same pid
      which is always 1. Added material `tv_usec` and verify that even
      consecutive calls brings different values and makes the difference.
      
      (cherry picked from commit 52b2fbe9)
      33f7e12b
    • Moti Cohen's avatar
      Fixed Sentinel support for hostnames (#10146) · 90891a7d
      Moti Cohen authored
      Sentinel tries to resolve instances hostname to IP only during registration.
      It might be that the instance is unavailable during that time, such as
      leader crashed and failover took place. Yet, promoted replica must support:
      
       - Register leader, even if it fails to resolve its hostname during failover
       - Try later to resolve it, if instance is disconnected. Note that
         this condition also support ip-change of an instance.
      
      (cherry picked from commit 79f089bd)
      90891a7d
    • David CARLIER's avatar
      zmalloc_get_rss netbsd impl fix proposal. (#10116) · 4a59230d
      David CARLIER authored
      Seems like the previous implementation was broken (always returning 0)
      
      since kinfo_proc2 is used the KERN_PROC2 sysctl oid is more appropriate
      and also the query's length was not necessarily accurate (6 here).
      
      (cherry picked from commit 50fa627b)
      4a59230d
    • Binbin's avatar
      LPOP/RPOP with count against non existing list return null array (#10095) · e24b947c
      Binbin authored
      It used to return `$-1` in RESP2, now we will return `*-1`.
      This is a bug in redis 6.2 when COUNT was added, the `COUNT`
      option was introduced in #8179. Fix #10089.
      
      the documentation of [LPOP](https://redis.io/commands/lpop) says
      ```
      When called without the count argument:
      Bulk string reply: the value of the first element, or nil when key does not exist.
      
      When called with the count argument:
      Array reply: list of popped elements, or nil when key does not exist.
      ```
      
      (cherry picked from commit 39feee8e)
      e24b947c
    • guybe7's avatar
      lpGetInteger returns int64_t, avoid overflow (#10068) · c5a753c8
      guybe7 authored
      Fix #9410
      
      Crucial for the ms and sequence deltas, but I changed all
      calls, just in case (e.g. "flags")
      
      Before this commit:
      `ms_delta` and `seq_delta` could have overflown, causing `currid` to be wrong,
      which in turn would cause `streamTrim` to trim the entire rax node (see new test)
      
      (cherry picked from commit 7cd6a64d)
      c5a753c8
    • 王辉's avatar
      Fix C11_ATOMIC detection on GNU Make 4.3 (#10033) · 64327e41
      王辉 authored
      Older version of GNU Make (<4.3) required quoting of number signs (#) to
      avoid them being treated as a comment. Newer versions will treat this
      quote as a literal.
      
      This issue and a proposed solution is discussed here:
      https://lists.gnu.org/archive/html/info-gnu/2020-01/msg00004.html
      
      Co-authored-by: default avatarYossi Gottlieb <yossigo@gmail.com>
      (cherry picked from commit 747b08be)
      64327e41
    • sundb's avatar
      Fix when the master connection is disconnected, replication retry read indefinitely (#10032) · aab9b127
      sundb authored
      Now if redis is still loading when we receive sigterm, we will wait for the loading to reach the event
      loop (once in 2mb) before actually shutting down. See #10003.
      
      This change caused valgrind CI to fail.
      See https://github.com/redis/redis/runs/4662901673?check_suite_focus=true
      
      This pr is mainly to solve the problem that redis process cannot be exited normally.
      When the master is disconnected, if repl is processing diskless loading and using `connRead` to read data from master,
      it may enter an infinite retry state, which does not handle `connRead` returning 0(master connection disconnected).
      
      (cherry picked from commit 73951abe)
      aab9b127
    • Madelyn Olson's avatar
      Redact ACL SETUSER arguments if the user has spaces (#9935) · 066b6832
      Madelyn Olson authored
      
      (cherry picked from commit c40d23b8)
      066b6832
    • sundb's avatar
      Santize dump payload: fix invalid listpack entry start with EOF (#9889) · db3e3ebc
      sundb authored
      When an invalid listpack entry starts with EOF, we will skip it when we verify it in the loop.
      
      (cherry picked from commit 1808618f)
      db3e3ebc
    • OfirMos's avatar
      fixed mem leak on rdb load error (#9860) · 86db5091
      OfirMos authored
      a rare case of short read that can happen when breaking the master-replica
      connection on diskless load mode,
      
      (cherry picked from commit 9f9c7857)
      86db5091
    • Meir Shpilraien (Spielrein)'s avatar
      Clean Lua stack before parsing call reply to avoid crash on a call with many arguments (#9809) · 93c1d31d
      Meir Shpilraien (Spielrein) authored
      This commit 0f8b634c (CVE-2021-32626 released in 6.2.6, 6.0.16, 5.0.14)
      fixes an invalid memory write issue by using `lua_checkstack` API to make
      sure the Lua stack is not overflow. This fix was added on 3 places:
      1. `luaReplyToRedisReply`
      2. `ldbRedis`
      3. `redisProtocolToLuaType`
      
      On the first 2 functions, `lua_checkstack` is handled gracefully while the
      last is handled with an assert and a statement that this situation can
      not happened (only with misbehave module):
      
      > the Redis reply might be deep enough to explode the LUA stack (notice
      that currently there is no such command in Redis that returns such a nested
      reply, but modules might do it)
      
      The issue that was discovered is that user arguments is also considered part
      of the stack, and so the following script (for example) make the assertion reachable:
      ```
      local a = {}
      for i=1,7999 do
          a[i] = 1
      end
      return redis.call("lpush", "l", unpack(a))
      ```
      
      This is a regression because such a script would have worked before and now
      its crashing Redis. The solution is to clear the function arguments from the Lua
      stack which makes the original assumption true and the assertion unreachable.
      
      (cherry picked from commit 6b0b04f1)
      93c1d31d
    • Binbin's avatar
      Add tests to cover EXPIRE overflow fix (#9839) · 8fca090e
      Binbin authored
      In #8287, some overflow checks have been added. But when
      `when *= 1000` overflows, it will become a positive number.
      And the check not able to catch it. The key will be added with
      a short expiration time and will deleted a few seconds later.
      
      In #9601, will check the overflow after `*=` and return an
      error first, and avoiding this situation.
      
      In this commit, added some tests to cover those code paths.
      Found it in #9825, and close it.
      
      (cherry picked from commit 9273d09d)
      8fca090e
    • Oran Agra's avatar
      fix invalid read on corrupt ziplist (#9831) · e38d0b5a
      Oran Agra authored
      If the last bytes in ziplist are corrupt and we decode from tail to head,
      we may reach slightly outside the ziplist.
      
      (cherry picked from commit a3a01429)
      e38d0b5a
    • Wen Hui's avatar
      Sentinel tls memory leak (#9753) · 94013b8e
      Wen Hui authored
      There was a memory leak when tls is used in Sentinels.
      The memory leak is noticed when some of the replicas are offline.
      
      (cherry picked from commit 2ce29e03)
      94013b8e
    • Itamar Haber's avatar
      Fixes LPOP/RPOP wrong replies when count is 0 (#9692) · 968cd2b9
      Itamar Haber authored
      Introduced in #8179, this fixes the command's replies in the 0 count edge case.
      [BREAKING] changes the reply type when count is 0 to an empty array (instead of nil)
      Moves LPOP ... 0 fast exit path after type check to reply with WRONGTYPE
      
      (cherry picked from commit 06dd202a)
      968cd2b9
    • Rafi Einstein's avatar
      Fix memory leak when there's a read error of module aux data from rdb. (#9705) · 19c2b179
      Rafi Einstein authored
      
      (cherry picked from commit 734cde7e)
      19c2b179
    • Oran Agra's avatar
      fix new cluster tests issues (#9657) · 3ba7c6ac
      Oran Agra authored
      Following #9483 the daily CI exposed a few problems.
      
      * The cluster creation code (uses redis-cli) is complicated to test with TLS enabled.
        for now i'm just skipping them since the tests we run there don't really need that kind of coverage
      * cluster port binding failures
        note that `find_available_port` already looks for a free cluster port
        but the code in `wait_server_started` couldn't detect the failure of binding
        (the text it greps for wasn't found in the log)
      
      (cherry picked from commit 7d6744c7)
      3ba7c6ac
    • qetu3790's avatar
      Release clients blocked on module commands in cluster resharding and down state (#9483) · 936ee017
      qetu3790 authored
      
      
      Prevent clients from being blocked forever in cluster when they block with their own module command
      and the hash slot is migrated to another master at the same time.
      These will get a redirection message when unblocked.
      Also, release clients blocked on module commands when cluster is down (same as other blocked clients)
      
      This commit adds basic tests for the main (non-cluster) redis test infra that test the cluster.
      This was done because the cluster test infra can't handle some common test features,
      but most importantly we only build the test modules with the non-cluster test suite.
      
      note that rather than really supporting cluster operations by the test infra, it was added (as dup code)
      in two files, one for module tests and one for non-modules tests, maybe in the future we'll refactor that.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 4962c552)
      936ee017
    • DarrenJiang13's avatar
      add missed error counting (#9646) · 8a0515d9
      DarrenJiang13 authored
      * add: add missed error counting in sentinel.c and cluster.c
      
      (cherry picked from commit aa6deff0)
      8a0515d9
    • Yossi Gottlieb's avatar
      hiredis: improve calloc() overflow fix. (#9630) · 21ab5d4f
      Yossi Gottlieb authored
      Cherry pick a more complete fix to 0215324a that also doesn't leak
      memory from latest hiredis.
      
      (cherry picked from commit 922ef86a)
      21ab5d4f
    • Huang Zhw's avatar
      Make tracking invalidation messages always after command's reply (#9422) · 0fb96d55
      Huang Zhw authored
      Tracking invalidation messages were sometimes sent in inconsistent order,
      before the command's reply rather than after.
      In addition to that, they were sometimes embedded inside other commands
      responses, like MULTI-EXEC and MGET.
      
      (cherry picked from commit fd135f3e)
      0fb96d55
    • Binbin's avatar
      Add missing pause tcl test to test_helper.tcl (#9158) · f5d8a369
      Binbin authored
      * Add keyname tags to avoid CROSSSLOT errors in external server CI
      * Use new wait_for_blocked_clients_count in pause.tcl
      
      (cherry picked from commit 5dddf496)
      f5d8a369
    • meir's avatar
      Protect any table which is reachable from globals and added globals allow list. · 11b602fb
      meir authored
      The allow list is done by setting a metatable on the global table before initializing
      any library. The metatable set the `__newindex` field to a function that check
      the allow list before adding the field to the table. Fields which is not on the
      allow list are simply ignored.
      
      After initialization phase is done we protect the global table and each table
      that might be reachable from the global table. For each table we also protect
      the table metatable if exists.
      11b602fb