1. 18 Nov, 2021 7 commits
    • Wen Hui's avatar
      75d50e5d
    • perryitay's avatar
      Fix crashes when list-compress-depth is used. (#9779) · 0c10f0e1
      perryitay authored
      Recently we started using list-compress-depth in tests (was completely untested till now).
      Turns this triggered test failures with the external mode, since the tests left the setting enabled
      and then it was used in other tests (specifically the fuzzer named "Stress tester for #3343-alike bugs").
      
      This PR fixes the issue of the `recompress` flag being left set by mistake, which caused the code to
      later to compress the head or tail nodes (which should never be compressed)
      
      The solution is to reset the recompress flag when it should have been (when it was decided not to compress).
      
      Additionally we're adding some assertions and improve the tests so in order to catch other similar bugs.
      0c10f0e1
    • Binbin's avatar
      Add --dump-logs to ci tests and daily tests (#9803) · 020092e0
      Binbin authored
      So that we can print server log on test failure.
      020092e0
    • Eduardo Semprebon's avatar
      Reject PING with MASTERDOWN when replica-serve-stale-data=no (#9757) · 1a255e31
      Eduardo Semprebon authored
      Currently PING returns different status when server is not serving data,
      for example when `LOADING` or `BUSY`.
      But same was not true for `MASTERDOWN`
      This commit makes PING reply with `MASTERDOWN` when
      replica-serve-stale-data=no and link is MASTER is down.
      1a255e31
    • guybe7's avatar
      Obliterate STRALGO! add LCS (which only works on keys) (#9799) · af748988
      guybe7 authored
      Drop the STRALGO command, now LCS is a command of its own and it only works on keys (not input strings).
      The motivation is that STRALGO's syntax was really messed-up...
      - assumes all (future) string algorithms will take similar arguments
      - mixes command that takes keys and one that doesn't in the same command.
      - make it nearly impossible to expose the right key spec in COMMAND INFO (issues cluster clients)
      - hard for cluster clients to determine the key names (firstkey, lastkey, etc)
      - hard for ACL / flags (is it a read command?)
      
      This is a breaking change.
      af748988
    • Binbin's avatar
      Fixes ZPOPMIN/ZPOPMAX wrong replies when count is 0 with non-zset (#9711) · 91e77a0c
      Binbin authored
      Moves ZPOP ... 0 fast exit path after type check to reply with
      WRONGTYPE. In the past it will return an empty array.
      
      Also now count is not allowed to be negative.
      
      see #9680
      
      before:
      ```
      127.0.0.1:6379> set zset str
      OK
      127.0.0.1:6379> zpopmin zset 0
      (empty array)
      127.0.0.1:6379> zpopmin zset -1
      (empty array)
      ```
      
      after:
      ```
      127.0.0.1:6379> set zset str
      OK
      127.0.0.1:6379> zpopmin zset 0
      (error) WRONGTYPE Operation against a key holding the wrong kind of value
      127.0.0.1:6379> zpopmin zset -1
      (error) ERR value is out of range, must be positive
      ```
      91e77a0c
    • Madelyn Olson's avatar
      Unpause clients earlier during manual cluster failover (#9676) · 32215e78
      Madelyn Olson authored
      Unpause clients after manual failover ends instead of the timed offset
      32215e78
  2. 16 Nov, 2021 6 commits
    • Oran Agra's avatar
      Daily tests flags inconsistency (#9792) · 45144fc7
      Oran Agra authored
      Add --accurate to unit tests (new feature recently added)
      Add --no-latency to valgrind run (was present only for modules)
      add --no-latency to macos and freebsd runs (was not present for modules)
      add --timeout to freebsd (same one we have for valgrind)
      45144fc7
    • Axlgrep's avatar
      optimze the judgment logic of use_pattern in scanGenericCommand() (#9789) · 5f8ee381
      Axlgrep authored
      in `scan 0 match ""` case,  pat is empty sds(patlen is 0), I don't think should access the
      first character directly in this case(even though the first character is ' \0 '), for the
      code readability, I switch the two positions of judgment logic. 
      5f8ee381
    • harleyliao's avatar
      set aof rewrite status err, when fork fail (#5606) · 992a610f
      harleyliao authored
      when aof rewrite is failed by fork(), It'll be indicated by aof_last_bgrewrite_status
      INFO field, same as when the fork child fails later on.
      992a610f
    • sundb's avatar
      Change lzf to handle values larger than UINT32_MAX (#9776) · 985430b4
      sundb authored
      Redis supports inserting data over 4GB into string (and recently for lists too, see #9357),
      But LZF compression used in RDB files (see `rdbcompression` config), and in quicklist
      (see `list-compress-depth` config) does not support compress/decompress data over
      UINT32_MAX, which will result in corrupting the rdb after compression.
      
      Internal changes:
      1. Modify the `unsigned int` parameter of `lzf_compress/lzf_decompress` to `size_t`.
      2. Modify the variable types in `lzf_compress` involving offsets and lengths to `size_t`.
      3. Set LZF_USE_OFFSETS to 0.
          When LZF_USE_OFFSETS is 1, lzf store offset into `LZF_HSLOT`(32bit). 
          Even in 64-bit, `LZF_USE_OFFSETS` defaults to 1, because lzf assumes that it only
          compresses and decompresses data smaller than UINT32_MAX.
          But now we need to make lzf support 64-bit, turning on `LZF_USE_OFFSETS` will make
          it impossible to store 64-bit offsets or pointers.
          BTW, disable LZF_USE_OFFS...
      985430b4
    • sundb's avatar
      Add --large-memory flag for REDIS_TEST to enable tests that consume more than 100mb (#9784) · e725d737
      sundb authored
      This is a preparation step in order to add a new test in quicklist.c see #9776
      e725d737
    • guoxiang1996's avatar
      insufficient size for cached client flags in call() (#9783) · aba70df4
      guoxiang1996 authored
      The client flags is a 64 bit integer, but the temporary cached value on the stack of call() is 32 bit.
      luckily this doesn't lead to any bugs since the only flags used against this variables are below 32 bit.
      aba70df4
  3. 15 Nov, 2021 1 commit
    • yoav-steinberg's avatar
      Connection leak in external tests. (#9777) · e968d9ac
      yoav-steinberg authored
      Two issues:
      1. In many tests we simply forgot to close the connections we created, which doesn't matter for normal tests where the server is killed, but creates a leak on external server tests.
      2. When calling `start_server` on external test we create a fresh connection instead of really starting a new server, but never clean it at the end.
      e968d9ac
  4. 13 Nov, 2021 1 commit
    • Binbin's avatar
      Tune expire test threshold. (#9775) · 174eedce
      Binbin authored
      I have seen this CI failure twice on MacOS:
      
      *** [err]: PEXPIRE/PSETEX/PEXPIREAT can set sub-second expires in tests/unit/expire.tcl
      Expected 'somevalue {} somevalue {} somevalue {}' to equal or match '{} {} {} {} somevalue {}'
      
      I did some loop test in my own daily CI, the results show that is
      not particularly stable. Change the threshold from 30 to 50.
      174eedce
  5. 11 Nov, 2021 4 commits
    • Ozan Tezcan's avatar
      Enable running daily CI from forks (#9771) · bcb7961f
      Ozan Tezcan authored
      Was impossible to run the daily CI from private forks due to "redis/redis" repo check.
      Let's disable that check for manual triggers.
      bcb7961f
    • Ozan Tezcan's avatar
      Add sanitizer support and clean up sanitizer findings (#9601) · b91d8b28
      Ozan Tezcan authored
      - Added sanitizer support. `address`, `undefined` and `thread` sanitizers are available.  
      - To build Redis with desired sanitizer : `make SANITIZER=undefined`
      - There were some sanitizer findings, cleaned up codebase
      - Added tests with address and undefined behavior sanitizers to daily CI.
      - Added tests with address sanitizer to the per-PR CI (smoke out mem leaks sooner).
      
      Basically, there are three types of issues : 
      
      **1- Unaligned load/store** : Most probably, this issue may cause a crash on a platform that
      does not support unaligned access. Redis does unaligned access only on supported platforms.
      
      **2- Signed integer overflow.** Although, signed overflow issue can be problematic time to time
      and change how compiler generates code, current findings mostly about signed shift or simple
      addition overflow. For most platforms Redis can be compiled for, this wouldn't cause any issue
      as far as I can tell (checked generated code on godbolt.org).
      
       **3 -Minor leak** (redis-cli), **use-after-free**(just before calling exit());
      
      UB means nothing guaranteed and risky to reason about program behavior but I don't think any
      of the fixes here worth backporting. As sanitizers are now part of the CI, preventing new issues
      will be the real benefit. 
      b91d8b28
    • yoav-steinberg's avatar
      Archive external redis log in external tests (#9765) · cd6b3d55
      yoav-steinberg authored
      On test failure store the external redis server logs as CI artifacts so we can review them.
      
      Write test name to server log for external server tests.
      This is attempted and silently failed in case external server doesn't support it.
      Note that in non-external server mode we use a more robust method of writing to the log which doesn't depend on the
      server actually running/working. This isn't possible for externl servers and required for some complex tests which are
      skipped in external mode anyway.
      
      Cleanup: remove dup code.
      cd6b3d55
    • Vadim Pushtaev's avatar
      f069d09e
  6. 10 Nov, 2021 2 commits
    • Oran Agra's avatar
      Try solving test timeout on freebsd CI (#9768) · 0927a0dd
      Oran Agra authored
      First, avoid using --accurate on the freebsd CI, we only care about
      systematic issues there due to being different platform, but not
      accuracy
      
      Secondly, when looking at the test which timed out it seems silly and
      outdated:
      - it used KEYS to attempt to trigger lazy expiry, but KEYS doesn't do
        that anymore.
      - it used some hard coded sleeps rather than waiting for things to
        happen and exiting ASAP
      0927a0dd
    • Oran Agra's avatar
      Increase test timeout in valgrind runs (#9767) · 978eadba
      Oran Agra authored
      We saw some tests sporadically time out on valgrind (namely the ones
      from #9323).
      
      Increasing valgrind timeout from 20 mins to 40 mins in CI.
      And fixing an outdated help message.
      978eadba
  7. 09 Nov, 2021 3 commits
  8. 08 Nov, 2021 3 commits
  9. 07 Nov, 2021 2 commits
    • chendianqiang's avatar
      Test suite - user server socket to optimize port detection (#9663) · a527c3e8
      chendianqiang authored
      
      
      Optimized port detection for tcl, use 'socket -server' instead of 'socket' to rule out port on TIME_WAIT
      Co-authored-by: default avatarchendianqiang <chendianqiang@meituan.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      a527c3e8
    • yoav-steinberg's avatar
      Refactor config.c for generic setter interface (#9644) · 79ac5756
      yoav-steinberg authored
      
      
      This refactors all `CONFIG SET`s and conf file loading arguments go through
      the generic config handling interface.
      
      Refactoring changes:
      - All config params go through the `standardConfig` interface (some stuff which
        is only related to the config file and not the `CONFIG` command still has special
        handling for rewrite/config file parsing, `loadmodule`, for example.) .
      - Added `MULTI_ARG_CONFIG` flag for configs to signify they receive a variable
        number of arguments instead of a single argument. This is used to break up space
        separated arguments to `CONFIG SET` so the generic setter interface can pass
        multiple arguments to the setter function. When parsing the config file we also break
        up anything after the config name into multiple arguments to the setter function.
      
      Interface changes:
      - A side effect of the above interface is that the `bind` argument in the config file can
        be empty (no argument at all) this is treated the same as passing an single empty
        string argument (same as `save` already used to work).
      - Support rewrite and setting `watchdog-period` from config file (was only supported
        by the CONFIG command till now).
      - Another side effect is that the `save T X` config argument now supports multiple
        Time-Changes pairs in a single line like its `CONFIG SET` counterpart. So in the
        config file you can either do:
        ```
        save 3600 1
        save 600 10
        ```
        or do
        ```
        save 3600 1 600 10
        ```
      Co-authored-by: default avatarBjorn Svensson <bjorn.a.svensson@est.tech>
      79ac5756
  10. 05 Nov, 2021 1 commit
  11. 04 Nov, 2021 4 commits
    • Eduardo Semprebon's avatar
      Replica keep serving data during repl-diskless-load=swapdb for better availability (#9323) · 91d0c758
      Eduardo Semprebon authored
      
      
      For diskless replication in swapdb mode, considering we already spend replica memory
      having a backup of current db to restore in case of failure, we can have the following benefits
      by instead swapping database only in case we succeeded in transferring db from master:
      
      - Avoid `LOADING` response during failed and successful synchronization for cases where the
        replica is already up and running with data.
      - Faster total time of diskless replication, because now we're moving from Transfer + Flush + Load
        time to Transfer + Load only. Flushing the tempDb is done asynchronously after swapping.
      - This could be implemented also for disk replication with similar benefits if consumers are willing
        to spend the extra memory usage.
      
      General notes:
      - The concept of `backupDb` becomes `tempDb` for clarity.
      - Async loading mode will only kick in if the replica is syncing from a master that has the same
        repl-id the one it had before. i.e. the data it's getting belongs to a different time of the same timeline. 
      - New property in INFO: `async_loading` to differentiate from the blocking loading
      - Slot to Key mapping is now a field of `redisDb` as it's more natural to access it from both server.db
        and the tempDb that is passed around.
      - Because this is affecting replicas only, we assume that if they are not readonly and write commands
        during replication, they are lost after SYNC same way as before, but we're still denying CONFIG SET
        here anyways to avoid complications.
      
      Considerations for review:
      - We have many cases where server.loading flag is used and even though I tried my best, there may
        be cases where async_loading should be checked as well and cases where it shouldn't (would require
        very good understanding of whole code)
      - Several places that had different behavior depending on the loading flag where actually meant to just
        handle commands coming from the AOF client differently than ones coming from real clients, changed
        to check CLIENT_ID_AOF instead.
      
      **Additional for Release Notes**
      - Bugfix - server.dirty was not incremented for any kind of diskless replication, as effect it wouldn't
        contribute on triggering next database SAVE
      - New flag for RM_GetContextFlags module API: REDISMODULE_CTX_FLAGS_ASYNC_LOADING
      - Deprecated RedisModuleEvent_ReplBackup. Starting from Redis 7.0, we don't fire this event.
        Instead, we have the new RedisModuleEvent_ReplAsyncLoad holding 3 sub-events: STARTED,
        ABORTED and COMPLETED.
      - New module flag REDISMODULE_OPTIONS_HANDLE_REPL_ASYNC_LOAD for RedisModule_SetModuleOptions
        to allow modules to declare they support the diskless replication with async loading (when absent, we fall
        back to disk-based loading).
      Co-authored-by: default avatarEduardo Semprebon <edus@saxobank.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      91d0c758
    • Itamar Haber's avatar
      Fixes LPOP/RPOP wrong replies when count is 0 (#9692) · 06dd202a
      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
      06dd202a
    • menwen's avatar
      Retry when a blocked connection system call is interrupted by a signal (#9629) · ccf8a651
      menwen authored
      
      
      When repl-diskless-load is enabled, the connection is set to the blocking state.
      The connection may be interrupted by a signal during a system call.
      This would have resulted in a disconnection and possibly a reconnection loop.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      ccf8a651
    • Oran Agra's avatar
      Fix race condition in cluster test 22-replica-in-sync (#9721) · d04f3069
      Oran Agra authored
      
      
      there was a chance that by the time the assertion is executed,
      the replica already manages to reconnect.
      
      now we make sure the replica is unable to re-connect to the master.
      
      additionally, we wait for some gossip from the disconnected replica,
      to see that it doesn't mess things up.
      
      unrelated: fix a typo when trying to exhaust the backlog, one that
      didn't have any harmful implications
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      d04f3069
  12. 03 Nov, 2021 3 commits
    • perryitay's avatar
      Add support for list type to store elements larger than 4GB (#9357) · f27083a4
      perryitay authored
      
      
      Redis lists are stored in quicklist, which is currently a linked list of ziplists.
      Ziplists are limited to storing elements no larger than 4GB, so when bigger
      items are added they're getting truncated.
      This PR changes quicklists so that they're capable of storing large items
      in quicklist nodes that are plain string buffers rather than ziplist.
      
      As part of the PR there were few other changes in redis: 
      1. new DEBUG sub-commands: 
         - QUICKLIST-PACKED-THRESHOLD - set the threshold of for the node type to
           be plan or ziplist. default (1GB)
         - QUICKLIST <key> - Shows low level info about the quicklist encoding of <key>
      2. rdb format change:
         - A new type was added - RDB_TYPE_LIST_QUICKLIST_2 . 
         - container type (packed / plain) was added to the beginning of the rdb object
           (before the actual node list).
      3. testing:
         - Tests that requires over 100MB will be by default skipped. a new flag was
           added to 'runtest' to run the large memory tests (not used by default)
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      f27083a4
    • guybe7's avatar
      Fix COMMAND GETKEYS on EVAL without keys (#9733) · f11a2d4d
      guybe7 authored
      Add new no-mandatory-keys flag to support COMMAND GETKEYS of commands
      which have no mandatory keys.
      
      In the past we would have got this error:
      ```
      127.0.0.1:6379> command getkeys eval "return 1" 0
      (error) ERR Invalid arguments specified for command
      ```
      f11a2d4d
    • perryitay's avatar
      fix: lookupKey on SETNX and SETXX only once (#9640) · 77d3c6bf
      perryitay authored
      When using SETNX and SETXX we could end up doing key lookup twice.
      This presents a small inefficiency price.
      Also once we have statistics of write hit and miss they'll be wrong (recording the same key hit twice) 
      77d3c6bf
  13. 02 Nov, 2021 3 commits