1. 12 Mar, 2024 3 commits
  2. 11 Mar, 2024 1 commit
  3. 10 Mar, 2024 1 commit
    • Matthew Douglass's avatar
      Fix conversion of numbers in lua args to redis args (#13115) · 5fdaa53d
      Matthew Douglass authored
      
      
      Since lua_Number is not explicitly an integer or a double, we need to
      make an effort
      to convert it as an integer when that's possible, since the string could
      later be used
      in a context that doesn't support scientific notation (e.g. 1e9 instead
      of 100000000).
      
      Since fpconv_dtoa converts numbers with the equivalent of `%f` or `%e`,
      which ever is shorter,
      this would break if we try to pass a long integer number to a command
      that takes integer.
      we'll get an implicit conversion to string in Lua, and then the parsing
      in getLongLongFromObjectOrReply will fail.
      
      ```
      > eval "redis.call('hincrby', 'key', 'field', '1000000000')" 0
      (nil)
      > eval "redis.call('hincrby', 'key', 'field', tonumber('1000000000'))" 0
      (error) ERR value is not an integer or out of range script: ac99c32e4daf7e300d593085b611de261954a946, on @user_script:1.
      ```
      
      Switch to using ll2string if the number can be safely represented as a
      long long.
      
      The problem was introduced in #10587 (Redis 7.2).
      closes #13113.
      
      ---------
      Co-authored-by: default avatarBinbin <binloveplay1314@qq.com>
      Co-authored-by: default avatardebing.sun <debing.sun@redis.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      5fdaa53d
  4. 08 Mar, 2024 1 commit
  5. 05 Mar, 2024 1 commit
    • Ping Xie's avatar
      Fix PONG message processing for primary-ship tracking during failovers (#13055) · 28976a90
      Ping Xie authored
      This commit updates the processing of PONG gossip messages in the
      cluster. When a node (B) becomes a replica due to a failover, its PONG
      messages include its new primary node's (A) information and B's
      configuration epoch is aligned with A's. This allows observer nodes to
      identify changes in primary-ship, addressing issues of intermediate
      states and enhancing cluster state consistency during topology changes.
      
      Fix #13018
      28976a90
  6. 04 Mar, 2024 1 commit
    • debing.sun's avatar
      Implement defragmentation for pubsub kvstore (#13058) · ad127303
      debing.sun authored
      
      
      After #13013
      
      ### This PR make effort to defrag the pubsub kvstore in the following
      ways:
      
      1. Till now server.pubsub(shard)_channels only share channel name obj
      with the first subscribed client, now change it so that the clients and
      the pubsub kvstore share the channel name robj.
      This would save a lot of memory when there are many subscribers to the
      same channel.
      It also means that we only need to defrag the channel name robj in the
      pubsub kvstore, and then update
      all client references for the current channel, avoiding the need to
      iterate through all the clients to do the same things.
          
      2. Refactor the code to defragment pubsub(shard) in the same way as
      defragment of keys and EXPIRES, with the exception that we only
      defragment pubsub(without shard) when slot is zero.
      
      
      ### Other
      Fix an overlook in #11695, if defragment doesn't reach the end time, we
      should wait for the current
      db's keys and expires, pubsub and pubsubshard to finish before leaving,
      now it's possible to exit
      early when the keys are defragmented.
      
      ---------
      Co-authored-by: default avataroranagra <oran@redislabs.com>
      ad127303
  7. 03 Mar, 2024 1 commit
  8. 02 Mar, 2024 1 commit
    • YaacovHazan's avatar
      redis-cli fixes around help hints version filtering (#13097) · a50bbcb6
      YaacovHazan authored
      
      
      - In removeUnsupportedArgs, trying to access the next item after the
      last one and causing an out of bounds read.
      - In versionIsSupported, when the 'version' is equal to 'since', the
      return value is 0 (not supported).
      Also, change the function to return `not supported` in case they have
      different numbers of digits
      
      Both issues were found by `Non-interactive non-TTY CLI: Test
      command-line hinting - old server` under `test-sanitizer-address` (When
      changing the `src/version.h` locally to `8.0.0`)
      
      The new `MAXAGE` argument inside `client-kill` triggered the issue (new
      argument at the end of the list)
      
      ---------
      Co-authored-by: default avatarYaacovHazan <yaacov.hazan@redislabs.com>
      a50bbcb6
  9. 01 Mar, 2024 1 commit
    • Chen Tianjie's avatar
      Add overhead of all DBs and rehashing dict count to info. (#12913) · 4cae99e7
      Chen Tianjie authored
      
      
      Sometimes we need to make fast judgement about why Redis is suddenly
      taking more memory. One of the reasons is main DB's dicts doing
      rehashing.
      
      We may use `MEMORY STATS` to monitor the overhead memory of each DB, but
      there still lacks a total sum to show an overall trend. So this PR adds
      the total overhead of all DBs to `INFO MEMORY` section, together with
      the total count of rehashing DB dicts, providing some intuitive metrics
      about main dicts rehashing.
      
      This PR adds the following metrics to INFO MEMORY
      * `mem_overhead_db_hashtable_rehashing` - only size of ht[0] in
      dictionaries we're rehashing (i.e. the memory that's gonna get released
      soon)
      
      and a similar ones to MEMORY STATS:
      * `overhead.db.hashtable.lut` (complements the existing
      `overhead.hashtable.main` and `overhead.hashtable.expires` which also
      counts the `dictEntry` structs too)
      * `overhead.db.hashtable.rehashing` - temporary rehashing overhead.
      * `db.dict.rehashing.count` - number of top level dictionaries being
      rehashed.
      
      ---------
      Co-authored-by: default avatarzhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      4cae99e7
  10. 29 Feb, 2024 2 commits
    • Binbin's avatar
      Fix propagation of entries_read by calling streamPropagateGroupID unconditionally (#12898) · f17381a3
      Binbin authored
      In XREADGROUP ACK, because streamPropagateXCLAIM does not propagate
      entries-read, entries-read will be inconsistent between master and
      replicas.
      I.e. if no entries were claimed, it would have propagated correctly, but
      if some
      were claimed, then the entries-read field would be inconsistent on the
      replica.
      
      The fix was suggested by guybe7, call streamPropagateGroupID
      unconditionally,
      so that we will normalize entries_read on the replicas. In the past, we
      would
      only set propagate_last_id when NOACK was specified. And in #9127,
      XCLAIM did
      not propagate entries_read in ACK, which would cause entries_read to be
      inconsistent between master and replicas.
      
      Another approach is add another arg to XCLAIM and let it propagate
      entries_read,
      but we decided not to use it. Because we want minimal damage in case
      there's an
      old target and new source (in the worst case scenario, the new source
      doesn't
      recognize XGROUP SETID ... ENTRIES READ and the lag is lost. If we
      change XCLAIM,
      the damage is much more severe).
      
      In this patch, now if the user uses XREADGROUP .. COUNT 1 there will be
      an additional
      overhead of MULTI, EXEC and XGROUPSETID. We assume the extra command in
      case of
      COUNT 1 (4x factor, changing from one XCLAIM to
      MULTI+XCLAIM+XSETID+EXEC), is probably
      ok since reading just one entry is in any case very inefficient (a
      client round trip
      per record), so we're hoping it's not a common case.
      
      Issue was introduced in #9127.
      f17381a3
    • zhaozhao.zz's avatar
      freeDictIfNeeded when kvstoreEmpty (#13098) · cc9fbd27
      zhaozhao.zz authored
      just like `kvstoreDictDelete`, we need check `freeDictIfNeeded` when
      `kvstoreEmpty`.
      cc9fbd27
  11. 28 Feb, 2024 2 commits
    • Binbin's avatar
      SCRIPT FLUSH run truly async, close lua interpreter in bio (#13087) · a7abc2f0
      Binbin authored
      Even if we have SCRIPT FLUSH ASYNC now, when there are a lot of
      lua scripts, SCRIPT FLUSH ASYNC will still block the main thread.
      This is because lua_close is executed in the main thread, and lua
      heap needs to release a lot of memory.
      
      In this PR, we take the current lua instance on lctx.lua and call
      lua_close on it in a background thread, to close it in async way.
      This is MeirShpilraien's idea.
      a7abc2f0
    • LiiNen's avatar
      Fix redis-cli --count (for --scan, --bigkeys, etc) was ignored unless... · 763827c9
      LiiNen authored
      Fix redis-cli --count (for --scan, --bigkeys, etc) was ignored unless --pattern was also used (#13092)
      
      The --count option for redis-cli has been released in redis 7.2.
      https://github.com/redis/redis/pull/12042
      But I have found in code, that some logic was missing for using this
      'count' option.
      
      ```
      static redisReply *sendScan(unsigned long long *it) {
          redisReply *reply;
      
          if (config.pattern)
              reply = redisCommand(context, "SCAN %llu MATCH %b COUNT %d",
                  *it, config.pattern, sdslen(config.pattern), config.count);
          else
              reply = redisCommand(context,"SCAN %llu",*it);
      ```
      
      The intention was being able to using scan count.
      But in this case, the --count will be only applied when 'pattern' is
      declared.
      So, I had fix it simply, to be worked properly - even if --pattern
      option is not being used.
      
      I tested it simply with time() command several times, and I could see it
      works as intended with this commit.
      The examples of test results are below:
      ```
      # unstable build
      
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan >/dev/null 2>/dev/null)
      
      real    0m1.287s
      user    0m0.011s
      sys     0m0.022s
      
      # count is not applied
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan --count 1000 >/dev/null 2>/dev/null)
      
      real    0m1.117s
      user    0m0.011s
      sys     0m0.020s
      
      # count is applied with --pattern
      
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan --count 1000 --pattern "hash:*" >/dev/null 2>/dev/null)
      
      real    0m0.045s
      user    0m0.002s
      sys     0m0.002s
      ```
      
      ```
      # fix-redis-cli-scan-count build
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan >/dev/null 2>/dev/null)
      
      real    0m1.084s
      user    0m0.008s
      sys     0m0.024s
      
      # count is applied even if --pattern is not declared
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan --count 1000 >/dev/null 2>/dev/null)
      
      real    0m0.043s
      user    0m0.000s
      sys     0m0.004s
      
      # of course this also applied
      time(./redis-cli -a $AUTH -p $PORT -h $HOST --scan --count 1000 --pattern "hash:*" >/dev/null 2>/dev/null)
      
      real    0m0.031s
      user    0m0.002s
      sys     0m0.002s
      ```
      
      
      
      Thanks a lot.
      763827c9
  12. 26 Feb, 2024 2 commits
    • Yanqi Lv's avatar
      Optimize DEL on expired keys (#13080) · 0a12f380
      Yanqi Lv authored
      
      
      If we call `DEL` on expired keys, keys may be deleted in
      `expireIfNeeded` and we don't need to call `dbSyncDelete` or
      `dbAsyncDelete` after, which repeat the deletion process(i.e. find keys
      in main db).
      
      In this PR, I refine the return values of `expireIfNeeded` to indicate
      whether we have deleted the expired key to avoid the potential redundant
      deletion logic in `delGenericCommand`. Besides, because both KEY_EXPIRED
      and KEY_DELETED are non-zero, this PR won't affect other functions
      calling `expireIfNeeded`.
      
      I also make a performance test. I first close active expiration by
      `debug set-active-expire 0` and write 1 million keys with 1ms TTL. Then
      I repeatedly delete 100 expired keys in one `DEL`. The results are as
      follow, which shows that this PR can improve performance by about 10% in
      this situation.
      **unstable**
      ```
      Summary:
        throughput summary: 10080.65 requests per second
        latency summary (msec):
                avg       min       p50       p95       p99       max
              0.953     0.136     0.959     1.215     1.335     2.247
      ```
      
      **This PR**
      ```
      Summary:			
        throughput summary: 11074.20 requests per second			
        latency summary (msec):			
                avg       min       p50       p95       p99       max			
              0.865     0.128     0.879     1.055     1.175     2.159			
      ```
      
      ---------
      Co-authored-by: default avatarViktor Söderqvist <viktor.soderqvist@est.tech>
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      0a12f380
    • Binbin's avatar
      Fix size stat in malloc(0) and cleanups around zmalloc file (#13068) · 104b2076
      Binbin authored
      In #8554, we added a MALLOC_MIN_SIZE to use a minimum allocation
      size when using malloc(0). However, we did not update the size,
      when malloc_size is missing.
      
      When malloc_size exists, we record the size that was allocated
      instead of the size that was requested. This would work with both
      jemalloc, and libc malloc (the change in #8554, doesn't break this).
      
      When malloc_size is missing, we allocate extra size_t bytes and
      store the requested size in it. In that case, the requested size
      is probably different than the allocated size anyway (the change
      in #8554 doesn't conceptually change that).
      
      So we have room for improvement since in this case we are aware
      of the extra bytes we asked for. Same as we're also aware of the
      extra size_t bytes we asked for.
      
      In addition, some cleaning was done:
      1. fixes some outupdated comments.
      2. test cleanups
      104b2076
  13. 22 Feb, 2024 4 commits
    • Binbin's avatar
      Fix minor memory leak in rewriteSetObject (#13086) · bfcaa7db
      Binbin authored
      It seems to be a leak caused by code refactoring in #11290.
      it's a small leak, that only happens if there's an IO error.
      bfcaa7db
    • debing.sun's avatar
      Expose lua os.clock() api (#12971) · 4a265554
      debing.sun authored
      
      
      Implement #12699
      
      This PR exposing Lua os.clock() api for getting the elapsed time of Lua
      code execution.
      
      Using:
      ```lua
      local start = os.clock()
      ...
      do something
      ...
      local elpased = os.clock() - start
      ```
      
      ---------
      Co-authored-by: default avatarMeir Shpilraien (Spielrein) <meir@redis.com>
      Co-authored-by: default avatarMadelyn Olson <34459052+madolson@users.noreply.github.com>
      4a265554
    • debing.sun's avatar
      Determine the large limit of the quicklist node based on fill (#12659) · 165afc5f
      debing.sun authored
      Following #12568
      
      In issue #9357, when inserting an element larger than 1GB, we currently
      store it in a plain node instead of a listpack.
      Presently, when we insert an element that exceeds the maximum size of a
      packed node, it cannot be accommodated in any other nodes, thus ending
      up isolated like a large element.
      I.e. it's a node with only one element, but it's listpack encoded rather
      than a plain buffer.
      
      This PR lowers the threshold for considering an element as 'large' from
      1GB to the maximum size of a node.
      While this change doesn't completely resolve the bug mentioned in the
      previous PR, it does mitigate its potential impact.
      
      As a result of this change, we can now only use LSET to replace an
      element with another element that falls below the maximum size
      threshold.
      In the worst-case scenario, with a fill of -5, the largest packed node
      we can create is 2GB (32k * 64k):
      * 32k: The smallest element in a listpack is 2 bytes, which allows us to
      store up to 32k elements.
      * 64k: This is the maximum size for a single quicklist node.
      
      ## Others
      To fully fix #9357, we need more work, as discussed in #12568, when we
      insert an element into a quicklistNode, it may be created in a new node,
      put into another node, or merged, and we can't correctly delete the node
      that was supposed to be deleted.
      I'm not sure it's worth it, since it involves a lot of modifications.
      165afc5f
    • guybe7's avatar
      Edit the history field of xinfo-consumers (#13078) · 820a4e45
      guybe7 authored
      Now it matches the information in xinfo-stream.json
      820a4e45
  14. 21 Feb, 2024 1 commit
  15. 20 Feb, 2024 3 commits
    • debing.sun's avatar
      Defragger improvements around large bins (#12996) · f6785df6
      debing.sun authored
      
      
      Implement #12963
      
      ## Changes
      1. large bins don't have external fragmentation or are at least
      non-defraggable, so we should ignore the effect of
      large bins when measuring fragmentation, and only measure fragmentation
      of small bins. this affects both the allocator_frag* metrics and also
      the active-defrag trigger
      2. Adding INFO metrics for `muzzy` memory, which is memory returned to
      the OS but still shows as RSS until the OS reclaims it.
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      f6785df6
    • Binbin's avatar
      xinfo-stream add minimum to seen-time, skip logreqres in fuzzer (#13056) · ca5cac99
      Binbin authored
      
      
      Recently I saw in CI that reply-schemas-validator fails here:
      ```
      Failed validating 'minimum' in schema[1]['properties']['groups']['items']['properties']['consumers']['items']['properties']['active-time']:
          {'description': 'Last time this consumer was active (successful '
                          'reading/claiming).',
           'minimum': 0,
           'type': 'integer'}
      
      On instance['groups'][0]['consumers'][0]['active-time']:
          -1729380548878722639
      ```
      
      The reason is that in fuzzer, we may restore corrupted active-time,
      which will cause the reply schema CI to fail.
      
      The fuzzer can cause corrupt the state in many places, which will
      bugs that mess up the reply, so we decided to skip logreqres.
      
      Also, seen-time is the same type as active-time, adding the minimum.
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      ca5cac99
    • Binbin's avatar
      Fix wathced client test timing issue caused by late close (#13062) · 3c2ea1ea
      Binbin authored
      There is a timing issue in the test, close may arrive late, or in
      freeClientAsync we will free the client in async way, which will
      lead to errors in watching_clients statistics, since we will only
      unwatch all keys when we truly freeClient.
      
      Add a wait here to avoid this problem. Also fixed some outdated
      comments i saw. The test was introduced in #12966.
      3c2ea1ea
  16. 19 Feb, 2024 3 commits
    • judeng's avatar
      add -fno-omit-frame-pointer to default complication flags (#12973) · fc3a68d8
      judeng authored
      Currently redis uses O3 level optimization would remove the frame pointer
      in the target bin.
      
      In the very old past, when gcc optimized at O1 and above levels, the
      frame pointer is deleted by default to improve performance. This saves
      the RBP registers and reduces the pop/push instructions. But it makes it
      difficult for us to observe the running status of the program. For
      example, the perf tool cannot be used effectively, especially the modern
      eBPF tools such as bcc/memleak.
      fc3a68d8
    • guybe7's avatar
      Adds a README to the command JSON files (#13066) · 6df42df2
      guybe7 authored
      Add readme about the command json folder, what it does, and who should
      (not) use it.
      see discussion
      https://github.com/redis/redis/issues/9359#issuecomment-1936420698
      
      
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarBinbin <binloveplay1314@qq.com>
      6df42df2
    • zhaozhao.zz's avatar
      Calculate the incremental rehash time more precisely (#13063) · 8876d264
      zhaozhao.zz authored
      In the `databasesCron()`, the time consumed by
      `kvstoreIncrementallyRehash()` is used to calculate the exit condition.
      However, within `kvstoreIncrementallyRehash()`, the loop first checks
      for timeout before performing rehashing. Therefore, the time for the
      last rehash isn't accounted for, making the consumed time inaccurate. We
      need to precisely calculate all the time spent on rehashing.
      Additionally, the time allocated to `kvstoreIncrementallyRehash()`
      should be the remaining time, which is
      `INCREMENTAL_REHASHING_THRESHOLD_US` minus the already consumed
      `elapsed_us`.
      8876d264
  17. 18 Feb, 2024 3 commits
    • Binbin's avatar
      AOF_FSYNC_EVERYSEC higher resolution, change aof_last_fsync and... · 9103ccc3
      Binbin authored
      
      AOF_FSYNC_EVERYSEC higher resolution, change aof_last_fsync and aof_flush_postponed_start to use mstime (#13041)
      
      Currently aof_last_fsync is using a low resolution unixtime is really
      bad,
      it checks if the absolute number of (full) seconds changed by one.
      depending on which side of the second barrier it falls, we can get very
      different results.
      
      This PR change the resolution to use milliseconds instead of complete
      seconds.
      
      In cases where the event loop cycle duration is short and their rapid
      (e.g. running
      many fast commands with short pipeline, or a high `hz` config), this
      change will not
      make much difference, since in anyway, we'll be quick to detect that
      we're on a "new
      second", and it's likely that these fsync will always be executed close
      to the second
      switch barrier.
      
      But in cases of rare or slow event loops cycles (e.g. either slow
      commands, or very
      low rate of traffic to redis, and low `hz`), it could easily be that
      with the old code,
      in some cases we'll have over 1.5 seconds between fsyncs, and in others
      less than 0.5.
      
      see discussion in #8612
      
      This PR also handle aof_flush_postponed_start as well, the damage there
      is smaller
      since the threshold is 2 seconds, and not 1.
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      9103ccc3
    • Binbin's avatar
      redis-cli - fix sscanf incorrect return-value check warnings (#13059) · dd92dd8f
      Binbin authored
      From CodeQL: The result of scanf is only checked against 0, but
      it can also return EOF.
      
      Reported in https://github.com/redis/redis/security/code-scanning/38.
      Reported in https://github.com/redis/redis/security/code-scanning/39.
      dd92dd8f
    • zhaozhao.zz's avatar
      Add metrics for WATCH (#12966) · 50d6fe8c
      zhaozhao.zz authored
      Redis has some special commands that mark the client's state, such as
      `subscribe` and `blpop`, which mark the client as `CLIENT_PUBSUB` or
      `CLIENT_BLOCKED`, and we have metrics for the special use cases.
      
      However, there are also other special commands, like `WATCH`, which
      although do not have a specific flags, and should also be considered
      stateful client types. For stateful clients, in many scenarios, the
      connections cannot be shared in "connection pool", meaning connection
      pool cannot be used. For example, whenever the `WATCH` command is
      executed, a new connection is required to put the client into the "watch
      state" because the watched keys are stored in the client.
      
      If different business logic requires watching different keys, separate
      connections must be used; otherwise, there will be contamination. This
      also means that if a user's business heavily relies on the `WATCH`
      command, a large number of connections will be required.
      
      Recently we have encountered this situation in our platform, where some
      users consume a significant number of connections when using Redis
      because of `WATCH`.
      
      I hope we can have a way to observe these special use cases and special
      client connections. Here I add a few monitoring metrics:
      
      1. `watching_clients` in `INFO` reply: The number of clients currently
      in the "watching" state.
      2. `total_watched_keys` in `INFO` reply: The total number of keys being
      watched.
      3. `watch` in `CLIENT LIST` reply: The number of keys each client is
      currently watching.
      50d6fe8c
  18. 15 Feb, 2024 2 commits
    • Binbin's avatar
      Minor optimization in kvstoreDictAddRaw when dict exists (#13054) · c8548737
      Binbin authored
      Usually, the probability that a dict exists is much greater than the
      probability that it does not exist. In kvstoreDictAddRaw, we will call
      kvstoreGetDict multiple times. Based on this assumption, we change
      createDictIfNeeded to something like get or create function:
      ```
      before:
      dict exist: 2 kvstoreGetDict
      dict non-exist: 2 kvstoreGetDict
      
      after:
      dict exist: 1 kvstoreGetDict
      dict non-exist: 3 kvstoreGetDict
      ```
      
      A possible 3% performance improvement was observed:
      
      In addition, some typos/comments i saw have been cleaned up.
      c8548737
    • Binbin's avatar
      zunionInterDiffGenericCommand use ztrycalloc to avoid OOM panic (#13052) · 063de675
      Binbin authored
      In low memory situations, sending a big number of arguments (sets)
      may cause OOM panic. Use ztrycalloc, like we do on LCS and XAUTOCLAIM,
      and fail gracefully.
      
      This change affects the following commands: ZUNION, ZINTER, ZDIFF,
      ZUNIONSTORE, ZINTERSTORE, ZDIFFSTORE, ZINTERCARD.
      063de675
  19. 14 Feb, 2024 1 commit
    • Sankar's avatar
      Do not include gossip about receiver in cluster messages (#13046) · c1d2ac2a
      Sankar authored
      The receiver does not update any of its cluster state based on gossip
      about itself. This commit explicitly avoids sending or processing gossip
      about the receiver.
      
      Currently cluster bus gossips include 10% of nodes in the cluster with a
      minimum of 3 nodes. For up to 30 node clusters, this commit makes sure
      that 1/3 of the gossip (1 out of 3 gossips) is never discarded. This
      should help with relatively faster convergence of cluster state in
      general.
      c1d2ac2a
  20. 12 Feb, 2024 2 commits
  21. 11 Feb, 2024 1 commit
    • debing.sun's avatar
      Fix the failure of defrag test under 32-bit (#13013) · 676f27ac
      debing.sun authored
      Fail CI:
      https://github.com/redis/redis/actions/runs/7837608438/job/21387609715
      
      
      
      ## Why defragment tests only failed under 32-bit
      
      First of all, under 32-bit jemalloc will allocate more small bins and
      less large bins, which will also lead to more external fragmentation,
      therefore, the fragmentation ratio is higher in 32-bit than in 64-bit,
      so the defragment tests(`Active defrag eval scripts: cluster` and
      `Active defrag big keys: cluster`) always fails in 32-bit.
      
      ## Why defragment tests only failed with cluster
      The fowllowing is the result of `Active defrag eval scripts: cluster`
      test.
      
      1) Before #11695, the fragmentation ratio is 3.11%.
      
      2) After #11695, the fragmentation ratio grew to 4.58%.
      Since we are using per-slot dictionary to manage slots, we will only
      defragment the contents of these dictionaries (keys, values), but not
      the dictionaries' struct and ht_table, which means that frequent
      shrinking and expanding of the dictionaries, will make more fragments.
      
      3) After #12850 and #12948, In cluster mode, a large number of cluster
      slot dicts will be shrunk, creating additional fragmention, and the
      dictionary will not be defragged.
      
      ## Solution
      * Add defragmentation of the per-slot dictionary's own structures, dict
      struct and ht_table.
      
      ## Other change
      * Increase floating point print precision of `frags` and `rss` in debug
      logs for defrag
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      676f27ac
  22. 08 Feb, 2024 3 commits
    • Binbin's avatar
      Add new DEBUG dict-resizing command to disable the dict resize (#13043) · 493e31e3
      Binbin authored
      The test fails here and there:
      ```
      *** [err]: expire scan should skip dictionaries with lot's of empty buckets in tests/unit/expire.tcl
      scan didn't handle slot skipping logic.
      ```
      
      There are two case:
      1. In the case of passing the test, we use child process to avoid the
      dict resize, but it can not completely limit it, since in the dictDelete
      we still have chance to trigger the resize (hit the force radio). The
      reason why our test passed before is because the expire dict is still
      in the rehashing process, so the dictDelete, the dictShrinkIfNeeded can
      not trigger the resize.
      
      2. In the case of failing the test, the expire dict finished the
      rehashing,
      so the last dictDelete, the dictShrinkIfNeeded trigger the dict resize
      since it hit the force radio, so the skipping logic fail.
      
      This PR add a new DEBUG command to disbale the dict resize.
      493e31e3
    • Binbin's avatar
      Fix SORT STORE quicklist with the right options (#13042) · 813327b2
      Binbin authored
      We forgot to call quicklistSetOptions after createQuicklistObject,
      in the sort store scenario, we will create a quicklist with default
      fill or compress options.
      
      This PR adds fill and depth parameters to createQuicklistObject to
      specify that options need to be set after creating a quicklist.
      
      This closes #12871.
      
      release notes:
      > Fix lists created by SORT STORE to respect list compression and
      packing configs.
      813327b2
    • debing.sun's avatar
      Fix crash due to merge of quicklist node introduced by #12955 (#13040) · 1e8dc1da
      debing.sun authored
      Fix two crash introducted by #12955
      
      When a quicklist node can't be inserted and split, we eventually merge
      the current node with its neighboring
      nodes after inserting, and compress the current node and its siblings.
      
      1. When the current node is merged with another node, the current node
      may become invalid and can no longer be used.
      
         Solution: let `_quicklistMergeNodes()` return the merged nodes.
      
      3. If the current node is a LZF quicklist node, its recompress will be
      1. If the split node can be merged with a sibling node to become head or
      tail, recompress may cause the head and tail to be compressed, which is
      not allowed.
      
          Solution: always recompress to 0 after merging.
      1e8dc1da