1. 24 Dec, 2020 2 commits
  2. 23 Dec, 2020 1 commit
    • Yang Bodong's avatar
      Tests: fix the problem that Darwin memory leak detection may fail (#8213) · ee59dc1b
      Yang Bodong authored
      Apparently the "leaks" took reports a different error string about process
      that's not found in each version of MacOS.
      This cause the test suite to fail on some OS versions, since some tests terminate
      the process before looking for leaks.
      Instead of looking at the error string, we now look at the (documented) exit code.
      ee59dc1b
  3. 22 Dec, 2020 1 commit
    • Oran Agra's avatar
      Remove read-only flag from non-keyspace cmds, different approach for EXEC to... · 411c18bb
      Oran Agra authored
      Remove read-only flag from non-keyspace cmds, different approach for EXEC to propagate MULTI (#8216)
      
      In the distant history there was only the read flag for commands, and whatever
      command that didn't have the read flag was a write one.
      Then we added the write flag, but some portions of the code still used !read
      Also some commands that don't work on the keyspace at all, still have the read
      flag.
      
      Changes in this commit:
      1. remove the read-only flag from TIME, ECHO, ROLE and LASTSAVE
      
      2. EXEC command used to decides if it should propagate a MULTI by looking at
         the command flags (!read & !admin).
         When i was about to change it to look at the write flag instead, i realized
         that this would cause it not to propagate a MULTI for PUBLISH, EVAL, and
         SCRIPT, all 3 are not marked as either a read command or a write one (as
         they should), but all 3 are calling forceCommandPropagation.
      
         So instead of introducing a new flag to denote a command that "writes" but
         not into the keyspace, and still needs propagation, i decided to rely on
         the forceCommandPropagation, and just fix the code to propagate MULTI when
         needed rather than depending on the command flags at all.
      
         The implication of my change then is that now it won't decide to propagate
         MULTI when it sees one of these: SELECT, PING, INFO, COMMAND, TIME and
         other commands which are neither read nor write.
      
      3. Changing getNodeByQuery and clusterRedirectBlockedClientIfNeeded in
         cluster.c to look at !write rather than read flag.
         This should have no implications, since these code paths are only reachable
         for commands which access keys, and these are always marked as either read
         or write.
      
      This commit improve MULTI propagation tests, for modules and a bunch of
      other special cases, all of which used to pass already before that commit.
      the only one that test change that uncovered a change of behavior is the
      one that DELs a non-existing key, it used to propagate an empty
      multi-exec block, and no longer does.
      411c18bb
  4. 17 Dec, 2020 1 commit
  5. 14 Dec, 2020 3 commits
  6. 13 Dec, 2020 3 commits
    • Yossi Gottlieb's avatar
      Several (mostly Solaris-related) cleanups (#8171) · 86e3395c
      Yossi Gottlieb authored
      * Allow runtest-moduleapi use a different 'make', for systems where GNU Make is 'gmake'.
      * Fix issue with builds on Solaris re-building everything from scratch due to CFLAGS/LDFLAGS not stored.
      * Fix compile failure on Solaris due to atomicvar and a bunch of warnings.
      * Fix garbled log timestamps on Solaris.
      86e3395c
    • Oran Agra's avatar
      Add module event for repl-diskless-load swapdb (#8153) · ab60dcf5
      Oran Agra authored
      When a replica uses the diskless-load swapdb approach, it backs up the old database,
      then attempts to load a new one, and in case of failure, it restores the backup.
      
      this means that modules with global out of keyspace data, must have an option to
      subscribe to events and backup/restore/discard their global data too.
      ab60dcf5
    • Yossi Gottlieb's avatar
      Modules: add defrag API support. (#8149) · 63c1303c
      Yossi Gottlieb authored
      Add a new set of defrag functions that take a defrag context and allow
      defragmenting memory blocks and RedisModuleStrings.
      
      Modules can register a defrag callback which will be invoked when the
      defrag process handles globals.
      
      Modules with custom data types can also register a datatype-specific
      defrag callback which is invoked for keys that require defragmentation.
      The callback and associated functions support both one-step and
      multi-step options, depending on the complexity of the key as exposed by
      the free_effort callback.
      63c1303c
  7. 12 Dec, 2020 1 commit
    • 杨博东's avatar
      Add GEOSEARCH / GEOSEARCHSTORE commands (#8094) · 4d06d99b
      杨博东 authored
      Add commands to query geospatial data with bounding box.
      
      Two new commands that replace the existing 4 GEORADIUS* commands.
      
      GEOSEARCH key [FROMMEMBER member] [FROMLOC long lat] [BYRADIUS radius
      unit] [BYBOX width height unit] [WITHCORD] [WITHDIST] [WITHASH] [COUNT
      count] [ASC|DESC]
      
      GEOSEARCHSTORE dest_key src_key [FROMMEMBER member] [FROMLOC long lat]
      [BYRADIUS radius unit] [BYBOX width height unit] [WITHCORD] [WITHDIST]
      [WITHASH] [COUNT count] [ASC|DESC] [STOREDIST]
      
      - Add two types of CIRCULAR_TYPE and RECTANGLE_TYPE to achieve different searches
      - Judge whether the point is within the rectangle, refer to:
      geohashGetDistanceIfInRectangle
      4d06d99b
  8. 11 Dec, 2020 1 commit
    • Yossi Gottlieb's avatar
      TLS: Add different client cert support. (#8076) · 8c291b97
      Yossi Gottlieb authored
      This adds a new `tls-client-cert-file` and `tls-client-key-file`
      configuration directives which make it possible to use different
      certificates for the TLS-server and TLS-client functions of Redis.
      
      This is an optional directive. If it is not specified the `tls-cert-file`
      and `tls-key-file` directives are used for TLS client functions as well.
      
      Also, `utils/gen-test-certs.sh` now creates additional server-only and client-only certs and will skip intensive operations if target files already exist.
      8c291b97
  9. 09 Dec, 2020 1 commit
    • Yossi Gottlieb's avatar
      Add module data-type support for COPY. (#8112) · 4e064fba
      Yossi Gottlieb authored
      This adds a copy callback for module data types, in order to make
      modules compatible with the new COPY command.
      
      The callback is optional and COPY will fail for keys with data types
      that do not implement it.
      4e064fba
  10. 08 Dec, 2020 5 commits
  11. 07 Dec, 2020 2 commits
  12. 06 Dec, 2020 9 commits
    • Oran Agra's avatar
      Sanitize dump payload: fail RESTORE if memory allocation fails · 7ca00d69
      Oran Agra authored
      When RDB input attempts to make a huge memory allocation that fails,
      RESTORE should fail gracefully rather than die with panic
      7ca00d69
    • Oran Agra's avatar
      Sanitize dump payload: validate no duplicate records in hash/zset/intset · 3716950c
      Oran Agra authored
      If RESTORE passes successfully with full sanitization, we can't affort
      to crash later on assertion due to duplicate records in a hash when
      converting it form ziplist to dict.
      This means that when doing full sanitization, we must make sure there
      are no duplicate records in any of the collections.
      3716950c
    • Oran Agra's avatar
      testsuite: fix fd leak, prevent port clashing when using --baseport · 5b446313
      Oran Agra authored
      when using --baseport to run two tests suite in parallel (different
      folders), we need to also make sure the port used by the testsuite to
      communicate with it's workers is unique. otherwise the attept to find a
      free port connects to the other test suite and messes it.
      
      maybe one day we need to attempt to bind, instead of connect when tring
      to find a free port.
      5b446313
    • Oran Agra's avatar
      Sanitize dump payload: fuzz tester and fixes for segfaults and leaks it exposed · c31055db
      Oran Agra authored
      The test creates keys with various encodings, DUMP them, corrupt the payload
      and RESTORES it.
      It utilizes the recently added use-exit-on-panic config to distinguish between
       asserts and segfaults.
      If the restore succeeds, it runs random commands on the key to attempt to
      trigger a crash.
      
      It runs in two modes, one with deep sanitation enabled and one without.
      In the first one we don't expect any assertions or segfaults, in the second one
      we expect assertions, but no segfaults.
      We also check for leaks and invalid reads using valgrind, and if we find them
      we print the commands that lead to that issue.
      
      Changes in the code (other than the test):
      - Replace a few NPD (null pointer deference) flows and division by zero with an
        assertion, so that it doesn't fail the test. (since we set the server to use
        `exit` rather than `abort` on assertion).
      - Fix quite a lot of flows in rdb.c that could have lead to memory leaks in
        RESTORE command (since it now responds with an error rather than panic)
      - Add a DEBUG flag for SET-SKIP-CHECKSUM-VALIDATION so that the test don't need
        to bother with faking a valid checksum
      - Remove a pile of code in serverLogObjectDebugInfo which is actually unsafe to
        run in the crash report (see comments in the code)
      - fix a missing boundary check in lzf_decompress
      
      test suite infra improvements:
      - be able to run valgrind checks before the process terminates
      - rotate log files when restarting servers
      c31055db
    • Oran Agra's avatar
      Sanitize dump payload: improve tests of ziplist and stream encodings · 01c13bdd
      Oran Agra authored
      - improve stream rdb encoding test to include more types of stream metadata
      - add test to cover various ziplist encoding entries (although it does
        look like the stress test above it is able to find some too
      - add another test for ziplist encoding for hash with full sanitization
      - add similar ziplist encoding tests for list
      01c13bdd
    • Oran Agra's avatar
      Sanitize dump payload: ziplist, listpack, zipmap, intset, stream · ca1c1825
      Oran Agra authored
      When loading an encoded payload we will at least do a shallow validation to
      check that the size that's encoded in the payload matches the size of the
      allocation.
      This let's us later use this encoded size to make sure the various offsets
      inside encoded payload don't reach outside the allocation, if they do, we'll
      assert/panic, but at least we won't segfault or smear memory.
      
      We can also do 'deep' validation which runs on all the records of the encoded
      payload and validates that they don't contain invalid offsets. This lets us
      detect corruptions early and reject a RESTORE command rather than accepting
      it and asserting (crashing) later when accessing that payload via some command.
      
      configuration:
      - adding ACL flag skip-sanitize-payload
      - adding config sanitize-dump-payload [yes/no/clients]
      
      For now, we don't have a good way to ensure MIGRATE in cluster resharding isn't
      being slowed down by these sanitation, so i'm setting the default value to `no`,
      but later on it should be set to `clients` by default.
      
      changes:
      - changing rdbReportError not to `exit` in RESTORE command
      - adding a new stat to be able to later check if cluster MIGRATE isn't being
        slowed down by sanitation.
      ca1c1825
    • Oran Agra's avatar
      prevent client tracking from causing feedback loop in performEvictions (#8100) · c4fdf09c
      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.
      c4fdf09c
    • guybe7's avatar
      Make sure we do not propagate nested MULTI/EXEC (#8097) · 1df5bb56
      guybe7 authored
      One way this was happening is when a module issued an RM_Call which would inject MULTI.
      If the module command that does that was itself issued by something else that already did
      added MULTI (e.g. another module, or a Lua script), it would have caused nested MULTI.
      
      In fact the MULTI state in the client or the MULTI_EMITTED flag in the context isn't
      the right indication that we need to propagate MULTI or not, because on a nested calls
      (possibly a module action called by a keyspace event of another module action), these
      flags aren't retained / reflected.
      
      instead there's now a global propagate_in_transaction flag for that.
      
      in addition to that, we now have a global in_eval and in_exec flags, to serve the flags
      of RM_GetContextFlags, since their dependence on the current client is wrong for the same
      reasons mentioned above.
      1df5bb56
    • Wang Yuan's avatar
      Limit the main db and expires dictionaries to expand (#7954) · 75f9dec6
      Wang Yuan authored
      As we know, redis may reject user's requests or evict some keys if
      used memory is over maxmemory. Dictionaries expanding may make
      things worse, some big dictionaries, such as main db and expires dict,
      may eat huge memory at once for allocating a new big hash table and be
      far more than maxmemory after expanding.
      There are related issues: #4213 #4583
      
      More details, when expand dict in redis, we will allocate a new big
      ht[1] that generally is double of ht[0], The size of ht[1] will be
      very big if ht[0] already is big. For db dict, if we have more than
      64 million keys, we need to cost 1GB for ht[1] when dict expands.
      
      If the sum of used memory and new hash table of dict needed exceeds
      maxmemory, we shouldn't allow the dict to expand. Because, if we
      enable keys eviction, we still couldn't add much more keys after
      eviction and rehashing, what's worse, redis will keep less keys when
      redis only remains a little memory for storing new hash table instead
      of users' data. Moreover users can't write data in redis if disable
      keys eviction.
      
      What this commit changed ?
      
      Add a new member function expandAllowed for dict type, it provide a way
      for caller to allow expand or not. We expose two parameters for this
      function: more memory needed for expanding and dict current load factor,
      users can implement a function to make a decision by them.
      For main db dict and expires dict type, these dictionaries may be very
      big and cost huge memory for expanding, so we implement a judgement
      function: we can stop dict to expand provisionally if used memory will
      be over maxmemory after dict expands, but to guarantee the performance
      of redis, we still allow dict to expand if dict load factor exceeds the
      safe load factor.
      Add test cases to verify we don't allow main db to expand when left
      memory is not enough, so that avoid keys eviction.
      
      Other changes:
      
      For new hash table size when expand. Before this commit, the size is
      that double used of dict and later _dictNextPower. Actually we aim to
      control a dict load factor between 0.5 and 1.0. Now we replace *2 with
      +1, since the first check is that used >= size, the outcome of before
      will usually be the same as _dictNextPower(used+1). The only case where
      it'll differ is when dict_can_resize is false during fork, so that later
      the _dictNextPower(used*2) will cause the dict to jump to *4 (i.e.
      _dictNextPower(1025*2) will return 4096).
      Fix rehash test cases due to changing algorithm of new hash table size
      when expand.
      75f9dec6
  13. 03 Dec, 2020 1 commit
  14. 02 Dec, 2020 3 commits
  15. 01 Dec, 2020 3 commits
    • Madelyn Olson's avatar
      Getset fix (#8118) · 69b7113b
      Madelyn Olson authored
      
      
      * Fixed SET GET executing on wrong type
      Co-authored-by: default avatarMadelyn Olson <madelyneolson@gmail.com>
      69b7113b
    • sundb's avatar
      Improve dbid range check for SELECT, MOVE, COPY (#8085) · 3ba2281f
      sundb authored
      SELECT used to read the index into a `long` variable, and then pass it to a function
      that takes an `int`, possibly causing an overflow before the range check.
      
      Now all these commands use better and cleaner range check, and that also results in
      a slight change of the error response in case of an invalid database index.
      
      SELECT:
      in the past it would have returned either `-ERR invalid DB index` (if not a number),
      or `-ERR DB index is out of range` (if not between 1..16 or alike).
      now it'll return either `-ERR value is out of range` (if not a number), or
      `-ERR value is out of range, value must between -2147483648 and 2147483647`
      (if not in the range for an int), or `-ERR DB index is out of range`
      (if not between 0..16 or alike)
      
      
      MOVE:
      in the past it would only fail with `-ERR index out of range` no matter the reason.
      now return the same errors as the new ones for SELECT mentioned above.
      (i.e. unlike for SELECT even for a value like 17 we changed the error message)
      
      COPY:
      doesn't really matter how it behaved in the past (new command), new behavior is
      like the above two.
      3ba2281f
    • Itamar Haber's avatar
      Adds pub/sub channel patterns to ACL (#7993) · c1b1e8c3
      Itamar Haber authored
      Fixes #7923.
      
      This PR appropriates the special `&` symbol (because `@` and `*` are taken),
      followed by a literal value or pattern for describing the Pub/Sub patterns that
      an ACL user can interact with. It is similar to the existing key patterns
      mechanism in function (additive) and implementation (copy-pasta). It also adds
      the allchannels and resetchannels ACL keywords, naturally.
      
      The default user is given allchannels permissions, whereas new users get
      whatever is defined by the acl-pubsub-default configuration directive. For
      backward compatibility in 6.2, the default of this directive is allchannels but
      this is likely to be changed to resetchannels in the next major version for
      stronger default security settings.
      
      Unless allchannels is set for the user, channel access permissions are checked
      as follows :
      * Calls to both PUBLISH and SUBSCRIBE will fail unless a pattern matching the
        argumentative channel name(s) exists for the user.
      * Calls to PSUBSCRIBE will fail unless the pattern(s) provided as an argument
        literally exist(s) in the user's list.
      
      Such failures are logged to the ACL log.
      
      Runtime changes to channel permissions for a user with existing subscribing
      clients cause said clients to disconnect unless the new permissions permit the
      connections to continue. Note, however, that PSUBSCRIBErs' patterns are matched
      literally, so given the change bar:* -> b*, pattern subscribers to bar:* will be
      disconnected.
      
      Notes/questions:
      * UNSUBSCRIBE, PUNSUBSCRIBE and PUBSUB remain unprotected due to lack of reasons
        for touching them.
      c1b1e8c3
  16. 29 Nov, 2020 1 commit
    • guybe7's avatar
      XPENDING with IDLE (#7972) · ada2ac9a
      guybe7 authored
      Used to filter stream pending entries by their idle-time,
      useful for XCLAIMing entries that have not been processed
      for some time
      ada2ac9a
  17. 23 Nov, 2020 1 commit
  18. 18 Nov, 2020 1 commit
    • guybe7's avatar
      EXISTS should not alter LRU, OBJECT should not reveal expired keys on replica (#8016) · f8ae9917
      guybe7 authored
      The bug was introduced by #5021 which only attempted avoid EXIST on an
      already expired key from returning 1 on a replica.
      
      Before that commit, dbExists was used instead of
      lookupKeyRead (which had an undesired effect to "touch" the LRU/LFU)
      
      Other than that, this commit fixes OBJECT to also come empty handed on
      expired keys in replica.
      
      And DEBUG DIGEST-VALUE to behave like DEBUG OBJECT (get the data from
      the key regardless of it's expired state)
      f8ae9917