1. 26 Dec, 2023 2 commits
    • zalj's avatar
      fix comment of aeProcessEvents (#12884) · baf5699d
      zalj authored
      The implementation of aeProcessEvents seems have different behavior from
      the top comment.
      The implementation process file events first, then process time events.
      baf5699d
    • Binbin's avatar
      Add restart option to create-cluster script (#12885) · 71f31da6
      Binbin authored
      When testing and debugging the cluster code before, you need
      to stop the cluster after making changes, and then start the
      cluster again. Add a restart option for ease of use.
      71f31da6
  2. 24 Dec, 2023 2 commits
  3. 21 Dec, 2023 1 commit
    • Binbin's avatar
      Move cliVersion to cli_common and add --version support for redis-check-aof (#10856) · 23e980e7
      Binbin authored
      Let us see which version of redis this tool is part of.
      Similarly to redis-cli, redis-benchmark and redis-check-rdb
      
      redis-rdb-check and redis-aof-check are actually symlinks to redis,
      so they will directly use getVersion in server, the format became:
      ```
      {title} v={redis_version} sha={sha}:{dirty} malloc={malloc} bits={bits} build={build}
      ```
      
      Move cliVersion into cli_common, redis-cli and redis-benchmark will
      use it, and the format is not change:
      ```
      {title} {redis_version} (git:{sha})
      ```
      23e980e7
  4. 17 Dec, 2023 1 commit
  5. 15 Dec, 2023 2 commits
    • Binbin's avatar
      Always keep an in-memory history of all commands in redis-cli (#12862) · adbb534f
      Binbin authored
      redis-cli avoids saving sensitive commands in it's history (doesn't
      persist them to the history file).
      this means that if you had a typo and you wanna re-run the command, you
      can't easily do that.
      This PR changes that to keep an in-memory history of all the redacted
      commands, and just
      not persist them to disk. This way we would be able to press the up
      arrow and
      re-try the command freely, and it'll just not survive a redis-cli
      restart.
      adbb534f
    • zhaozhao.zz's avatar
      Unified db rehash method for both standalone and cluster (#12848) · d8a21c57
      zhaozhao.zz authored
      After #11695, we added two functions `rehashingStarted` and
      `rehashingCompleted` to the dict structure. We also registered two
      handlers for the main database's dict and expire structures. This allows
      the main database to record the dict in `rehashing` list when rehashing
      starts. Later, in `serverCron`, the `incrementallyRehash` function is
      continuously called to perform the rehashing operation. However,
      currently, when rehashing is completed, `rehashingCompleted` does not
      remove the dict from the `rehashing` list. This results in the
      `rehashing` list containing many invalid dicts. Although subsequent cron
      checks and removes dicts that don't require rehashing, it is still
      inefficient.
      
      This PR implements the functionality to remove the dict from the
      `rehashing` list in `rehashingCompleted`. This is achieved by adding
      `metadata` to the dict structure, which keeps track of its position in
      the `rehashing` list, allowing for quick removal. This approach avoids
      storing duplicate dicts in the `rehashing` list.
      
      Additionally, there are other modifications:
      
      1. Whether in standalone or cluster mode, the dict in database is
      inserted into the rehashing linked list when rehashing starts. This
      eliminates the need to distinguish between standalone and cluster mode
      in `incrementallyRehash`. The function only needs to focus on the dicts
      in the `rehashing` list that require rehashing.
      2. `rehashing` list is moved from per-database to Redis server level.
      This decouples `incrementallyRehash` from the database ID, and in
      standalone mode, there is no need to iterate over all databases,
      avoiding unnecessary access to databases that do not require rehashing.
      In the future, even if unsharded-cluster mode supports multiple
      databases, there will be no risk involved.
      3. The insertion and removal operations of dict structures in the
      `rehashing` list are decoupled from `activerehashing` config.
      `activerehashing` only controls whether `incrementallyRehash` is
      executed in serverCron. There is no need for additional steps when
      modifying the `activerehashing` switch, as in #12705.
      d8a21c57
  6. 14 Dec, 2023 1 commit
    • Guillaume Koenig's avatar
      Extend rax usage by allowing any long long value (#12837) · 967fb3c6
      Guillaume Koenig authored
      The raxFind implementation uses a special pointer value (the address of
      a static string) as the "not found" value. It works as long as actual
      pointers were used. However we've seen usages where long long,
      non-pointer values have been used. It creates a risk that one of the
      long long value precisely is the address of the special "not found"
      value. This commit changes raxFind to return 1 or 0 to indicate
      elementhood, and take in a new void **value to optionally return the
      associated value.
      
      By extension, this also allow the RedisModule_DictSet/Replace operations
      to also safely insert integers instead of just pointers.
      967fb3c6
  7. 13 Dec, 2023 3 commits
    • Chen Tianjie's avatar
      Support by/get options for sort(_ro) in cluster mode when pattern implies slot. (#12728) · e95a5d48
      Chen Tianjie authored
      The by/get options of sort/sort_ro command used to be forbidden in
      cluster mode, since we are not sure which slot the pattern may be in.
      
      As the optimization done in #12536, patterns now can be mapped to slots,
      we should allow by/get options in cluster mode when the pattern maps to
      the same slot as the key.
      e95a5d48
    • Binbin's avatar
      Redact ACL username information and mark *-key-file-pass configs as sensitive (#12860) · 3c0fd252
      Binbin authored
      In #11489, we consider acl username to be sensitive information,
      and consider the ACL GETUSER a sensitive command and remove it
      from redis-cli historyfile.
      
      This PR redact username information in ACL GETUSER and ACL DELUSER
      from SLOWLOG, and also remove ACL DELUSER from redis-cli historyfile.
      
      This PR also mark tls-key-file-pass and tls-client-key-file-pass
      as sensitive config, will redact it from SLOWLOG and also
      remove them from redis-cli historyfile.
      3c0fd252
    • Chen Tianjie's avatar
      Add metric to INFO CLIENTS: pubsub_clients. (#12849) · f9cc25c1
      Chen Tianjie authored
      In INFO CLIENTS section, we already have blocked_clients and
      tracking_clients. We should add a new metric showing the number of
      pubsub connections, which helps performance monitoring and trouble
      shooting.
      f9cc25c1
  8. 11 Dec, 2023 1 commit
    • Binbin's avatar
      Fix delKeysInSlot server events are not executed inside an execution unit (#12745) · c85a9b78
      Binbin authored
      This is a follow-up fix to #12733. We need to apply the same changes to
      delKeysInSlot. Refer to #12733 for more details.
      
      This PR contains some other minor cleanups / improvements to the test
      suite and docs.
      It uses the postnotifications test module in a cluster mode test which
      revealed a leak in the test module (fixed).
      c85a9b78
  9. 10 Dec, 2023 4 commits
    • Binbin's avatar
      Handle missing fields in dbSwapDatabases and swapMainDbWithTempDb (#12763) · 62419c01
      Binbin authored
      The change in dbSwapDatabases seems harmless. Because in non-clustered
      mode, dbBuckets calculations are strictly accurate and in cluster mode,
      we only have one DB. Modify it for uniformity (just like resize_cursor).
      
      The change in swapMainDbWithTempDb is needed in case we swap with the
      temp db, otherwise the overhead memory usage of db can be miscalculated.
      
      In addition we will swap all fields (including rehashing list), just for
      completeness (and reduce the chance of surprises in the future).
      
      Introduced in #12697.
      62419c01
    • Binbin's avatar
      Fix rehashingStarted miscalculating bucket_count in dict initialization (#12846) · e6423b7a
      Binbin authored
      In the old dictRehashingInfo implementation, for the initialization
      scenario,
      it mistakenly directly set to_size to DICTHT_SIZE(DICT_HT_INITIAL_EXP),
      which
      is 4 in our code by default.
      
      In scenarios where dictExpand directly passes the target size as
      initialization,
      the code will calculate bucket_count incorrectly. For example, in DEBUG
      POPULATE
      or RDB load scenarios, it will cause the final bucket_count to be
      initialized to
      65536 (16384 * 4), see:
      ```
      before:
      DB 0: 10000000 keys (0 volatile) in 65536 slots HT.
      
      it should be:
      DB 0: 10000000 keys (0 volatile) in 16777216 slots HT.
      ```
      
      In PR, new ht will also be initialized before calling rehashingStarted
      in
      _dictExpand, so that the calls in dictRehashingInfo can be unified.
      
      Bug was introduced in #12697.
      e6423b7a
    • Binbin's avatar
      Remove dead code around should_expand_db (#12767) · a3ae2ed3
      Binbin authored
      when dbExpand is called from rdb.c with try_expand set to 0, it will
      either panic panic on OOM, or be non-fatal (should not fail RDB loading)
      
      At the same time, the log text has been slightly adjusted to make it
      more unified.
      a3ae2ed3
    • Binbin's avatar
      Remove overhead.hashtable.slot-to-keys from memory-stats reply_schema (#12784) · 7410d985
      Binbin authored
      overhead.hashtable.slot-to-keys was added in 7.0 in #10017, then removed
      in #11695. Now remove it from reply_schema.
      7410d985
  10. 08 Dec, 2023 2 commits
  11. 07 Dec, 2023 2 commits
    • Chen Tianjie's avatar
      Avoid unnecessary slot computing in KEYS command. (#12843) · f2d59c4f
      Chen Tianjie authored
      If not in cluster mode, there is no need to compute slot.
      
      A bit optimization for #12754
      f2d59c4f
    • zhaozhao.zz's avatar
      Fix replica node cannot expand dicts when loading legacy RDB (#12839) · 8e11f84d
      zhaozhao.zz authored
      When loading RDB on cluster nodes, it is necessary to consider the
      scenario where a node is a replica.
      
      For example, during a rolling upgrade, new version instances are often
      mounted as replicas on old version instances. In this case, the full
      synchronization legacy RDB does not contain slot information, and the
      new version instance, acting as a replica, should be able to handle the
      legacy RDB correctly for `dbExpand`.
      
      Additionally, renaming `getMyClusterSlotCount` to `getMyShardSlotCount`
      would be appropriate.
      
      Introduced in #11695
      8e11f84d
  12. 06 Dec, 2023 5 commits
    • Moshe Kaplan's avatar
      coverity.yml: Upload should go to project redis-unstable (#12841) · e2a3f309
      Moshe Kaplan authored
      Coverity project name was changed from redis to redis-unstable. Fix the
      upload destination to also go to redis-unstable.
      
      Continuation of #12807
      e2a3f309
    • Binbin's avatar
      Fix outdated LFU comments to eliminate confusion (#12244) · 2f6d4dab
      Binbin authored
      The decrement time was replaced by access time in
      583c3147.
      
      The halved and doubled LFU_INIT_VAL logic has been changed in
      06ca9d68.
      Now we just decrement the counter by num_periods. This has been
      previously fixed in redis.conf, #11108.
      2f6d4dab
    • Moshe Kaplan's avatar
      GH Workflows: Create CI job for Coverity scan (#12807) · 77e69d88
      Moshe Kaplan authored
      I've noticed that https://scan.coverity.com/projects/redis already
      exists, but appears to be only updated on an ad-hoc basis. creating
      [redis-unstable](https://scan.coverity.com/projects/redis-unstable?tab=project_settings)
      project in coverity for this CI.
      
      This PR adds a GitHub Action-based CI job to create a new Coverity build
      once daily, so that there is always a recent scan available.
      
      This is within the limit, as Redis is ~150K LOC and per
      https://scan.coverity.com/faq#frequency :
      
      > Up to 21 builds per week, with a maximum of 3 builds per day, for
      projects with 100K to 500K lines of code
      
      Before this is merged in, two new secrets will need to be created:
      
      COVERITY_SCAN_EMAIL with the email address used for accessing Coverity
      COVERITY_SCAN_TOKEN with the Project token from
      https://scan.coverity.com/projects/redis-unstable?tab=project_settings
      
      
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      77e69d88
    • zhaozhao.zz's avatar
      Fix multi dbs donot dbExpand when loading RDB (#12840) · b730404c
      zhaozhao.zz authored
      Currently, during RDB loading, once a `dbExpand` is performed, the
      `should_expand_db` flag is set to 0. This causes the remaining DBs
      unable to do `dbExpand` when there are multiple DBs.
      
      To fix this issue, we need to set `should_expand_db` back to 1 whenever
      we encounter `RDB_OPCODE_RESIZEDB`. This ensures that each DB can
      perform `dbExpand` correctly.
      
      Additionally, the initial value of `should_expand_db` should also be set
      to 0 to prevent invalid `dbExpand` in older versions of RDB where
      `RDB_OPCODE_RESIZEDB` is not present.
      
      problem introduced in #11695
      b730404c
    • zhaozhao.zz's avatar
      Make the sampling logic in eviction clearer (#12781) · 9ee1cc33
      zhaozhao.zz authored
      
      
      Additional optimizations for the eviction logic in #11695:
      
      To make the eviction logic clearer and decouple the number of sampled
      keys from the running mode (cluster or standalone).
      * When sampling in each database, we only care about the number of keys
      in the current database (not the dicts we sampled from).
      * If there are a insufficient number of keys in the current database
      (e.g. 10 times the value of `maxmemory_samples`), we can break out
      sooner (to avoid looping on a sparse database).
      * We'll never try to sample the db dicts more times than the number of
      non-empty dicts in the db (max 1 in non-cluster mode).
      
      And it also ensures that each database has a sufficient amount of
      sampled keys, so even if unsharded-cluster supports multiple databases,
      there won't be any issues.
      
      other changes:
      1. keep track of the number of non-empty dicts in each database.
      2. move key_count tracking into cumulativeKeyCountAdd rather than all
      it's callers
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      9ee1cc33
  13. 05 Dec, 2023 2 commits
  14. 04 Dec, 2023 1 commit
    • Binbin's avatar
      Check whether the client is NULL in luaCreateFunction (#12829) · 764838d6
      Binbin authored
      It was first added to load lua from RDB, see 28dfdca7. After #9812,
      we no longer save lua in RDB. luaCreateFunction will only be called
      in script load and eval*, both of which are available in the client.
      
      It could be that that some day we'll still want to load scripts from
      somewhere that's not a client. This fix is in dead code.
      764838d6
  15. 03 Dec, 2023 1 commit
  16. 30 Nov, 2023 3 commits
  17. 29 Nov, 2023 3 commits
    • Binbin's avatar
      Use CLZ in _dictNextExp to get the next power of two (#12815) · 22cc9b51
      Binbin authored
      In the past, we did not call _dictNextExp frequently. It was only
      called when the dictionary was expanded.
      
      Later, dictTypeExpandAllowed was introduced in #7954, which is 6.2.
      For the data dict and the expire dict, we can check maxmemory before
      actually expanding the dict. This is a good optimization to avoid
      maxmemory being exceeded due to the dict expansion.
      
      And in #11692, we moved the dictTypeExpandAllowed check before the
      threshold check, this caused a bit of performance degradation, every
      time a key is added to the dict, dictTypeExpandAllowed is called to
      check.
      
      The main reason for degradation is that in a large dict, we need to
      call _dictNextExp frequently, that is, every time we add a key, we
      need to call _dictNextExp once. Then the threshold is checked to see
      if the dict needs to be expanded. We can see that the order of checks
      here can be optimized.
      
      So we moved the dictTypeExpandAllowed check back to after the threshold
      check in #12789. In this way, before the dict is actually expanded (that
      is, before the threshold is reached), we will not do anything extra
      compared to before, that is, we will not call _dictNextExp frequently.
      
      But note we'll still hit the degradation when we over the thresholds.
      When the threshold is reached, because #7954, we may delay the dict
      expansion due to maxmemory limitations. In this case, we will call
      _dictNextExp every time we add a key during this period.
      
      This PR use CLZ in _dictNextExp to get the next power of two. CLZ (count
      leading zeros) can easily give you the next power of two. It should be
      noted that we have actually introduced the use of __builtin_clzl in
      #8687,
      which is 7.0. So i suppose all the platforms we use have it (even if the
      CPU doesn't have an instruction).
      
      We build 67108864 (2**26) keys through DEBUG POPULTE, which will use
      approximately 5.49G memory (used_memory:5898522936). If expansion is
      triggered, the additional hash table will consume approximately 1G
      memory (2 ** 27 * 8). So we set maxmemory to 6871947673 (that is, 6.4G),
      which will be less than 5.49G + 1G, so we will delay the dict rehash
      while addint the keys.
      
      After that, each time an element is added to the dict, an allow check
      will be performed, that is, we can frequently call _dictNextExp to test
      the comparison before and after the optimization. Using DEBUG HTSTATS 0
      to
      check and make sure that our dict expansion is dealyed.
      
      Using `./src/redis-server redis.conf --save "" --maxmemory 6871947673`.
      Using `./src/redis-benchmark -P 100 -r 1000000000 -t set -n 5000000`.
      After ten rounds of testing:
      ```
      unstable:           this PR:
      769585.94           816860.00
      771724.00           818196.69
      775674.81           822368.44
      781983.12           822503.69
      783576.25           828088.75
      784190.75           828637.75
      791389.69           829875.50
      794659.94           835660.69
      798212.00           830013.25
      801153.62           833934.56
      ```
      
      We can see there is about 4-5% performance improvement in this case.
      22cc9b51
    • Binbin's avatar
      Fix the check for new RDB_OPCODE_SLOT_INFO in redis-check-rdb (#12768) · bdceaf50
      Binbin authored
      We did not read expires_slot_size, causing its check to fail.
      An overlook in #11695.
      bdceaf50
    • zhaozhao.zz's avatar
      format cpu config as redis style (#7351) · 3431b1f1
      zhaozhao.zz authored
      The following four configurations are renamed to align with Redis style:
      
      1. server_cpulist renamed to server-cpulist
      2. bio_cpulist renamed to bio-cpulist
      3. aof_rewrite_cpulist renamed to aof-rewrite-cpulist
      4. bgsave_cpulist renamed to bgsave-cpulist
      
      The original names are retained as aliases to ensure compatibility with
      old configuration files. We recommend users to gradually transition to
      using the new configuration names to maintain consistency in style.
      3431b1f1
  18. 28 Nov, 2023 2 commits
    • zhaozhao.zz's avatar
      Fix resize hash tables stuck on the last non-empty slot (#12802) · a1c5171c
      zhaozhao.zz authored
      Introduced in #11695 .
      
      The tryResizeHashTables function gets stuck on the last non-empty slot
      while iterating through dictionaries. It does not restart from the
      beginning. The reason for this issue is a problem with the usage of
      dbIteratorNextDict:
      
      /* Returns next dictionary from the iterator, or NULL if iteration is complete. */
      dict *dbIteratorNextDict(dbIterator *dbit) {
          if (dbit->next_slot == -1) return NULL;
          dbit->slot = dbit->next_slot;
          dbit->next_slot = dbGetNextNonEmptySlot(dbit->db, dbit->slot, dbit->keyType);
          return dbGetDictFromIterator(dbit);
      }
      
      When iterating to the last non-empty slot, next_slot is set to -1,
      causing it to loop indefinitely on that slot. We need to modify the code
      to ensure that after iterating to the last non-empty slot, it returns to
      the first non-empty slot.
      
      BTW, function tryResizeHashTables is actually iterating over slots
      that have keys. However, in its implementation, it leverages the
      dbIterator (which is a key iterator) to obtain slot and dictionary
      information. While this approach works fine, but it is not very
      intuitive. This PR also improves readability by changing the iteration
      to directly iterate over slots, thereby enhancing clarity.
      a1c5171c
    • zhaozhao.zz's avatar
      clarify the comment of findSlotByKeyIndex function (#12811) · 095d0578
      zhaozhao.zz authored
      The current comment for `findSlotByKeyIndex` is a bit ambiguous and can
      be misleading, as it may be misunderstood as getting the next slot
      corresponding to target.
      095d0578
  19. 27 Nov, 2023 2 commits
    • Binbin's avatar
      Un-register notification and server event when RedisModule_OnLoad fails (#12809) · d6f19539
      Binbin authored
      When we register notification or server event in RedisModule_OnLoad, but
      RedisModule_OnLoad eventually fails, triggering notification or server
      event
      will cause the server to crash.
      
      If the loading fails on a later stage of moduleLoad, we do call
      moduleUnload
      which handles all un-registration, but when it fails on the
      RedisModule_OnLoad
      call, we only un-register several specific things and these were
      missing:
      
      - moduleUnsubscribeNotifications
      - moduleUnregisterFilters
      - moduleUnsubscribeAllServerEvents
      
      Refactored the code to reuse the code from moduleUnload.
      
      Fixes #12808.
      d6f19539
    • zhaozhao.zz's avatar
      Optimize the efficiency of active expiration when databases exceeds 16. (#12738) · 1bd0b549
      zhaozhao.zz authored
      Currently, when the number of databases exceeds 16,
      the efficiency of cleaning expired keys is relatively low.
      
      The reason is that by default only 16 databases are scanned when
      attempting to clean expired keys (CRON_DBS_PER_CALL is 16). But users
      may set databases higher than 16, such as 256, but it does not
      necessarily mean that all 256 databases have expiration time set. If
      only one database has expiration time set, this database needs 16
      activeExpireCycle rounds in order to be scanned once, and 15 of those
      rounds are meaningless.
      
      To optimize the efficiency of expiration in such scenarios, we use dbs_per_call
      to control the number of databases with expired keys being scanned.
      
      Additionally, add a condition to limit the maximum number of rounds
      to server.dbnum to prevent excessive spinning. This ensures that even if
      only one database has expired keys, it can be triggered within a single cron job.
      1bd0b549