1. 22 Feb, 2021 7 commits
    • Huang Zw's avatar
      Client tracking tracking-redir-broken push len is 2 not 3 (#8456) · e6a94aa5
      Huang Zw authored
      When redis responds with tracking-redir-broken push message (RESP3),
      it was responding with a broken protocol: an array of 3 elements, but only
      pushes 2 elements.
      
      (cherry picked from commit f687ac0c)
      e6a94aa5
    • filipe oliveira's avatar
      redis-benchmark: Fix broken protocol when used with -a or --dbnum (#8486) · 840b393c
      filipe oliveira authored
      Fix the pointers to the slot hash tags in the case of prefixed commands usage
      i.e. AUTH / SELECT
      
      It adjusts the pointers to the slot hash tags in the case of prefixed commands
      usage as soon as we get the 1st reply (same like we already did for the random
      strings within the command )
      
      (cherry picked from commit efccd635)
      840b393c
    • zhaozhao.zz's avatar
      XINFO should use lookupKeyReadOrReply (#8436) · 56ef0aa7
      zhaozhao.zz authored
      This bug would have let users observe logically expired keys on replicas and
      during CLIENT PAUSE WRITE.
      
      (cherry picked from commit f2a5fe36)
      56ef0aa7
    • Yossi Gottlieb's avatar
      Avoid assertions when testing arm64 cow bug. (#8405) · dcf409f8
      Yossi Gottlieb authored
      At least in one case the arm64 cow kernel bug test triggers an assert, which is a problem because it cannot be ignored like cases where the bug is found.
      
      On older systems (Linux <4.5) madvise fails because MADV_FREE is not supported. We treat these failures as an indication the system is not affected.
      
      Fixes #8351, #8406
      
      (cherry picked from commit 3a504904)
      dcf409f8
    • Yossi Gottlieb's avatar
      CONFIG REWRITE should honor umask settings. (#8371) · 1237cce6
      Yossi Gottlieb authored
      Fixes a regression introduced due to a new (safer) way of rewriting configuration files. In the past the file was simply overwritten (same inode), but now Redis creates a new temporary file and later renames it over the old one.
      
      The temp file typically gets created with 0600 permissions so we later fchmod it to fix that. Unlike open with O_CREAT, fchmod doesn't consider umask so we have to do that explicitly.
      
      Fixes #8369
      
      (cherry picked from commit b548ffab)
      1237cce6
    • guybe7's avatar
      Fix firstkey,lastkey,step in COMMAND command for some commands (#8367) · d182820f
      guybe7 authored
      The output for COMMAND command was wrong for some commands.
      clients can use firstkey,lastkey,step to find (some) key name arguments, and the
      "movablekeys" flag to know that they can't know all (or any) of the key name arguments.
      
      These commands had the wrong output:
      1. GEORADIUS*_RO used to have "movablekeys" (which it doesn't really need)
      2. XREAD and XREADGROUP used to have (1,1,1). but that's completely wrong.
      3. Z*STORE used to have (0,0,0) but it can at lest give the index of the dstkey (1,1,1)
      
      (cherry picked from commit baf92f3f)
      d182820f
    • Madelyn Olson's avatar
      Remove race condition and consistency issues with client pause and threaded IO (#8520) · 0c653428
      Madelyn Olson authored
      clientsArePaused isn't thread safe because it has a side effect of attempting to unpause,
      which may cause multiple threads concurrently updating the unblocked_clients global list.
      This change resolves this issue by no longer postponing client for threaded reads when
      clients are paused and then skipping the check for client paused for threaded reads,
      in case one is postponed and then clients are paused. (I don't think this is strictly possible,
      but being defensive seems better here)
      0c653428
  2. 12 Jan, 2021 33 commits
    • Oran Agra's avatar
      Redis 6.0.10 · 8d70d498
      Oran Agra authored
      8d70d498
    • Oran Agra's avatar
      Improve stability of new CSC eviction test (#8160) · 5745b469
      Oran Agra authored
      c4fdf09c added a test that now fails with valgrind
      it fails for two resons:
      1) the test samples the used memory and then limits the maxmemory to
         that value, but it turns out this is not atomic and on slow machines
         the background cron process that clean out old query buffers reduces
         the memory so that the setting doesn't cause eviction.
      2) the dbsize was tested late, after reading some invalidation messages
         by that time more and more keys got evicted, partially draining the
         db. this is not the focus of this fix (still a known limitation)
      
      (cherry picked from commit a102b21d)
      5745b469
    • Oran Agra's avatar
      fix race in cluster transactions test (#8312) · a11c842e
      Oran Agra authored
      we didn't wait for the commands executed on the master to reach the replica.
      
      (cherry picked from commit 4f8458d8)
      a11c842e
    • Oran Agra's avatar
      Fix cluster diskless load swapdb test (#8308) · 615eb0db
      Oran Agra authored
      The test was trying to wait for the replica to start loading the rdb
      from the master before it kills the master, but it was actually waiting
      for ROLE to be in "sync" mode, which corresponds to REPL_STATE_TRANSFER
      that starts before the actual loading starts.
      now instead it waits for the loading flag to be set.
      
      Besides, the test was dependent on the previous configuration of the
      servers, relying on the fact the replica is configured to persist
      (either RDB of AOF), now it is set explicitly.
      
      (cherry picked from commit 26495387)
      615eb0db
    • Bob Li's avatar
      Fix overflow of rdbWriteRaw return value (#8306) · 203feb6e
      Bob Li authored
      Saving string of more than 2GB to the RDB file, can result in corrupt RDB, or failure in rdbSave.
      S
      
      (cherry picked from commit 542455ce)
      203feb6e
    • guybe7's avatar
      Modules: Fix an integer sign bug in moduleTimerHandler (#8131) · 08c3f0bc
      guybe7 authored
      bug was introduced in 1a91a270
      
      (cherry picked from commit 2f41a385)
      08c3f0bc
    • Egor Seredin's avatar
      Allow '\0' inside of result of sdscatvprintf, and efficiency improvements (#6260) · 3bf02ea7
      Egor Seredin authored
      This will allow to use: RedisModule_CreateStringPrintf(ctx, "%s %c %s", "string1", 0, "string2");
      
      On large string, the previous code would incrementally retry to double the output buffer.
      now it uses the the return value of snprintf and grows to the right size in one step.
      
      and also avoids an excessive strlen in sdscat at the end.
      
      (cherry picked from commit f4ca3d87)
      3bf02ea7
    • guybe7's avatar
      Modules: Improve timer accuracy (#7987) · acb4cd21
      guybe7 authored
      The bug occurs when 'callback' re-registers itself to a point
      in the future and the execution time in non-negligible:
      'now' refers to time BEFORE callback was executed and is used
      to calculate 'next_period'.
      We must get the actual current time when calculating 'next_period'
      
      (cherry picked from commit 1a91a270)
      acb4cd21
    • Meir Shpilraien (Spielrein)'s avatar
      Moved RMAPI_FUNC_SUPPORTED location such that it will be visible to modules (#8037) · fb86ecb8
      Meir Shpilraien (Spielrein) authored
      The RMAPI_FUNC_SUPPORTED was defined in the wrong place on redismodule.h
      and was not visible to modules.
      
      (cherry picked from commit 97d647a1)
      fb86ecb8
    • David CARLIER's avatar
      raspberry build fix. (#8095) · 2c5865cd
      David CARLIER authored
      __ILP32__ is 32 bits ABI and does not imply x86, this patch resolves this. 
      
      (cherry picked from commit 0719388c)
      2c5865cd
    • Yossi Gottlieb's avatar
      Fix crash log output on ARM. (#8020) · ebbcd05e
      Yossi Gottlieb authored
      
      (cherry picked from commit 7e4325cb)
      ebbcd05e
    • Oran Agra's avatar
      Fix cluster access to unaligned memory (SIGBUS on old ARM) (#7958) · de4f04a2
      Oran Agra authored
      Turns out this was broken since version 4.0 when we added sds size
      classes.
      The cluster code uses sds for the receive buffer, and then casts it to a
      struct and accesses a 64 bit variable.
      This commit replaces the use of sds with a simple reallocated buffer.
      
      (cherry picked from commit 380f6048)
      de4f04a2
    • Oran Agra's avatar
      prevent client tracking from causing feedback loop in performEvictions (#8100) · f2f57eb4
      Oran Agra authored
      When client tracking is enabled signalModifiedKey can increase memory usage,
      this can cause the loop in performEvictions to keep running since it was measuring
      the memory usage impact of signalModifiedKey.
      
      The section that measures the memory impact of the eviction should be just on dbDelete,
      excluding keyspace notification, client tracking, and propagation to AOF and replicas.
      
      This resolves part of the problem described in #8069
      p.s. fix took 1 minute, test took about 3 hours to write.
      
      (cherry picked from commit c4fdf09c)
      f2f57eb4
    • Madelyn Olson's avatar
      White space tweaks and skip categories already applied · 8119259b
      Madelyn Olson authored
      (cherry picked from commit d310beb4)
      8119259b
    • Madelyn Olson's avatar
      Further improved ACL algorithm for picking categories · e664f381
      Madelyn Olson authored
      (cherry picked from commit 411bcf1a)
      e664f381
    • Yang Bodong's avatar
      Swapdb should make transaction fail if there is any client watching keys (#8239) · f464cf23
      Yang Bodong authored
      
      
      This PR not only fixes the problem that swapdb does not make the
      transaction fail, but also optimizes the FLUSHALL and FLUSHDB command to
      set the CLIENT_DIRTY_CAS flag to avoid unnecessary traversal of clients.
      
      FLUSHDB was changed to first iterate on all watched keys, and then on the
      clients watching each key.
      Instead of iterating though all clients, and for each iterate on watched keys.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit 10f94b0a)
      f464cf23
    • Itamar Haber's avatar
      Adds 'use-memory' to GEORADIUS[BYMEMBER] (#8107) · 02c395a1
      Itamar Haber authored
      Partial resolution for #6860, item 7
      
      (cherry picked from commit feba7cbf)
      02c395a1
    • Oran Agra's avatar
      fix valgrind warning created by recent pidfile fix (#8235) · acdccab0
      Oran Agra authored
      This isn't a leak, just an warning due to unreachable
      allocation on the fork child.
      Problem created by 92a483bc
      
      (cherry picked from commit 2426aaa0)
      acdccab0
    • Meir Shpilraien (Spielrein)'s avatar
      Fix issue where fork process deletes the parent pidfile (#8231) · e9f674e5
      Meir Shpilraien (Spielrein) authored
      Turns out that when the fork child crashes, the crash log was deleting
      the pidfile from the disk (although the parent is still running.
      
      Now we set the pidfile of the fork process to NULL so the fork process
      will never deletes it.
      
      (cherry picked from commit 92a483bc)
      e9f674e5
    • Wen Hui's avatar
      0f8d47c9
    • Wen Hui's avatar
      fix memory leak in processInlineBuffer error handling code (#8295) · f1d56c98
      Wen Hui authored
      This code path is normally executed only when v6.0 and above replicates from v2.4
      
      (cherry picked from commit cfcd0fa6)
      f1d56c98
    • huangzhw's avatar
      sdscatfmt call sdsMakeRoomFor, asked for more space than intended (#8286) · ea595784
      huangzhw authored
      instead of asking for the extra new space it wanted, it asked to grow the
      string by the size it already has too.
      i.e. a string of 1000 bytes, needing to grow by 10 bytes, would have been
      asking for an **additional** 1010 bytes.
      
      (cherry picked from commit 595ecd5f)
      ea595784
    • huangzhw's avatar
      sort Command lookupKeyRead and lookupKeyWrite are used on the opposite (#8283) · 549fe1f3
      huangzhw authored
      This is a recent problem, introduced by 74717438 (redis 6.0)
      
      The implications are:
      The sole difference between LookupKeyRead and LookupKeyWrite is for command
      executed on a replica, which are not received from its master client. (for the master,
      and for the master client on the replica, these two functions behave the same)!
      
      Since SORT is a write command, this bug only implicates a writable-replica.
      And these are its implications:
      
      - SORT STORE will behave as it did before the above mentioned commit (like before
        redis 6.0). on a writable-replica an already logically expired the key would have
        appeared missing. (store dest key would be deleted, instead of being populated
        with the data from the already logically expired key)
      - SORT (the non store variant, which in theory could have been executed on
        read-only-replica if it weren't for the write flag), will (in redis 6.0) have a new bug
        and return the data from the already logically expired key instead of empty response.
      
      (cherry picked from commit 08ad6abd)
      549fe1f3
    • Oran Agra's avatar
      Fix rdb checksum / crc64 on bigendian (#8270) · cf6b0f9c
      Oran Agra authored
      Turns out the RDB checksum in Redis 6.0 on bigendian is broken.
      It always returned 0, so the RDB files are generated as if checksum is
      disabled, and will be loaded ok on littleendian, and on bigendian.
      But it'll not be able to load RDB files generated on littleendian or older versions.
      
      Similarly DUMP and RESTORE will work on the same version (0==0),
      but will be unable to exchange dump payloads with littleendian or old versions.
      
      (cherry picked from commit 324070c8)
      cf6b0f9c
    • Oran Agra's avatar
      fix crash in redis-cli after making cluster backup (#8267) · 02e93062
      Oran Agra authored
      getRDB is "designed" to work in two modes: one for redis-cli --rdb and
      one for redis-cli --cluster backup.
      in the later case it uses the hiredis connection from the cluster nodes
      and it used to free it without nullifying the context, so a later
      attempt to free the context would crash.
      
      I suppose the reason it seems to want to free the hiredis context ASAP
      is that it wants to disconnect the replica link, so that replication
      buffers will not be accumulated.
      
      (cherry picked from commit 41b2ed2b)
      02e93062
    • Oran Agra's avatar
      Fix wrong order of key/value in Lua map response (#8266) · ec56906b
      Oran Agra authored
      When a Lua script returns a map to redis (a feature which was added in
      redis 6 together with RESP3), it would have returned the value first and
      the key second.
      
      If the client was using RESP2, it was getting them out of order, and if
      the client was in RESP3, it was getting a map of value => key.
      This was happening regardless of the Lua script using redis.setresp(3)
      or not.
      
      This also affects a case where the script was returning a map which it got
      from from redis by doing something like: redis.setresp(3); return redis.call()
      
      This fix is a breaking change for redis 6.0 users who happened to rely
      on the wrong order (either ones that used redis.setresp(3), or ones that
      returned a map explicitly).
      
      This commit also includes other two changes in the tests:
      1. The test suite now handles RESP3 maps as dicts rather than nested
         lists
      2. Remove some redundant (duplicate) tests from tracking.tcl
      
      (cherry picked from commit 2017407b)
      ec56906b
    • Oran Agra's avatar
      Crash log would crash half way on commands with no arguments (#8260) · 23d29923
      Oran Agra authored
      The crash log attempts to print the current client info, and when it
      does that it attempts to check if the first argument happens to be a key
      but it did so for commands with no arguments too, which caused the crash
      log to crash half way and not reach its end.
      
      (cherry picked from commit 152b5d46)
      23d29923
    • Yossi Gottlieb's avatar
      Fix crashes with io-threads-do-reads enabled. (#8230) · f84c181e
      Yossi Gottlieb authored
      Normally IO threads should simply read data from the socket into the
      buffer and attempt to parse it.
      
      If a protocol error is detected, a reply is generated which may result
      with installing a write handler which is not thread safe. This fix
      delays that until the client is processed back in the main thread.
      
      Fixes #8220
      
      (cherry picked from commit e7047ec2)
      f84c181e
    • George Prekas's avatar
      Add check for the MADV_FREE/fork arm64 Linux kernel bug (#8224) · 8ff02263
      George Prekas authored
      
      
      Older arm64 Linux kernels have a bug that could lead to data corruption during
      background save under the following scenario:
      
      1) jemalloc uses MADV_FREE on a page,
      2) jemalloc reuses and writes the page,
      3) Redis forks the background save process, and
      4) Linux performs page reclamation.
      
      Under these conditions, Linux will reclaim the page wrongfully and the
      background save process will read zeros when it tries to read the page.
      
      The bug has been fixed in Linux with commit:
      ff1712f953e27f0b0718762ec17d0adb15c9fd0b ("arm64: pgtable: Ensure dirty bit is
      preserved across pte_wrprotect()")
      
      This Commit adds an ignore-warnings config, when not found, redis will
      print a warning and exit on startup (default behavior).
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit b02780c4)
      
      in 6.0 this warning is ignored by default in order to avoid adding
      regression, specifically for deployments that don't need persistence or
      replication
      8ff02263
    • Qu Chen's avatar
      Not over-allocate client query buffer when reading large objects. (#5954) · d756b11f
      Qu Chen authored
      In response to large client query buffer optimization introduced in 1898e6ce. The calculation of the amount of
      remaining bytes we need to write to the query buffer was calculated wrong, as a result we are unnecessarily
      growing the client query buffer by sdslen(c->querybuf) always. This fix corrects that behavior.
      
      Please note the previous behavior prior to the before-mentioned change was correctly calculating the remaining
      additional bytes, and this change makes that calculate to be consistent.
      
      Useful context, the argument of size `ll` starts at qb_pos (which is now the beginning of the sds), but much of it
      may have already been read from the socket, so we only need to grow the sds for the remainder of it.
      
      (cherry picked from commit 11b3325e)
      d756b11f
    • Oran Agra's avatar
      Handle output buffer limits for Module blocked clients (#8141) · ec74ae7e
      Oran Agra authored
      Module blocked clients cache the response in a temporary client,
      the reply list in this client would be affected by the recent fix
      in #7202, but when the reply is later copied into the real client,
      it would have bypassed all the checks for output buffer limit, which
      would have resulted in both: responding with a partial response to
      the client, and also not disconnecting it at all.
      
      (cherry picked from commit 48efc25f)
      ec74ae7e
    • Yossi Gottlieb's avatar
      Fix setproctitle related crashes. (#8150) · 785851a7
      Yossi Gottlieb authored
      Makes spt_init more careful with assumptions about what memory regions
      may be overwritten. It will now only consider a contiguous block of argv
      and envp elements and mind any gaps.
      
      (cherry picked from commit ec02c761)
      785851a7
    • Yossi Gottlieb's avatar
      Fix use-after-free issue in spt_copyenv. (#8088) · 2a2d5a7a
      Yossi Gottlieb authored
      Seems to have gone unnoticed for a long time, because at least with
      glibc it will only be triggered if setenv() was called before spt_init,
      which Redis doesn't.
      
      Fixes #8064.
      
      (cherry picked from commit 7e5a6313)
      2a2d5a7a