1. 09 Sep, 2021 3 commits
    • yvette903's avatar
      Fix: client pause uses an old timeout (#9477) · f560531d
      yvette903 authored
      A write request may be paused unexpectedly because `server.client_pause_end_time` is old.
      
      **Recreate this:**
      redis-cli -p 6379
      127.0.0.1:6379> client pause 500000000 write
      OK
      127.0.0.1:6379> client unpause
      OK
      127.0.0.1:6379> client pause 10000 write
      OK
      127.0.0.1:6379> set key value
      
      The write request `set key value` is paused util  the timeout of 500000000 milliseconds was reached.
      
      **Fix:**
      reset `server.client_pause_end_time` = 0 in `unpauseClients`
      f560531d
    • Binbin's avatar
      Add LMPOP/BLMPOP commands. (#9373) · c50af0ae
      Binbin authored
      We want to add COUNT option for BLPOP.
      But we can't do it without breaking compatibility due to the command arguments syntax.
      So this commit introduce two new commands.
      
      Syntax for the new LMPOP command:
      `LMPOP numkeys [<key> ...] LEFT|RIGHT [COUNT count]`
      
      Syntax for the new BLMPOP command:
      `BLMPOP timeout numkeys [<key> ...] LEFT|RIGHT [COUNT count]`
      
      Some background:
      - LPOP takes one key, and can return multiple elements.
      - BLPOP takes multiple keys, but returns one element from just one key.
      - LMPOP can take multiple keys and return multiple elements from just one key.
      
      Note that LMPOP/BLMPOP  can take multiple keys, it eventually operates on just one key.
      And it will propagate as LPOP or RPOP with the COUNT option.
      
      As a new command, it still return NIL if we can't pop any elements.
      For the normal response is nested arrays in RESP2 and RESP3, like:
      ```
      LMPOP/BLMPOP 
      1) keyname
      2) 1) element1
         2) element2
      ```
      I.e. unlike BLPOP that returns a key name and one element so it uses a flat array,
      and LPOP that returns multiple elements with no key name, and again uses a flat array,
      this one has to return a nested array, and it does for for both RESP2 and RESP3 (like SCAN does)
      
      Some discuss can see: #766 #8824
      c50af0ae
    • Huang Zhw's avatar
      Add INFO total_active_defrag_time and current_active_defrag_time (#9377) · 216f168b
      Huang Zhw authored
      Add two INFO metrics:
      ```
      total_active_defrag_time:12345
      current_active_defrag_time:456
      ```
      `current_active_defrag_time` if greater than 0, means how much time has
      passed since active defrag started running. If active defrag stops, this metric is reset to 0.
      `total_active_defrag_time` means total time the fragmentation
      was over the defrag threshold since the server started.
      
      This is a followup PR for #9031
      216f168b
  2. 08 Sep, 2021 1 commit
    • zhaozhao.zz's avatar
      Fix wrong offset when replica pause (#9448) · 1b83353d
      zhaozhao.zz authored
      When a replica paused, it would not apply any commands event the command comes from master, if we feed the non-applied command to replication stream, the replication offset would be wrong, and data would be lost after failover(since replica's `master_repl_offset` grows but command is not applied).
      
      To fix it, here are the changes:
      * Don't update replica's replication offset or propagate commands to sub-replicas when it's paused in `commandProcessed`.
      * Show `slave_read_repl_offset` in info reply.
      * Add an assert to make sure master client should never be blocked unless pause or module (some modules may use block way to do background (parallel) processing and forward original block module command to the replica, it's not a good way but it can work, so the assert excludes module now, but someday in future all modules should rewrite block command to propagate like what `BLPOP` does).
      1b83353d
  3. 02 Sep, 2021 1 commit
    • guybe7's avatar
      Fix two minor bugs (MIGRATE key args and getKeysUsingCommandTable) (#9455) · 6aa2285e
      guybe7 authored
      1. MIGRATE has a potnetial key arg in argv[3]. It should be reflected in the command table.
      2. getKeysUsingCommandTable should never free getKeysResult, it is always freed by the caller)
         The reason we never encountered this double-free bug is that almost always getKeysResult
         uses the statis buffer and doesn't allocate a new one.
      6aa2285e
  4. 31 Aug, 2021 1 commit
    • Viktor Söderqvist's avatar
      Slot-to-keys using dict entry metadata (#9356) · f24c63a2
      Viktor Söderqvist authored
      
      
      * Enhance dict to support arbitrary metadata carried in dictEntry
      Co-authored-by: default avatarViktor Söderqvist <viktor.soderqvist@est.tech>
      
      * Rewrite slot-to-keys mapping to linked lists using dict entry metadata
      
      This is a memory enhancement for Redis Cluster.
      
      The radix tree slots_to_keys (which duplicates all key names prefixed with their
      slot number) is replaced with a linked list for each slot. The dict entries of
      the same cluster slot form a linked list and the pointers are stored as metadata
      in each dict entry of the main DB dict.
      
      This commit also moves the slot-to-key API from db.c to cluster.c.
      Co-authored-by: default avatarJim Brunner <brunnerj@amazon.com>
      f24c63a2
  5. 24 Aug, 2021 1 commit
    • Garen Chan's avatar
      Fix boundary problem of adjusting open files limit. (#5722) · 945a83d4
      Garen Chan authored
      When `decr_step` is greater than `oldlimit`, the final `bestlimit` may be invalid.
      
          For example, oldlimit = 10, decr_step = 16.
          Current bestlimit = 15 and setrlimit() failed. Since bestlimit  is less than decr_step , then exit the loop.
          The final bestlimit is larger than oldlimit but is invalid.
      
      Note that this only matters if the system fd limit is below 16, so unlikely to have any actual effect.
      945a83d4
  6. 12 Aug, 2021 1 commit
    • Yossi Gottlieb's avatar
      Improve setup operations order after fork. (#9365) · 1221f7cd
      Yossi Gottlieb authored
      The order of setting things up follows some reasoning: Setup signal
      handlers first because a signal could fire at any time. Adjust OOM score
      before everything else to assist the OOM killer if memory resources are
      low.
      
      The trigger for this is a valgrind test failure which resulted with the
      child catching a SIGUSR1 before initializing the handler.
      1221f7cd
  7. 10 Aug, 2021 2 commits
    • DarrenJiang13's avatar
      fix a compilation error around madvise when make with jemalloc on MacOS (#9350) · 8ab33c18
      DarrenJiang13 authored
      We only use MADV_DONTNEED on Linux, that's were it was tested.
      8ab33c18
    • sundb's avatar
      Replace all usage of ziplist with listpack for t_hash (#8887) · 02fd76b9
      sundb authored
      
      
      Part one of implementing #8702 (taking hashes first before other types)
      
      ## Description of the feature
      1. Change ziplist encoded hash objects to listpack encoding.
      2. Convert existing ziplists on RDB loading time. an O(n) operation.
      
      ## Rdb format changes
      1. Add RDB_TYPE_HASH_LISTPACK rdb type.
      2. Bump RDB_VERSION to 10
      
      ## Interface changes
      1. New `hash-max-listpack-entries` config is an alias for `hash-max-ziplist-entries` (same with `hash-max-listpack-value`)
      2. OBJECT ENCODING will return `listpack` instead of `ziplist`
      
      ## Listpack improvements:
      1. Support direct insert, replace integer element (rather than convert back and forth from string)
      3. Add more listpack capabilities to match the ziplist ones (like `lpFind`, `lpRandomPairs` and such)
      4. Optimize element length fetching, avoid multiple calculations
      5. Use inline to avoid function call overhead.
      
      ## Tests
      1. Add a new test to the RDB load time conversion
      2. Adding the listpack unit tests. (based on the one in ziplist.c)
      3. Add a few "corrupt payload: fuzzer findings" tests, and slightly modify existing ones.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      02fd76b9
  8. 09 Aug, 2021 1 commit
  9. 06 Aug, 2021 1 commit
  10. 05 Aug, 2021 1 commit
    • yoav-steinberg's avatar
      dict struct memory optimizations (#9228) · 5e908a29
      yoav-steinberg authored
      Reduce dict struct memory overhead
      on 64bit dict size goes down from jemalloc's 96 byte bin to its 56 byte bin.
      
      summary of changes:
      - Remove `privdata` from callbacks and dict creation. (this affects many files, see "Interface change" below).
      - Meld `dictht` struct into the `dict` struct to eliminate struct padding. (this affects just dict.c and defrag.c)
      - Eliminate the `sizemask` field, can be calculated from size when needed.
      - Convert the `size` field into `size_exp` (exponent), utilizes one byte instead of 8.
      
      Interface change: pass dict pointer to dict type call back functions.
      This is instead of passing the removed privdata field. In the future if
      we'd like to have private data in the callbacks we can extract it from
      the dict type. We can extend dictType to include a custom dict struct
      allocator and use it to allocate more data at the end of the dict
      struct. This data can then be used to store private data later acccessed
      by the callbacks.
      5e908a29
  11. 04 Aug, 2021 1 commit
    • Wang Yuan's avatar
      Use madvise(MADV_DONTNEED) to release memory to reduce COW (#8974) · d4bca53c
      Wang Yuan authored
      
      
      ## Backgroud
      As we know, after `fork`, one process will copy pages when writing data to these
      pages(CoW), and another process still keep old pages, they totally cost more memory.
      For redis, we suffered that redis consumed much memory when the fork child is serializing
      key/values, even that maybe cause OOM.
      
      But actually we find, in redis fork child process, the child process don't need to keep some
      memory and parent process may write or update that, for example, child process will never
      access the key-value that is serialized but users may update it in parent process.
      So we think it may reduce COW if the child process release memory that it is not needed.
      
      ## Implementation
      For releasing key value in child process, we may think we call `decrRefCount` to free memory,
      but i find the fork child process still use much memory when we don't write any data to redis,
      and it costs much more time that slows down bgsave. Maybe because memory allocator doesn't
      really release memory to OS, and it may modify some inner data for this free operation, especially
      when we free small objects.
      
      Moreover, CoW is based on  pages, so it is a easy way that we only free the memory bulk that is
      not less than kernel page size. madvise(MADV_DONTNEED) can quickly release specified region
      pages to OS bypassing memory allocator, and allocator still consider that this memory still is used
      and don't change its inner data.
      
      There are some buffers we can release in the fork child process:
      - **Serialized key-values**
        the fork child process never access serialized key-values, so we try to free them.
        Because we only can release big bulk memory, and it is time consumed to iterate all
        items/members/fields/entries of complex data type. So we decide to iterate them and
        try to release them only when their average size of item/member/field/entry is more
        than page size of OS.
      - **Replication backlog**
        Because replication backlog is a cycle buffer, it will be changed quickly if redis has heavy
        write traffic, but in fork child process, we don't need to access that.
      - **Client buffers**
        If clients have requests during having the fork child process, clients' buffer also be changed
        frequently. The memory includes client query buffer, output buffer, and client struct used memory.
      
      To get child process peak private dirty memory, we need to count peak memory instead
      of last used memory, because the child process may continue to release memory (since
      COW used to only grow till now, the last was equivalent to the peak).
      Also we're adding a new `current_cow_peak` info variable (to complement the existing
      `current_cow_size`)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      d4bca53c
  12. 03 Aug, 2021 1 commit
  13. 02 Aug, 2021 2 commits
    • Binbin's avatar
      Modify some error logs printing level. (#9306) · 4000cb7d
      Binbin authored
      1. In sendBulkToSlave, we used LL_VERBOSE in the past, changed to
      LL_WARNING. (all the other places that do freeClient(slave) use LL_WARNING)
      2. The old style LOG_WARNING, chang it to LL_WARNING. Introduced in an
      old pr (#1690).
      4000cb7d
    • Ning Sun's avatar
      Add NX/XX/GT/LT options to EXPIRE command group (#2795) · f74af0e6
      Ning Sun authored
      
      
      Add NX, XX, GT, and LT flags to EXPIRE, PEXPIRE, EXPIREAT, PEXAPIREAT.
      - NX - only modify the TTL if no TTL is currently set 
      - XX - only modify the TTL if there is a TTL currently set 
      - GT - only increase the TTL (considering non-volatile keys as infinite expire time)
      - LT - only decrease the TTL (considering non-volatile keys as infinite expire time)
      return value of the command is 0 when the operation was skipped due to one of these flags.
      Signed-off-by: default avatarNing Sun <sunng@protonmail.com>
      f74af0e6
  14. 26 Jul, 2021 1 commit
    • Huang Zhw's avatar
      Add INFO stat total_eviction_exceeded_time and current_eviction_exceeded_time (#9031) · 17511df5
      Huang Zhw authored
      
      
      Add two INFO metrics:
      ```
      total_eviction_exceeded_time:69734
      current_eviction_exceeded_time:10230
      ```
      `current_eviction_exceeded_time` if greater than 0, means how much time current used memory is greater than `maxmemory`. And we are still over the maxmemory. If used memory is below `maxmemory`, this metric is reset to 0.
      `total_eviction_exceeded_time` means total time used memory is greater than `maxmemory` since server startup. 
      The units of these two metrics are ms.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      17511df5
  15. 20 Jul, 2021 1 commit
    • Oran Agra's avatar
      Fix ACL category for SELECT, WAIT, ROLE, LASTSAVE, READONLY, READWRITE, ASKING (#9208) · 32e61ee2
      Oran Agra authored
      - SELECT and WAIT don't read or write from the keyspace (unlike DEL, EXISTS, EXPIRE, DBSIZE, KEYS, etc).
      they're more similar to AUTH and HELLO (and maybe PING and COMMAND).
      they only affect the current connection, not the server state, so they should be `@connection`, not `@keyspace`
      
      - ROLE, like LASTSAVE is `@admin` (and `@dangerous` like INFO) 
      
      - ASKING, READONLY, READWRITE are `@connection` too (not `@keyspace`)
      
      - Additionally, i'm now documenting the exact meaning of each ACL category so it's clearer which commands belong where.
      32e61ee2
  16. 11 Jul, 2021 1 commit
    • perryitay's avatar
      Fail EXEC command in case a watched key is expired (#9194) · ac8b1df8
      perryitay authored
      
      
      There are two issues fixed in this commit: 
      1. we want to fail the EXEC command in case there is a watched key that's logically
         expired but not yet deleted by active expire or lazy expire.
      2. we saw that currently cache time is update in every `call()` (including nested calls),
         this time is being also being use for the isKeyExpired comparison, we want to update
         the cache time only in the first call (execCommand)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      ac8b1df8
  17. 09 Jul, 2021 1 commit
  18. 05 Jul, 2021 2 commits
    • Oran Agra's avatar
      Query buffer shrinking improvements (#5013) · ec582cc7
      Oran Agra authored
      
      
      when tracking the peak, don't reset the peak to 0, reset it to the
      maximum of the current used, and the planned to be used by the current
      arg.
      
      when shrining, split the two separate conditions.
      the idle time shrinking will remove all free space.
      but the peak based shrinking will keep room for the current arg.
      
      when we resize due to a peak (rahter than idle time), don't trim all
      unused space, let the qbuf keep a size that's sufficient for the
      currently process bulklen, and the current peak.
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      Co-authored-by: default avataryoav-steinberg <yoav@monfort.co.il>
      ec582cc7
    • zhaozhao.zz's avatar
      resize query buffer more accurately · 2248eaac
      zhaozhao.zz authored
      1. querybuf_peak has not been updated correctly in readQueryFromClient.
      2. qbuf shrinking uses sdsalloc instead of sdsAllocSize
      
      see more details in issue #4983
      2248eaac
  19. 03 Jul, 2021 1 commit
  20. 01 Jul, 2021 1 commit
    • Wang Yuan's avatar
      Don't start in sentinel mode if only the folder name contains redis-sentinel (#9176) · 16e04ed9
      Wang Yuan authored
      Before this commit, redis-server starts in sentinel mode if the first startup
      argument has the string redis-sentinel, so redis also starts in sentinel mode
      if the directory it was started from contains the string redis-sentinel.
      Now we check the executable name instead of directory.
      
      Some examples:
      1. Execute ./redis-sentinel/redis/src/redis-sentinel, starts in sentinel mode.
      2. Execute ./redis-sentinel/redis/src/redis-server, starts in server mode,
         but before, redis will start in sentinel mode.
      3. Execute ./redis-sentinel/redis/src/redis-server --sentinel, of course, like
         before, starts in sentinel mode.
      16e04ed9
  21. 24 Jun, 2021 1 commit
    • Yossi Gottlieb's avatar
      Add bind-source-addr configuration argument. (#9142) · f233c4c5
      Yossi Gottlieb authored
      In the past, the first bind address that was explicitly specified was
      also used to bind outgoing connections. This could result with some
      problems. For example: on some systems using `bind 127.0.0.1` would
      result with outgoing connections also binding to `127.0.0.1` and failing
      to connect to remote addresses.
      
      With the recent change to the way `bind` is handled, this presented
      other issues:
      
      * The default first bind address is '*' which is not a valid address.
      * We make no distinction between user-supplied config that is identical
      to the default, and the default config.
      
      This commit addresses both these issues by introducing an explicit
      configuration parameter to control the bind address on outgoing
      connections.
      f233c4c5
  22. 22 Jun, 2021 1 commit
    • Yossi Gottlieb's avatar
      Improve bind and protected-mode config handling. (#9034) · 07b0d144
      Yossi Gottlieb authored
      * Specifying an empty `bind ""` configuration prevents Redis from listening on any TCP port. Before this commit, such configuration was not accepted.
      * Using `CONFIG GET bind` will always return an explicit configuration value. Before this commit, if a bind address was not specified the returned value was empty (which was an anomaly).
      
      Another behavior change is that modifying the `bind` configuration to a non-default value will NO LONGER DISABLE protected-mode implicitly.
      07b0d144
  23. 17 Jun, 2021 1 commit
  24. 16 Jun, 2021 1 commit
    • Uri Shachar's avatar
      Cleaning up the cluster interface by moving almost all related declar… (#9080) · c7e502a0
      Uri Shachar authored
      * Cleaning up the cluster interface by moving almost all related declarations into cluster.h
      (no logic change -- just moving declarations/definitions around)
      
      This initial effort leaves two items out of scope - the configuration parsing into the server
      struct and the internals exposed by the clusterNode struct.
      
      * Remove unneeded declarations of dictSds*
      Ideally all the dictSds functionality would move from server.c into a dedicated module
      so we can avoid the duplication in redis-benchmark/cli
      
      * Move crc16 back into server.h, will be moved out once we create a seperate header file for
      hashing functions
      c7e502a0
  25. 15 Jun, 2021 1 commit
    • sundb's avatar
      Fix the wrong reisze of querybuf (#9003) · e5d8a5eb
      sundb authored
      The initialize memory of `querybuf` is `PROTO_IOBUF_LEN(1024*16) * 2` (due to sdsMakeRoomFor being greedy), under `jemalloc`, the allocated memory will be 40k.
      This will most likely result in the `querybuf` being resized when call `clientsCronResizeQueryBuffer` unless the client requests it fast enough.
      
      Note that this bug existed even before #7875, since the condition for resizing includes the sds headers (32k+6).
      
      ## Changes
      1. Use non-greedy sdsMakeRoomFor when allocating the initial query buffer (of 16k).
      1. Also use non-greedy allocation when working with BIG_ARG (we won't use that extra space anyway)
      2. in case we did use a greedy allocation, read as much as we can into the buffer we got (including internal frag), to reduce system calls.
      3. introduce a dedicated constant for the shrinking (same value as before)
      3. Add test for querybuf.
      4. improve a maxmemory test by ignoring the effect of replica query buffers (can accumulate many ACKs on slow env)
      5. improve a maxmemory by disabling slowlog (it will cause slight memory growth on slow env).
      e5d8a5eb
  26. 14 Jun, 2021 1 commit
    • YaacovHazan's avatar
      cleanup around loadAppendOnlyFile (#9012) · 1677efb9
      YaacovHazan authored
      Today when we load the AOF on startup, the loadAppendOnlyFile checks if
      the file is openning for reading.
      This check is redundent (dead code) as we open the AOF file for writing at initServer,
      and the file will always be existing for the loadAppendOnlyFile.
      
      In this commit:
      - remove all the exit(1) from loadAppendOnlyFile, as it is the caller
        responsibility to decide what to do in case of failure.
      - move the opening of the AOF file for writing, to be after we loading it.
      - avoid return -ERR in DEBUG LOADAOF, when the AOF is existing but empty
      1677efb9
  27. 10 Jun, 2021 1 commit
    • Binbin's avatar
      Fixed some typos, add a spell check ci and others minor fix (#8890) · 0bfccc55
      Binbin authored
      This PR adds a spell checker CI action that will fail future PRs if they introduce typos and spelling mistakes.
      This spell checker is based on blacklist of common spelling mistakes, so it will not catch everything,
      but at least it is also unlikely to cause false positives.
      
      Besides that, the PR also fixes many spelling mistakes and types, not all are a result of the spell checker we use.
      
      Here's a summary of other changes:
      1. Scanned the entire source code and fixes all sorts of typos and spelling mistakes (including missing or extra spaces).
      2. Outdated function / variable / argument names in comments
      3. Fix outdated keyspace masks error log when we check `config.notify-keyspace-events` in loadServerConfigFromString.
      4. Trim the white space at the end of line in `module.c`. Check: https://github.com/redis/redis/pull/7751
      5. Some outdated https link URLs.
      6. Fix some outdated comment. Such as:
          - In README: about the rdb, we used to said create a `thread`, change to `process`
          - dbRandomKey function coment (about the dictGetRandomKey, change to dictGetFairRandomKey)
          - notifyKeyspaceEvent fucntion comment (add type arg)
          - Some others minor fix in comment (Most of them are incorrectly quoted by variable names)
      7. Modified the error log so that users can easily distinguish between TCP and TLS in `changeBindAddr`
      0bfccc55
  28. 30 May, 2021 1 commit
    • ny0312's avatar
      Always replicate time-to-live(TTL) as absolute timestamps in milliseconds (#8474) · 53d1acd5
      ny0312 authored
      Till now, on replica full-sync we used to transfer absolute time for TTL,
      however when a command arrived (EXPIRE or EXPIREAT),
      we used to propagate it as is to replicas (possibly with relative time),
      but always translate it to EXPIREAT (absolute time) to AOF.
      
      This commit changes that and will always use absolute time for propagation.
      see discussion in #8433
      
      Furthermore, we Introduce new commands: `EXPIRETIME/PEXPIRETIME`
      that allow extracting the absolute TTL time from a key.
      53d1acd5
  29. 19 May, 2021 1 commit
  30. 17 May, 2021 1 commit
  31. 13 May, 2021 1 commit
  32. 04 May, 2021 1 commit
  33. 29 Apr, 2021 1 commit
  34. 27 Apr, 2021 1 commit
    • Oran Agra's avatar
      Prevent replicas from sending commands that interact with keyspace (#8868) · 46f4ebbe
      Oran Agra authored
      This solves an issue reported in #8712 in which a replica would bypass
      the client write pause check and cause an assertion due to executing a
      write command during failover.
      
      The fact is that we don't expect replicas to execute any command other
      than maybe REPLCONF and PING, etc. but matching against the ADMIN
      command flag is insufficient, so instead i just block keyspace access
      for now.
      46f4ebbe
  35. 21 Apr, 2021 1 commit