1. 05 Aug, 2024 3 commits
    • Josh Hershberg's avatar
      Make cluster shards cmd implementation generic · 6d5d7541
      Josh Hershberg authored
      
      
      This and the previous commit make the cluster
      shards command a generic implementation instead of a
      specific implementation for each cluster API implementation.
      This commit (a) adds functions to the cluster API
      and (b) modifies the cluster shards cmd implementation
      to use cluster API functions instead of directly
      accessing the legacy clustering implementation.
      Signed-off-by: default avatarJosh Hershberg <yehoshua@redis.com>
      6d5d7541
    • Josh Hershberg's avatar
      Prep to make cluster shards cmd generic · e3e631f3
      Josh Hershberg authored
      
      
      This and the next following commit makes the cluster
      shards command a generic implementation instead of a
      specific implementation for each cluster API implementation.
      This commit simply moves the cluster shards implementation
      from cluster_legacy.c to cluster.c without changing the
      implementation at all. The reason for doing so was to
      help with reviewing the changes in the diff.
      Signed-off-by: default avatarJosh Hershberg <yehoshua@redis.com>
      e3e631f3
    • Zhongxian Pan's avatar
      Replace bit shift with __builtin_ctzll in HyperLogLog (#13218) · 6263823e
      Zhongxian Pan authored
      
      
      ## Replace bit shift with `__builtin_ctzll` in HyperLogLog
      
      Builtin function `__builtin_ctzll` is more effective than bit shift even
      though "in the average case there are high probabilities to find a 1
      after a few iterations" mentioned in the source file comment.
      
      ---------
      Co-authored-by: default avatardebing.sun <debing.sun@redis.com>
      6263823e
  2. 04 Aug, 2024 1 commit
  3. 03 Aug, 2024 1 commit
  4. 01 Aug, 2024 2 commits
  5. 30 Jul, 2024 1 commit
  6. 28 Jul, 2024 1 commit
  7. 25 Jul, 2024 1 commit
  8. 24 Jul, 2024 1 commit
  9. 22 Jul, 2024 1 commit
    • Oran Agra's avatar
      Different fix for the race in #13361 (#13434) · 13d227fa
      Oran Agra authored
      Recently in #13361, i attempted to fix a race between FLUSHALL and
      BGSAVE, where despite calling killRDBChild, the
      backgroundSaveDoneHandler will terminate with success.
      Turns out that even if the child didn't yet exit, there's a chance it'll
      still miss our signal and exit with success.
      in that case, we will still mess up the dirty counter (deducting
      dirty_before_bgsave) which is reset by FLUSHALL, and override the
      synchronous rdb file we saved.
      
      instead, we'll set a flag to treat the next done handler as a failed
      one.
      13d227fa
  10. 16 Jul, 2024 2 commits
    • debing.sun's avatar
      Trigger Lua GC after script loading (#13407) · 88af96c7
      debing.sun authored
      Nowdays we do not trigger LUA GC after loading lua script. This means
      that when a large number of scripts are loaded, such as when functions
      are propagating from the master to the replica, if the LUA scripts are
      never touched on the replica, the garbage might remain there
      indefinitely.
      
      Before this PR, we would share a gc_count between scripts and functions.
      This means that, under certain circumstances, the GC trigger for scripts
      and functions was not fair.
      For example, loading a large number of scripts followed by a small
      number of functions could result in the functions triggering GC.
      In this PR, we assign a unique `gc_count` to each of them, so the GC
      triggers between them will no longer affect each other.
      
      on the other hand, this PR will to bring regession for script loading
      commands(`FUNCTION LOAD` and `SCRIPT LOAD`), but they are not hot path,
      we can ignore it, and it will be replaced
      https://github.com/redis/redis/pull/13375
      
       in the future.
      
      ---------
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      88af96c7
    • debing.sun's avatar
      Prevent deleting RDB read event after restarting RDB saving for other diskless replicas (#13410) · 76415fa2
      debing.sun authored
      When we terminate the diskless RDB saving child process and, at the same
      time, we start a new BGSAVE for new replicas, we should not delete the
      RDB read event. Otherwise, these replicas will never receive a response.
      this is a result of the recent change in
      https://github.com/redis/redis/pull/13361
      
      
      
      ---------
      Co-authored-by: default avataroranagra <oran@redislabs.com>
      76415fa2
  11. 15 Jul, 2024 1 commit
  12. 14 Jul, 2024 1 commit
    • guybe7's avatar
      Crash report: Use more chars for argv (#13413) · b10e19e3
      guybe7 authored
      128 is not enough chars when we're talking about commands like RESTORE.
      Of course, it's impossible to find the perfect number, but 1024 is
      better than 128, and it's not obscenely large.
      b10e19e3
  13. 11 Jul, 2024 2 commits
    • guybe7's avatar
      KSN: HEXPIRE-like commands should emit hdel if expire-time is in the past (#13408) · 915a9e4b
      guybe7 authored
      To be more similar to EXPIRE-like commands, which emit a "del"
      notification if the expire-time is in the past
      915a9e4b
    • guybe7's avatar
      Module unblock on keys: updateStatsOnUnblock is called twice (#13405) · 81440a33
      guybe7 authored
      This commit reverts the deletion of the condition `!bc->blocked_on_keys`
      that was accidentally introduced by
      https://github.com/redis/redis/pull/12817
      
      .
      In case a blocked-on-keys module client is unblocked both
      `moduleUnblockClientOnKey` and `moduleHandleBlockedClients` are called
      which resulted in `updateStatsOnUnblock` being called twice
      
      Now, that `moduleHandleBlockedClients` doesn't call
      `updateStatsOnUnblock` in case of unblocked module key-blocked clients,
      in the unlikely event that the module decides to call `RM_UnblockClient`
      on a key-blocked client, we need to call `updateStatsOnUnblock` from
      within `moduleBlockedClientTimedOut`, but since
      `moduleBlockedClientTimedOut` is not tread-safe we can't call it
      directly from withing `RM_UnblockClient`.
      Added a new flag `blocked_on_keys_explicit_unblock` for that specific
      case, which will cause `moduleBlockedClientTimedOut` to be called from
      `moduleHandleBlockedClients` (which is only called from the main thread)
      
      ---------
      Co-authored-by: default avatardebing.sun <debing.sun@redis.com>
      81440a33
  14. 10 Jul, 2024 1 commit
    • debing.sun's avatar
      Rebuild function engines for function flush command (#13383) · ffff7fea
      debing.sun authored
      
      
      ### Issue
      The current implementation of `FUNCTION FLUSH` command uses
      `lua_unref()` to unreference script closures in Lua vm. However,
      invoking `lua_unref()` during lazy free (`ASYNC` argument) is risky
      since it is not thread-safe.
      
      Another issue is that using `lua_unref()` to unreference references does
      not trigger GC, This can result in the Lua VM leaves a significant
      amount of garbage, which may never be cleaned up if not properly GC.
      
      ### Solution
      The proposed solution is to completely rebuild the engines, resulting in
      a brand new Lua VM.
      
      ---------
      Co-authored-by: default avatarmeir <meir@redis.com>
      ffff7fea
  15. 09 Jul, 2024 1 commit
    • debing.sun's avatar
      Hide user data from log (#13400) · 69b480cb
      debing.sun authored
      
      
      This PR is based on the commits from PR #11747.
      
      In the event of an assertion failure, hide command arguments from the
      operator.
      
      In some cases, private client information can be voluntarily exposed
      when a redis instance crashes due to an assertion failure.
      This commit prevent וnintentional client info exposure.
      Operators can still access the hidden data, but they must actively
      request it.
      Any of the client info commands remains the unchanged.
      
      ### Config
      Add a new config `hide-user-data-from-log` to turn this feature on and
      off, default off.
      
      ---------
      Co-authored-by: default avatarnaglera <anagler123@gmail.com>
      Co-authored-by: default avatarnaglera <58042354+naglera@users.noreply.github.com>
      69b480cb
  16. 07 Jul, 2024 1 commit
    • Moti Cohen's avatar
      HFE - RDB serialize also hash min expiration (For ROF flow) (#13391) · dcf02298
      Moti Cohen authored
      * Following this feature, Redis (ROF) may implement flow that allows hashes to be
        dumped directly from RDB to FLUSH without parsing. In this scenario, it is still
        essential to determine when to update hashes due to expired fields. By writing
        and reading the next minimum hash-field expiration before serializing objects
        to and from RDB, we can effectively track and expire hash fields without the need
        to parse the hash during loading.
      
          Before:
          #define RDB_TYPE_HASH_METADATA 22
          #define RDB_TYPE_HASH_LISTPACK_EX 23
          
          After:
          /* Hash with HFEs. Doesn't attach min TTL at start */
          #define RDB_TYPE_HASH_METADATA_PRE_GA 22      
          /* Hash LP with HFEs. Doesn't attach min TTL at start */
          #define RDB_TYPE_HASH_LISTPACK_EX_PRE_GA 23   
          /* Hash with HFEs. Attach min TTL at start */
          #define RDB_TYPE_HASH_METADATA 24             
          /* Hash LP with HFEs. Attach min TTL at start */
          #define RDB_TYPE_HASH_LISTPACK_EX 25          
      
      
      * Manually test loading RDB file before the change and verify hash and its HFEs 
        is as expected.
      * Added `subexpires` counter to `redis-check-rdb`
      dcf02298
  17. 04 Jul, 2024 1 commit
  18. 03 Jul, 2024 1 commit
    • Filipe Oliveira (Redis)'s avatar
      Reduce getNodeByQuery overhead (#13221) · 26a2dcb9
      Filipe Oliveira (Redis) authored
      
      
      The following PR does the following changes based upon on CPU profile
      info. The `getNodeByQuery` function represents 8.2% of an overhead of
      12.3% when comparing single shard cluster with standalone.
      Proposed changes:
      - inlinging keyHashSlot to reduce overhead of that function call
      - Reduce duplicate calls to getCommandFlags within getNodeByQuery
      
      The above changes represent an improvement of approximately 5% on the achievable ops/sec.
      Co-authored-by: default avatarfilipecosta90 <filipecosta.90@gmail.com>
      26a2dcb9
  19. 02 Jul, 2024 1 commit
  20. 01 Jul, 2024 2 commits
    • Oran Agra's avatar
      Solve a race between BGSAVE and FLUSHALL messing up the dirty counter (#13361) · 799c5e5f
      Oran Agra authored
      If we run FLUSHALL when the 'save' config is set, and there's a fork
      child ding BGSAVE, there's a chance the child is already finished, and
      the parent process is unaware of it. in that case the child will not get
      the kill signal and will finish successfully, but the parent process
      thinks it killed it and will reset the dirty counter to 0, then the
      backgroundSaveDoneHandlerDisk method can set the dirty counter to a
      negative value.
      799c5e5f
    • Oran Agra's avatar
      Fix possible crash due to OOM panic on invalid command (#13380) · 69b7137d
      Oran Agra authored
      getKeysUsingKeySpece had the range check AFTER the allocation, of the
      keys buffer, which could lead to an OOM panic when invalid arguments are
      provided, leading to an overflow.
      The allocated memory is only used after the range check, so there's no
      risk of buffer overrun.
      The OOM panic can happen on 32bit builds, or 64 builds running on
      systems with less than 4GB of RAM, and is reachable via the COMMAND
      GETKEYSANDFLAGS, and ACL key name validation.
      69b7137d
  21. 26 Jun, 2024 2 commits
    • Moti Cohen's avatar
      HFE - count in command must match actual number of fields (#13369) · a9267137
      Moti Cohen authored
      There was wrong preliminary assumption that we can optionally provide
      vector of arguments more than count.
      This is error-prone approach that leaded to actual error in that case.
      This PR enforce that vector of argument match count.
      
      Also fixed flaky HRANDFIELD test.
      a9267137
    • debing.sun's avatar
      Don't keep global replication buffer reference for replicas marked CLIENT_CLOSE_ASAP (#13363) · 52e12d8b
      debing.sun authored
      
      
      In certain situations, we might generate a large number of propagates
      (e.g., multi/exec, Lua script, or a single command generating tons of
      propagations) within an event loop.
      During the process of propagating to a replica, if the replica is
      disconnected(marked as CLIENT_CLOSE_ASAP) due to exceeding the output
      buffer limit, we should remove its reference to the global replication
      buffer to avoid the global replication buffer being unable to be
      properly trimmed due to being referenced.
      
      ---------
      Co-authored-by: default avataroranagra <oran@redislabs.com>
      52e12d8b
  22. 25 Jun, 2024 1 commit
    • Moti Cohen's avatar
      Fix H(P)EXPIREAT command to propagate HDEL as well (#13364) · 5eac99c3
      Moti Cohen authored
      H(P)EXPIREAT command might delete fields in case the absolute time is in the 
      past. Those HDELs need to be propagated as well.
       
      In general, as we need to propagate H(P)EXPIRE(AT) command to the replica, each 
      field that is mentioned in the command should be categorized into one of the four
      options:
      1. Managed to update field’s expiration time - propagate it to replica as part 
         of the HPEXPIREAT command.
      2. Deleted the field because the time is in the past - propagate also HDEL command
         to delete the field and remove the field from the propagated HPEXPIREAT.
      3. Condition not met for the field - Remove the field from the propagated
         HPEXPIREAT command.
      4. Field does not exists - Remove the field from the propagated HPEXPIREAT command.
      
      If none of the provided fields match option number 1, then avoid also propagating 
      the HPEXPIREAT command to the replica.
      
      This approach is aligned with the EXPIRE command:
      If a given key has already expired, then DEL will be propagated instead of
      EXPIRE command. If condition not met, then command will be rejected. Otherwise, 
      EXPIRE command will be propagated for given key.
      5eac99c3
  23. 24 Jun, 2024 1 commit
    • Moti Cohen's avatar
      Adapt HRANDFIELD to HFE feature (#13348) · e26ea35c
      Moti Cohen authored
      Considerations for the selected imp of HRANDFIELD & HFE feature:
      
      HRANDFIELD might access any of the fields in the hash as some of them
      might be expired. And so the Implementation of HRANDFIELD along with HFEs
      might be one of the two options:
      1. Expire hash-fields before diving into handling HRANDFIELD.
      2. Refine HRANDFIELD cases to deal with expired fields.
      
      Regarding the first option, as reference, the command RANDOMKEY also
      declareson O(1) complexity, yet might be stuck on a very long (but not infinite)
      loop trying to find non-expired keys. Furthermore RANDOMKEY also evicts expired 
      keys along the way even though it is categorized as a read-only command. Note
      that the case of HRANDFIELD is more lightweight versus RANDOMKEY since 
      HFEs have much more effective and aggressive active-expiration for fields behind.
      
      The second option introduces additional implementation complexity to HRANDFIELD.
      We could further refine HRANDFIELD cases to differentiate between scenarios
      with many expired fields versus few expired fields, and adjust based on the
      percentage of expired fields. However, this approach could still lead to long
      loops or necessitate expiring fields before selecting them. For the “lightweight”
      cases it is also expected to have a lightweight expiration.
      
      Considering the pros and cons, and the fact that HRANDFIELD is an infrequent
      command (particularly with HFEs) and the fact we have effective active-expiration
      behind for hash-fields, it is better to keep it simple and choose option number 1.
      
      Other changes:
      * Don't mark command dirty by internal hashTypeExpire(). It causes to read 
        only command of HRANDFIELD to be accidently propagated (This flag
        should be indicated at higher level, by the command functions).
      * Align `hashTypeExpireIfNeeded()` and `hashTypeGetValue()` to be more
        aligned with `expireIfNeeded()` logic of keyspace.
      e26ea35c
  24. 21 Jun, 2024 2 commits
  25. 20 Jun, 2024 1 commit
    • Moti Cohen's avatar
      Fix rdbLoadObject() empty hash (#13347) · e18a173a
      Moti Cohen authored
      As part of HFE feature, the logic of rdbLoadObject() was wrongly
      modified to indicate of loaded empty hash from RDB as hash that all its
      fields got expired. Rollback to `emptykey` logic. This function should
      load blindly all fields, expired or not. Manually verified.
      
      Few more minor fixes:
      - remove hash double check of emptyKey
      - Fix from `sds` to `hfield` in rdbLoadObject() (not really a bug. Both
      are of type char*)
      - Revert code rdbLoadObject() to get dbid instead of db
      e18a173a
  26. 18 Jun, 2024 1 commit
    • Filipe Oliveira (Redis)'s avatar
      reduce getNodeByQuery CPU time by using less cache lines (from 2064 Bytes... · 24c85cc3
      Filipe Oliveira (Redis) authored
      
      reduce getNodeByQuery CPU time by using less cache lines (from 2064 Bytes struct to 64 Bytes): reduces LLC misses and Memory Loads (#13296)
      
      The following PR goes from 33 cacheline on getKeysResult struct (by
      default has 256 static buffer)
      
      ```
      root@hpe10:~/redis# pahole -p   ./src/server.o -C getKeysResult
      typedef struct {
      	keyReference               keysbuf[256];         /*     0  2048 */
      	/* --- cacheline 32 boundary (2048 bytes) --- */
      	/* typedef keyReference */ struct {
      		int                pos;
      		int                flags;
      	} *keys; /*  2048     8 */
      	int                        numkeys;              /*  2056     4 */
      	int                        size;                 /*  2060     4 */
      
      	/* size: 2064, cachelines: 33, members: 4 */
      	/* last cacheline: 16 bytes */
      } getKeysResult;
      ```
      
      
      to 1 cacheline with a static buffer of 6 keys per command):
      ```
      root@hpe10:~/redis# pahole -p   ./src/server.o -C getKeysResult
      typedef struct {
      	int                        numkeys;              /*     0     4 */
      	int                        size;                 /*     4     4 */
      	keyReference               keysbuf[6];           /*     8    48 */
      	/* typedef keyReference */ struct {
      		int                pos;
      		int                flags;
      	} *keys; /*    56     8 */
      
      	/* size: 64, cachelines: 1, members: 4 */
      } getKeysResult; 
      ```
      
      we get around 1.5% higher ops/sec, and a confirmation of around 15% less
      LLC loads on getNodeByQuery and 37% less Stores.
      
      Function / Call Stack | CPU Time: Difference | CPU Time:
      9462436fa444e746716845b1d807c74d8945831b | CPU Time: this PR | Loads:
      Difference | Loads: 9462436fa444e746716845b1d807c74d8945831b | Loads:
      this PR | Stores: Difference | Stores:
      9462436fa444e746716845b1d807c74d8945831b | Stores: This PR
      -- | -- | -- | -- | -- | -- | -- | -- | -- | --
      getNodeByQuery | 0.753767 | 1.57118 | 0.817416 | 144297829 (15% less
      loads) | 920575969 | 776278140 | 367607824 (37% less stores) | 991642384
      | 624034560
      
      ## results on client side
      
      ### baseline 
      ```
      taskset -c 2,3 memtier_benchmark -s 192.168.1.200 --port 6379 --authenticate perf --cluster-mode --pipeline 10 --data-size 100 --ratio 1:0 --key-pattern P:P --key-minimum=1 --key-maximum 1000000 --test-time 180 -c 25 -t 2 --hide-histogram 
      Writing results to stdout
      [RUN #1] Preparing benchmark client...
      [RUN #1] Launching threads now...
      [RUN #1 100%, 180 secs]  0 threads:   110333450 ops,  604992 (avg:  612942) ops/sec, 84.75MB/sec (avg: 85.86MB/sec),  0.82 (avg:  0.81) msec latency
      
      2         Threads
      25        Connections per thread
      180       Seconds
      
      
      ALL STATS
      ======================================================================================================================================================
      Type         Ops/sec     Hits/sec   Misses/sec    MOVED/sec      ASK/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
      ------------------------------------------------------------------------------------------------------------------------------------------------------
      Sets       612942.14          ---          ---         0.00         0.00         0.81332         0.80700         1.26300         2.92700     87924.12 
      Gets            0.00         0.00         0.00         0.00         0.00             ---             ---             ---             ---         0.00 
      Waits           0.00          ---          ---          ---          ---             ---             ---             ---             ---          --- 
      Totals     612942.14         0.00         0.00         0.00         0.00         0.81332         0.80700         1.26300         2.92700     87924.12 
      ```
      
      ### comparison 
      ```
      taskset -c 2,3 memtier_benchmark -s 192.168.1.200 --port 6379 --authenticate perf --cluster-mode --pipeline 10 --data-size 100 --ratio 1:0 --key-pattern P:P --key-minimum=1 --key-maximum 1000000 --test-time 180 -c 25 -t 2 --hide-histogram 
      Writing results to stdout
      [RUN #1] Preparing benchmark client...
      [RUN #1] Launching threads now...
      [RUN #1 100%, 180 secs]  0 threads:   111731310 ops,  610195 (avg:  620707) ops/sec, 85.48MB/sec (avg: 86.95MB/sec),  0.82 (avg:  0.80) msec latency
      
      2         Threads
      25        Connections per thread
      180       Seconds
      
      
      ALL STATS
      ======================================================================================================================================================
      Type         Ops/sec     Hits/sec   Misses/sec    MOVED/sec      ASK/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
      ------------------------------------------------------------------------------------------------------------------------------------------------------
      Sets       620707.72          ---          ---         0.00         0.00         0.80312         0.79900         1.23900         2.87900     89037.78 
      Gets            0.00         0.00         0.00         0.00         0.00             ---             ---             ---             ---         0.00 
      Waits           0.00          ---          ---          ---          ---             ---             ---             ---             ---          --- 
      Totals     620707.72         0.00         0.00         0.00         0.00         0.80312         0.79900         1.23900         2.87900     89037.78
      ```
      Co-authored-by: default avatarfilipecosta90 <filipecosta.90@gmail.com>
      24c85cc3
  27. 14 Jun, 2024 2 commits
    • Ozan Tezcan's avatar
      Reply with array of return codes if the key does not exist for HFE commands (#13343) · 4aa25d04
      Ozan Tezcan authored
      Currently, HFE commands reply with empty array if the key does not
      exist. Though, non-existing key and empty key is the same thing. 
      It means fields given in the command do not exist in the empty key. 
      So, replying with an array of 'no field' error codes (-2) suits better 
      to Redis logic. e.g. Similarly, `hmget` returns array of nulls if the 
      key does not exist.
      
      After this PR:
      ```
      127.0.0.1:6379> hpersist missingkey fields 2 a b
      1) (integer) -2
      2) (integer) -2
      ```
      4aa25d04
    • Jo's avatar
      Update `FIELDS` argument to block type for HFE commands schema (#13339) · 871c9859
      Jo authored
      I reviewed `XREAD` command syntax:
      ```
      XREAD [COUNT count] [BLOCK milliseconds] STREAMS key [key ...] id [id ...]
      ```
      
      Here’s the structure for `XREAD`:
      ```json
      "arguments": [
                  {
                      "token": "COUNT",
                      "name": "count",
                      "type": "integer",
                      "optional": true
                  },
                  {
                      "token": "BLOCK",
                      "name": "milliseconds",
                      "type": "integer",
                      "optional": true
                  },
                  {
                      "name": "streams",
                      "token": "STREAMS",
                      "type": "block",
                      "arguments": [
                          {
                              "name": "key",
                              "type": "key",
                              "key_spec_index": 0,
                              "multiple": true
                          },
                          {
                              "name": "ID",
                              "type": "string",
                              "multiple": true
                          }
                      ]
                  }
      ]
      ```
      
      Now, consider the `HEXPIRE` syntax:
      ```
      HEXPIRE key seconds [NX | XX | GT | LT] FIELDS numfields field [field ...]
      ```
      
      Since the `FIELDS` token functions similarly to `STREAMS`, and given that `STREAMS` is defined as a block, I believe the `FIELDS` in `hepxire` should also be defined as a block.
      871c9859
  28. 11 Jun, 2024 1 commit
    • debing.sun's avatar
      Add new hexpired notification for HFE (#13329) · ed10f737
      debing.sun authored
      
      
      When the hash field expired, we will send a new `hexpired` notification.
      It mainly includes the following three cases:
      1. When field expired by active expiration.
      2. When field expired by lazy expiration.
      3. When the user uses the `h(p)expire(at)` command, the user will also
      get a `hexpired` notification if the field expires during the command.
      
      ## Improvement
      1. Now if more than one field expires in the hmget command, we will only
      send a `hexpired` notification.
      2. When a field with TTL is deleted by commands like hdel without
      updating the global DS, active expire will not send a notification.
      
      ---------
      Co-authored-by: default avatarOzan Tezcan <ozantezcan@gmail.com>
      Co-authored-by: default avatarMoti Cohen <moti.cohen@redis.com>
      ed10f737
  29. 10 Jun, 2024 2 commits
    • Moti Cohen's avatar
      Reserve 2 bits out of EB_EXPIRE_TIME_MAX for possible future use (#13331) · f01fdc39
      Moti Cohen authored
      Reserve 2 bits out of hash-field expiration time (`EB_EXPIRE_TIME_MAX`)
      for possible future lightweight indexing/categorizing of fields. It can
      be achieved by hacking HFE as follows:
      ```
      HPEXPIREAT key [ 2^47 + USER_INDEX ] FIELDS numfields field [field …]
      ```
      
      Redis will also need to expose kind of `HEXPIRESCAN` and `HEXPIRECOUNT`
      for this idea. Yet to be better defined.
      
      `HFE_MAX_ABS_TIME_MSEC` constraint must be enforced only at API level.
      Internally, the expiration time can be up to `EB_EXPIRE_TIME_MAX` for
      future readiness.
      f01fdc39
    • Moti Cohen's avatar
      HFE - Avoid lazy expire if called by modules + cleanup (#13326) · ce121b92
      Moti Cohen authored
      Need to be carefull if called by modules since modules API allow to open
      and close key handler. We don't want to invalidate the handler
      underneath.
      
      * hashTypeExists(), hashTypeGetValueObject() - will return the logical
      state of the field. A flag will indicate noExpire.
      * RM_HashGet() - Will get NULL if the field expired. Fields won’t be
      deleted.
      * RM_ScanKey() - might return 0 items if all fields got expired. Fields
      won’t be deleted.
      * RM_HashSet() - If set, then override expired field. If delete, we can
      either delete or leave it to active-expiration. XX/NX - logically
      correct (Verify with tests).
      
      Nice to have (not implemented):
      * RedisModule_CloseKey() - We can local active-expire up-to 100 items. 
      
      Note:
      Length will be wrong to modules just like redis (Count expired fields).
      ce121b92
  30. 04 Jun, 2024 1 commit
    • debing.sun's avatar
      Prevent negative expire parameter in HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT commands (#13310) · 9a2c6ba4
      debing.sun authored
      
      
      1. Don't allow HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT command expire
      parameters is negative
      
      2. Remove a dead code reported from Coverity.
      when `unit` is not `UNIT_SECONDS`, the second `if (expire > (long long)
      EB_EXPIRE_TIME_MAX)` will be dead code.
      ```c
      # t_hash.c
      2988    /* Check expire overflow */
            	cond_at_most: Condition expire > 281474976710655LL, taking false branch. Now the value of expire is at most 281474976710655.
      2989    if (expire > (long long) EB_EXPIRE_TIME_MAX) {
      2990        addReplyErrorExpireTime(c);
      2991        return;
      2992    }
      
      2994    if (unit == UNIT_SECONDS) {
      2995        if (expire > (long long) EB_EXPIRE_TIME_MAX / 1000) {
      2996            addReplyErrorExpireTime(c);
      2997            return;
      2998        }
      2999        expire *= 1000;
      3000    } else {
            	at_most: At condition expire > 281474976710655LL, the value of expire must be at most 281474976710655.
            	dead_error_condition: The condition expire > 281474976710655LL cannot be true.
      3001        if (expire > (long long) EB_EXPIRE_TIME_MAX) {
            	
      CID 494223: (#1 of 1): Logically dead code (DEADCODE)
      dead_error_begin: Execution cannot reach this statement: addReplyErrorExpireTime(c);.
      3002            addReplyErrorExpireTime(c);
      3003            return;
      3004        }
      3005    }
      ```
      
      ---------
      Co-authored-by: default avatarOzan Tezcan <ozantezcan@gmail.com>
      9a2c6ba4