1. 01 Nov, 2022 1 commit
  2. 27 Oct, 2022 2 commits
    • Moti Cohen's avatar
      Refactor and (internally) rebrand from pause-clients to pause-actions (#11098) · c0d72262
      Moti Cohen authored
      Renamed from "Pause Clients" to "Pause Actions" since the mechanism can pause
      several actions in redis, not just clients (e.g. eviction, expiration).
      
      Previously each pause purpose (which has a timeout that's tracked separately from others purposes),
      also implicitly dictated what it pauses (reads, writes, eviction, etc). Now it is explicit, and
      the actions that are paused (bit flags) are defined separately from the purpose.
      
      - Previously, when using feature pause-client it also implicitly means to make the server static:
        - Pause replica traffic
        - Pauses eviction processing
        - Pauses expire processing
      
      Making the server static is used also for failover and shutdown. This PR internally rebrand
      pause-client API to become pause-action API. It also Simplifies pauseClients structure
      by replacing pointers array with static array.
      
      The context of this PR is to add another trigger to pause-client which will activated in case
      of OOM as throttling mechanism ([see here](https://github.com/redis/redis/issues/10907)).
      In this case we want only to pause client, and eviction actions.
      c0d72262
    • Shaya Potter's avatar
      RM_Call - only enforce OOM on scripts if 'M' flag is sent (#11425) · 38028dab
      Shaya Potter authored
      
      
      RM_Call is designed to let modules call redis commands disregarding the
      OOM state (the module is responsible to declare its command flags to redis,
      or perform the necessary checks).
      The other (new) alternative is to pass the "M" flag to RM_Call so that redis can
      OOM reject commands implicitly.
      
      However, Currently, RM_Call enforces OOM on scripts (excluding scripts that
      declared `allow-oom`) in all cases, regardless of the RM_Call "M" flag being present.
      
      This PR fixes scripts to be consistent with other commands being executed by RM_Call.
      It modifies the flow in effect treats scripts as if they if they have the ALLOW_OOM script
      flag, if the "M" flag is not passed (i.e. no OOM checking is being performed by RM_Call,
      so no OOM checking should be done on script).
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      38028dab
  3. 25 Oct, 2022 1 commit
  4. 24 Oct, 2022 1 commit
    • guybe7's avatar
      Set errno in case XADD with partial ID fails (#11424) · 737a0905
      guybe7 authored
      This is a rare failure mode of a new feature of redis 7 introduced in #9217
      (when the incremental part of the ID overflows).
      Till now, the outcome of that error was undetermined (could easily result in
      `Elements are too large to be stored` wrongly, due to unset `errno`).
      737a0905
  5. 22 Oct, 2022 2 commits
    • Binbin's avatar
      Make PFMERGE source key optional in docs, add tests with one input key, add... · 9e1b879f
      Binbin authored
      Make PFMERGE source key optional in docs, add tests with one input key, add tests on missing source keys (#11205)
      
      The following two cases will create an empty destkey HLL:
      1. called with no source keys, like `pfmerge destkey`
      2. called with non-existing source keys, like `pfmerge destkey non-existing-source-key`
      
      In the first case, in `PFMERGE`, the dest key is actually one of the source keys too.
      So `PFMERGE k1 k2` is equivalent to `SUNIONSTORE k1 k1 k2`,
      and `PFMERGE k1` is equivalent to `SUNIONSTORE k1 k1`.
      So the first case is reasonable, the source key is actually optional.
      
      And the second case, `PFMERGE` on missing keys should succeed and create an empty dest.
      This is consistent with `PFCOUNT`, and also with `SUNIONSTORE`, no need to change.
      9e1b879f
    • sundb's avatar
      Fix crash due to to reuse iterator entry after list deletion in module (#11383) · 6dd21355
      sundb authored
      
      
      In the module, we will reuse the list iterator entry for RM_ListDelete, but `listTypeDelete` will only update
      `quicklistEntry->zi` but not `quicklistEntry->node`, which will result in `quicklistEntry->node` pointing to
      a freed memory address if the quicklist node is deleted. 
      
      This PR sync `key->u.list.index` and `key->u.list.entry` to list iterator after `RM_ListDelete`.
      
      This PR also optimizes the release code of the original list iterator.
      Co-authored-by: default avatarViktor Söderqvist <viktor@zuiderkwast.se>
      6dd21355
  6. 18 Oct, 2022 2 commits
    • guybe7's avatar
      Blocked module clients should be aware when a key is deleted (#11310) · b57fd010
      guybe7 authored
      The use case is a module that wants to implement a blocking command on a key that
      necessarily exists and wants to unblock the client in case the key is deleted (much like
      what we implemented for XREADGROUP in #10306)
      
      New module API:
      * RedisModule_BlockClientOnKeysWithFlags
      
      Flags:
      * REDISMODULE_BLOCK_UNBLOCK_NONE
      * REDISMODULE_BLOCK_UNBLOCK_DELETED
      
      ### Detailed description of code changes
      
      blocked.c:
      1. Both module and stream functions are called whether the key exists or not, regardless of
        its type. We do that in order to allow modules/stream to unblock the client in case the key
        is no longer present or has changed type (the behavior for streams didn't change, just code
        that moved into serveClientsBlockedOnStreamKey)
      2. Make sure afterCommand is called in serveClientsBlockedOnKeyByModule, in order to propagate
        actions from moduleTryServeClientBlockedOnKey.
      3. handleClientsBlockedOnKeys: call propagatePendingCommands directly after lookupKeyReadWithFlags
        to prevent a possible lazy-expire DEL from being mixed with any command propagated by the
        preceding functions.
      4. blockForKeys: Caller can specifiy that it wants to be awakened if key is deleted.
         Minor optimizations (use dictAddRaw).
      5. signalKeyAsReady became signalKeyAsReadyLogic which can take a boolean in case the key is deleted.
        It will only signal if there's at least one client that awaits key deletion (to save calls to
        handleClientsBlockedOnKeys).
        Minor optimizations (use dictAddRaw)
      
      db.c:
      1. scanDatabaseForDeletedStreams is now scanDatabaseForDeletedKeys and will signalKeyAsReady
        for any key that was removed from the database or changed type. It is the responsibility of the code
        in blocked.c to ignore or act on deleted/type-changed keys.
      2. Use the new signalDeletedKeyAsReady where needed
      
      blockedonkey.c + tcl:
      1. Added test of new capabilities (FSL.BPOPGT now requires the key to exist in order to work)
      b57fd010
    • Meir Shpilraien (Spielrein)'s avatar
      Avoid saving module aux on RDB if no aux data was saved by the module. (#11374) · b43f2548
      Meir Shpilraien (Spielrein) authored
      ### Background
      
      The issue is that when saving an RDB with module AUX data, the module AUX metadata
      (moduleid, when, ...) is saved to the RDB even though the module did not saved any actual data.
      This prevent loading the RDB in the absence of the module (although there is no actual data in
      the RDB that requires the module to be loaded).
      
      ### Solution
      
      The solution suggested in this PR is that module AUX will be saved on the RDB only if the module
      actually saved something during `aux_save` function.
      
      To support backward compatibility, we introduce `aux_save2` callback that acts the same as
      `aux_save` with the tiny change of avoid saving the aux field if no data was actually saved by
      the module. Modules can use the new API to make sure that if they have no data to save,
      then it will be possible to load the created RDB even without the module.
      
      ### Concerns
      
      A module may register for the aux load and save hooks just in order to be notified when
      saving or loading starts or completed (there are better ways to do that, but it still possible
      that someone used it).
      
      However, if a module didn't save a single field in the save callback, it means it's not allowed
      to read in the read callback, since it has no way to distinguish between empty and non-empty
      payloads. furthermore, it means that if the module did that, it must never change it, since it'll
      break compatibility with it's old RDB files, so this is really not a valid use case.
      
      Since some modules (ones who currently save one field indicating an empty payload), need
      to know if saving an empty payload is valid, and if Redis is gonna ignore an empty payload
      or store it, we opted to add a new API (rather than change behavior of an existing API and
      expect modules to check the redis version)
      
      ### Technical Details
      
      To avoid saving AUX data on RDB, we change the code to first save the AUX metadata
      (moduleid, when, ...) into a temporary buffer. The buffer is then flushed to the rio at the first
      time the module makes a write operation inside the `aux_save` function. If the module saves
      nothing (and `aux_save2` was used), the entire temporary buffer is simply dropped and no
      data about this AUX field is saved to the RDB. This make it possible to load the RDB even in
      the absence of the module.
      
      Test was added to verify the fix.
      b43f2548
  7. 16 Oct, 2022 2 commits
    • Shaya Potter's avatar
      Unify ACL failure error messaging. (#11160) · 3193f086
      Shaya Potter authored
      Motivation: for applications that use RM ACL verification functions, they would
      want to return errors back to the user, in ways that are consistent with Redis.
      While investigating how we should return ACL errors to the user, we realized that
      Redis isn't consistent, and currently returns ACL error strings in 3 primary ways.
      
      [For the actual implications of this change, see the "Impact" section at the bottom]
      
      1. how it returns an error when calling a command normally
         ACL_DENIED_CMD -> "this user has no permissions to run the '%s' command"
         ACL_DENIED_KEY -> "this user has no permissions to access one of the keys used as arguments"
         ACL_DENIED_CHANNEL -> "this user has no permissions to access one of the channels used as arguments"
      
      2. how it returns an error when calling via 'acl dryrun' command
         ACL_DENIED_CMD ->  "This user has no permissions to run the '%s' command"
         ACL_DENIED_KEY -> "This user has no permissions to access the '%s' ke...
      3193f086
    • Meir Shpilraien (Spielrein)'s avatar
      Fix wrong replication on cluster slotmap changes with module KSN propagation (#11377) · 56f97bfa
      Meir Shpilraien (Spielrein) authored
      As discussed on #11084, `propagatePendingCommands` should happened after the del
      notification is fired so that the notification effect and the `del` will be replicated inside MULTI EXEC.
      
      Test was added to verify the fix.
      56f97bfa
  8. 15 Oct, 2022 1 commit
    • filipe oliveira's avatar
      optimizing d2string() and addReplyDouble() with grisu2: double to string... · 29380ff7
      filipe oliveira authored
      optimizing d2string() and addReplyDouble() with grisu2: double to string conversion based on Florian Loitsch's Grisu-algorithm (#10587)
      
      All commands / use cases that heavily rely on double to a string representation conversion,
      (e.g. meaning take a double-precision floating-point number like 1.5 and return a string like "1.5" ),
      could benefit from a performance boost by swapping snprintf(buf,len,"%.17g",value) by the
      equivalent [fpconv_dtoa](https://github.com/night-shift/fpconv) or any other algorithm that ensures
      100% coverage of conversion.
      
      This is a well-studied topic and Projects like MongoDB. RedPanda, PyTorch leverage libraries
      ( fmtlib ) that use the optimized double to string conversion underneath.
      
      
      The positive impact can be substantial. This PR uses the grisu2 approach ( grisu explained on
      https://www.cs.tufts.edu/~nr/cs257/archive/florian-loitsch/printf.pdf section 5 ). 
      
      test suite changes:
      Despite being compatible, in some cases it produces a different result from printf, and some tests
      had to be adjusted.
      one case is that `%.17g` (which means %e or %f which ever is shorter), chose to use `5000000000`
      instead of 5e+9, which sounds like a bug?
      In other cases, we changed TCL to compare numbers instead of strings to ignore minor rounding
      issues (`expr 0.8 == 0.79999999999999999`) 
      29380ff7
  9. 13 Oct, 2022 1 commit
    • C Charles's avatar
      MIGTATE with AUTH that contains "keys" is getting wrong key names in... · 9ab873d9
      C Charles authored
      MIGTATE with AUTH that contains "keys" is getting wrong key names in migrateGetKeys, leads to ACL errors (#11253)
      
      When using the MIGRATE, with a destination Redis that has the user name or password set to the string "keys",
      Redis would have determine the wrong set of key names the command is gonna access.
      This lead to ACL returning wrong authentication result.
      
      Destination instance:
      ```
      127.0.0.1:6380> acl setuser default >keys
      OK
      127.0.0.1:6380> acl setuser keys on nopass ~* &* +@all
      OK
      ```
      
      Source instance:
      ```
      127.0.0.1:6379> set a 123
      OK
      127.0.0.1:6379> acl setuser cc on nopass ~a* +@all
      OK
      127.0.0.1:6379> auth cc 1
      OK
      127.0.0.1:6379> migrate 127.0.0.1 6380 "" 0 1000 auth keys keys a
      (error) NOPERM this user has no permissions to access one of the keys used as arguments
      127.0.0.1:6379> migrate 127.0.0.1 6380 "" 0 1000 auth2 keys pswd keys a
      (error) NOPERM this user has no permissions to access one of the keys used as arguments
      ```
      
      Using `acl dryrun` we know that the parameters of `auth` and `auth2` are mistaken for the `keys` option.
      ```
      127.0.0.1:6379> acl dryrun cc migrate whatever whatever "" 0 1000 auth keys keys a
      "This user has no permissions to access the 'keys' key"
      127.0.0.1:6379> acl dryrun cc migrate whatever whatever "" 0 1000 auth2 keys pswd keys a
      "This user has no permissions to access the 'pswd' key"
      ```
      
      Fix the bug by editing db.c/migrateGetKeys function, which finds the `keys` option and all the keys following.
      9ab873d9
  10. 12 Oct, 2022 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Fix crash on RM_Call inside module load (#11346) · eb6accad
      Meir Shpilraien (Spielrein) authored
      PR #9320 introduces initialization order changes. Now cluster is initialized after modules.
      This changes causes a crash if the module uses RM_Call inside the load function
      on cluster mode (the code will try to access `server.cluster` which at this point is NULL).
      
      To solve it, separate cluster initialization into 2 phases:
      1. Structure initialization that happened before the modules initialization
      2. Listener initialization that happened after.
      
      Test was added to verify the fix.
      eb6accad
  11. 09 Oct, 2022 3 commits
    • Binbin's avatar
      Fix TIME command microseconds overflow under 32-bits (#11368) · 1cc511d7
      Binbin authored
      The old `server.unixtime*1000000` will overflow in 32-bits.
      This was introduced in #10300 (not released).
      1cc511d7
    • Binbin's avatar
      Freeze time sampling during command execution, and scripts (#10300) · 35b3fbd9
      Binbin authored
      Freeze time during execution of scripts and all other commands.
      This means that a key is either expired or not, and doesn't change
      state during a script execution. resolves #10182
      
      This PR try to add a new `commandTimeSnapshot` function.
      The function logic is extracted from `keyIsExpired`, but the related
      calls to `fixed_time_expire` and `mstime()` are removed, see below.
      
      In commands, we will avoid calling `mstime()` multiple times
      and just use the one that sampled in call. The background is,
      e.g. using `PEXPIRE 1` with valgrind sometimes result in the key
      being deleted rather than expired. The reason is that both `PEXPIRE`
      command and `checkAlreadyExpired` call `mstime()` separately.
      
      There are other more important changes in this PR:
      1. Eliminate `fixed_time_expire`, it is no longer needed. 
         When we want to sample time we should always use a time snapshot. 
         We will use `in_nested_call` instead to update the cached time in `call`.
      2. Move the call for `updateCachedTime` from `serverCron` to `afterSleep`.
          Now `commandTimeSnapshot` will always return the sample time, the
          `lookupKeyReadWithFlags` call in `getNodeByQuery` will get a outdated
          cached time (because `processCommand` is out of the `call` context).
          We put the call to `updateCachedTime` in `aftersleep`.
      3. Cache the time each time the module lock Redis.
          Call `updateCachedTime` in `moduleGILAfterLock`, affecting `RM_ThreadSafeContextLock`
          and `RM_ThreadSafeContextTryLock`
      
      Currently the commandTimeSnapshot change affects the following TTL commands:
      - SET EX / SET PX
      - EXPIRE / PEXPIRE
      - SETEX / PSETEX
      - GETEX EX / GETEX PX
      - TTL / PTTL
      - EXPIRETIME / PEXPIRETIME
      - RESTORE key TTL
      
      And other commands just use the cached mstime (including TIME).
      
      This is considered to be a breaking change since it can break a script
      that uses a loop to wait for a key to expire.
      35b3fbd9
    • Meir Shpilraien (Spielrein)'s avatar
      `RedisModule_ResetDataset` should not clear the functions. (#11268) · d2ad01ab
      Meir Shpilraien (Spielrein) authored
      As mentioned on docs, `RM_ResetDataset` Performs similar operation to FLUSHALL.
      As FLUSHALL do not clean the function, `RM_ResetDataset` should not clean the functions
      as well.
      d2ad01ab
  12. 07 Oct, 2022 1 commit
  13. 06 Oct, 2022 1 commit
  14. 03 Oct, 2022 2 commits
    • Madelyn Olson's avatar
      Stabilize cluster hostnames tests (#11307) · 663fbd34
      Madelyn Olson authored
      This PR introduces a couple of changes to improve cluster test stability:
      1. Increase the cluster node timeout to 3 seconds, which is similar to the
         normal cluster tests, but introduce a new mechanism to increase the ping
         period so that the tests are still fast. This new config is a debug config.
      2. Set `cluster-replica-no-failover yes` on a wider array of tests which are
         sensitive to failovers. This was occurring on the ARM CI.
      663fbd34
    • Binbin's avatar
      Fix redis-cli cluster add-node race in cli.tcl (#11349) · a549b78c
      Binbin authored
      There is a race condition in the test:
      ```
      *** [err]: redis-cli --cluster add-node with cluster-port in tests/unit/cluster/cli.tcl
      Expected '5' to be equal to '4' {assert_equal 5 [CI 0 cluster_known_nodes]} proc ::test)
      ```
      
      When using cli to add node, there can potentially be a race condition
      in which all nodes presenting cluster state o.k even though the added
      node did not yet meet all cluster nodes.
      
      This comment and the fix were taken from #11221. Also apply it in several
      other similar places.
      a549b78c
  15. 28 Sep, 2022 2 commits
    • sundb's avatar
      Fix the missing server.dirty increment and redundant signalModifiedKey in... · f106beeb
      sundb authored
      Fix the missing server.dirty increment and redundant signalModifiedKey in serveClientBlockedOnList (#11326)
      
      Mainly fix two minor bug
      1. When handle BL*POP/BLMOVE commands with blocked clients, we should increment server.dirty.
      2.  `listPopRangeAndReplyWithKey()` in `serveClientBlockedOnList()` should not repeat calling
         `signalModifiedKey()` which has been called when an element was pushed on the list.
         (was skipped in all bpop commands, other than blmpop) 
      
      Other optimization
      add `signal` param for `listElementsRemoved` to skip `signalModifiedKey()` to unify all pop operation.
      
      Unifying all pop operations also prepares us for #11303, so that we can avoid having to deal with the
      conversion from quicklist to listpack() in various places when the list shrinks.
      f106beeb
    • guybe7's avatar
      RM_CreateCommand should not set CMD_KEY_VARIABLE_FLAGS automatically (#11320) · 3330ea18
      guybe7 authored
      The original idea behind auto-setting the default (first,last,step) spec was to use
      the most "open" flags when the user didn't provide any key-spec flags information.
      
      While the above idea is a good approach, it really makes no sense to set
      CMD_KEY_VARIABLE_FLAGS if the user didn't provide the getkeys-api flag:
      in this case there's not way to retrieve these variable flags, so what's the point?
      
      Internally in redis there was code to ignore this already, so this fix doesn't change
      redis's behavior, it only affects the output of COMMAND command.
      3330ea18
  16. 26 Sep, 2022 1 commit
    • Ozan Tezcan's avatar
      Ignore RM_Call deny-oom flag if maxmemory is zero (#11319) · 18920813
      Ozan Tezcan authored
      If a command gets an OOM response and then if we set maxmemory to zero
      to disable the limit, server.pre_command_oom_state never gets updated
      and it stays true. As RM_Call() calls with "respect deny-oom" flag checks
      server.pre_command_oom_state, all calls will fail with OOM.
      
      Added server.maxmemory check in RM_Call() to process deny-oom flag
      only if maxmemory is configured.
      18920813
  17. 22 Sep, 2022 3 commits
    • Shaya Potter's avatar
      Add RM_SetContextUser to support acl validation in RM_Call (and scripts) (#10966) · 6e993a5d
      Shaya Potter authored
      Adds a number of user management/ACL validaiton/command execution functions to improve a
      Redis module's ability to enforce ACLs correctly and easily.
      
      * RM_SetContextUser - sets a RedisModuleUser on the context, which RM_Call will use to both
        validate ACLs (if requested and set) as well as assign to the client so that scripts executed via
        RM_Call will have proper ACL validation.
      * RM_SetModuleUserACLString - Enables one to pass an entire ACL string, not just a single OP
        and have it applied to the user
      * RM_GetModuleUserACLString - returns a stringified version of the user's ACL (same format as dump
        and list).  Contains an optimization to cache the stringified version until the underlying ACL is modified.
      * Slightly re-purpose the "C" flag to RM_Call from just being about ACL check before calling the
        command, to actually running the command with the right user, so that it also affects commands
        inside E...
      6e993a5d
    • Oran Agra's avatar
      Fix heap overflow vulnerability in XAUTOCLAIM (CVE-2022-35951) (#11301) · 6d215601
      Oran Agra authored
      Executing an XAUTOCLAIM command on a stream key in a specific state, with a
      specially crafted COUNT argument may cause an integer overflow, a subsequent
      heap overflow, and potentially lead to remote code execution.
      The problem affects Redis versions 7.0.0 or newer.
      6d215601
    • Binbin's avatar
      ACL default newly created user set USER_FLAG_SANITIZE_PAYLOAD flag (#11279) · bb6513cb
      Binbin authored
      Starting from 6.2, after ACL SETUSER user reset, the user
      will carry the sanitize-payload flag. It was added in #7807,
      and then ACL SETUSER reset is inconsistent with default
      newly created user which missing sanitize-payload flag.
      
      Same as `off` and `on` these two bits are mutually exclusive,
      the default created user needs to have sanitize-payload flag.
      Adds USER_FLAG_SANITIZE_PAYLOAD flag to ACLCreateUser.
      
      Note that the bug don't have any real implications,
      since the code in rdb.c (rdbLoadObject) checks for
      `USER_FLAG_SANITIZE_PAYLOAD_SKIP`, so the fact that
      `USER_FLAG_SANITIZE_PAYLOAD` is missing doesn't really matters.
      
      Added tests to make sure it won't be broken in the future,
      and updated the comment in ACLSetUser and redis.conf
      bb6513cb
  18. 21 Sep, 2022 1 commit
  19. 19 Sep, 2022 1 commit
    • sundb's avatar
      Fix crash due to delete entry from compress quicklistNode and wrongly split quicklistNode (#11242) · 13d25dd9
      sundb authored
      This PR mainly deals with 2 crashes introduced in #9357,
      and fix the QUICKLIST-PACKED-THRESHOLD mess in external test mode.
      
      1. Fix crash due to deleting an entry from a compress quicklistNode
         When inserting a large element, we need to create a new quicklistNode first,
         and then delete its previous element, if the node where the deleted element is
         located is compressed, it will cause a crash.
         Now add `dont_compress` to quicklistNode, if we want to use a quicklistNode
         after some operation, we can use this flag like following:
      
          ```c
          node->dont_compress = 1; /* Prevent to be compressed */
          some_operation(node); /* This operation might try to compress this node */
          some_other_operation(node); /* We can use this node without decompress it */
          node->dont_compress = 0; /* Re-able compression */
          quicklistCompressNode(node);
          ```
      
         Perhaps in the future, we could just disable the current entry from being
         compressed during the iterator loop, but that would require more work.
      
      2. Fix crash due to wrongly split quicklist
         before #9357, the offset param of _quicklistSplitNode() will not negative.
         For now, when offset is negative, the split extent will be wrong.
         following example:
          ```c
          int orig_start = after ? offset + 1 : 0;
          int orig_extent = after ? -1 : offset;
          int new_start = after ? 0 : offset;
          int new_extent = after ? offset + 1 : -1;
          # offset: -2, after: 1, node->count: 2
          # current wrong range: [-1,-1] [0,-1]
          # correct range: [1,-1] [0, 1]
          ```
      
         Because only `_quicklistInsert()` splits the quicklistNode and only
         `quicklistInsertAfter()`, `quicklistInsertBefore()` call _quicklistInsert(), 
         so `quicklistReplaceEntry()` and `listTypeInsert()` might occur this crash.
         But the iterator of `listTypeInsert()` is alway from head to tail(iter->offset is
         always positive), so it is not affected.
         The final conclusion is this crash only occur when we insert a large element
         with negative index into a list, that affects `LSET` command and `RM_ListSet`
         module api.
           
      3. In external test mode, we need to restore quicklist packed threshold after
         when the end of test.
      4. Show `node->count` in quicklistRepr().
      5. Add new tcl proc `config_get_set` to support restoring config in tests.
      13d25dd9
  20. 06 Sep, 2022 1 commit
    • ranshid's avatar
      fix test Migrate the last slot away from a node using redis-cli (#11221) · c0ce97fa
      ranshid authored
      When using cli to add node, there can potentially be a race condition in
      which all nodes presenting cluster state o.k even though the added node
      did not yet meet all cluster nodes.
      this adds another utility function to wait until all cluster nodes see the same cluster size
      c0ce97fa
  21. 05 Sep, 2022 1 commit
    • Shaya Potter's avatar
      Add a dry run flag to RM_Call execution (#11158) · 87e7973c
      Shaya Potter authored
      Add a new "D" flag to RM_Call which runs whatever verification the user requests,
      but returns before the actual execution of the command.
      
      It automatically enables returning error messages as CallReply objects to distinguish
      success (NULL) from failure (CallReply returned).
      87e7973c
  22. 29 Aug, 2022 1 commit
  23. 28 Aug, 2022 3 commits
  24. 24 Aug, 2022 3 commits
    • Oran Agra's avatar
      Fix assertion when a key is lazy expired during cluster key migration (#11176) · c789fb0a
      Oran Agra authored
      Redis 7.0 has #9890 which added an assertion when the propagation queue
      was not flushed and we got to beforeSleep.
      But it turns out that when processCommands calls getNodeByQuery and
      decides to reject the command, it can lead to a key that was lazy
      expired and is deleted without later flushing the propagation queue.
      
      This change prevents lazy expiry from deleting the key at this stage
      (not as part of a command being processed in `call`)
      c789fb0a
    • Binbin's avatar
      Bump codespell from 2.1.0 to 2.2.1 in /.codespell (#11184) · 78259826
      Binbin authored
      add a few terms to the white list, and fix a few newly detected typos
      78259826
    • Meir Shpilraien (Spielrein)'s avatar
      Reverts most of the changes of #10969 (#11178) · c1bd61a4
      Meir Shpilraien (Spielrein) authored
      The PR reverts the changes made on #10969.
      The reason for revert was trigger because of occasional test failure
      that started after the PR was merged.
      
      The issue is that if there is a lazy expire during the command invocation,
      the `del` command is added to the replication stream after the command
      placeholder. So the logical order on the primary is:
      
      * Delete the key (lazy expiration)
      * Command invocation
      
      But the replication stream gets it the other way around:
      
      * Command invocation (because the command is written into the placeholder)
      * Delete the key (lazy expiration)
      
      So if the command write to the key that was just lazy expired we will get
      inconsistency between primary and replica.
      
      One solution we considered is to add another lazy expire replication stream
      and write all the lazy expire there. Then when replicating, we will replicate the
      lazy expire replication stream first. This will solve this specific test failure but
      we realize that the issues does not ends here and the more we dig the more
      problems we find.One of the example we thought about (that can actually
      crashes Redis) is as follow:
      
      * User perform SINTERSTORE
      * When Redis tries to fetch the second input key it triggers lazy expire
      * The lazy expire trigger a module logic that deletes the first input key
      * Now Redis hold the robj of the first input key that was actually freed
      
      We believe we took the wrong approach and we will come up with another
      PR that solve the problem differently, for now we revert the changes so we
      will not have the tests failure.
      
      Notice that not the entire code was revert, some parts of the PR are changes
      that we would like to keep. The changes that **was** reverted are:
      
      * Saving a placeholder for replication at the beginning of the command (`call` function)
      * Order of the replication stream on active expire and eviction (we will decide how
        to handle it correctly on follow up PR)
      * `Spop` changes are no longer needed (because we reverted the placeholder code)
      
      Changes that **was not** reverted:
      
      * On expire/eviction, wrap the `del` and the notification effect in a multi exec.
      * `PropagateNow` function can still accept a special dbid, -1, indicating not to replicate select.
      * Keep optimisation for reusing the `alsoPropagate` array instead of allocating it each time.
      
      Tests:
      
      * All tests was kept and only few tests was modify to work correctly with the changes
      * Test was added to verify that the revert fixes the issues.
      c1bd61a4
  25. 23 Aug, 2022 1 commit
    • Oran Agra's avatar
      Build TLS as a loadable module · 4faddf18
      Oran Agra authored
      
      
      * Support BUILD_TLS=module to be loaded as a module via config file or
        command line. e.g. redis-server --loadmodule redis-tls.so
      * Updates to redismodule.h to allow it to be used side by side with
        server.h by defining REDISMODULE_CORE_MODULE
      * Changes to server.h, redismodule.h and module.c to avoid repeated
        type declarations (gcc 4.8 doesn't like these)
      * Add a mechanism for non-ABI neutral modules (ones who include
        server.h) to refuse loading if they detect not being built together with
        redis (release.c)
      * Fix wrong signature of RedisModuleDefragFunc, this could break
        compilation of a module, but not the ABI
      * Move initialization of listeners in server.c to be after loading
        the modules
      * Config TLS after initialization of listeners
      * Init cluster after initialization of listeners
      * Add TLS module to CI
      * Fix a test suite race conditions:
        Now that the listeners are initialized later, it's not sufficient to
        wait for the PID message in the log, we need to wait for the "Server
        Initialized" message.
      * Fix issues with moduleconfigs test as a result from start_server
        waiting for "Server Initialized"
      * Fix issues with modules/infra test as a result of an additional module
        present
      
      Notes about Sentinel:
      Sentinel can't really rely on the tls module, since it uses hiredis to
      initiate connections and depends on OpenSSL (won't be able to use any
      other connection modules for that), so it was decided that when TLS is
      built as a module, sentinel does not support TLS at all.
      This means that it keeps using redis_tls_ctx and redis_tls_client_ctx directly.
      
      Example code of config in redis-tls.so(may be use in the future):
      RedisModuleString *tls_cfg = NULL;
      
      void tlsInfo(RedisModuleInfoCtx *ctx, int for_crash_report) {
          UNUSED(for_crash_report);
          RedisModule_InfoAddSection(ctx, "");
          RedisModule_InfoAddFieldLongLong(ctx, "var", 42);
      }
      
      int tlsCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
      {
          if (argc != 2) return RedisModule_WrongArity(ctx);
          return RedisModule_ReplyWithString(ctx, argv[1]);
      }
      
      RedisModuleString *getStringConfigCommand(const char *name, void *privdata) {
          REDISMODULE_NOT_USED(name);
          REDISMODULE_NOT_USED(privdata);
          return tls_cfg;
      }
      
      int setStringConfigCommand(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) {
          REDISMODULE_NOT_USED(name);
          REDISMODULE_NOT_USED(err);
          REDISMODULE_NOT_USED(privdata);
          if (tls_cfg) RedisModule_FreeString(NULL, tls_cfg);
          RedisModule_RetainString(NULL, new);
          tls_cfg = new;
          return REDISMODULE_OK;
      }
      
      int RedisModule_OnLoad(void *ctx, RedisModuleString **argv, int argc)
      {
          ....
          if (RedisModule_CreateCommand(ctx,"tls",tlsCommand,"",0,0,0) == REDISMODULE_ERR)
              return REDISMODULE_ERR;
      
          if (RedisModule_RegisterStringConfig(ctx, "cfg", "", REDISMODULE_CONFIG_DEFAULT, getStringConfigCommand, setStringConfigCommand, NULL, NULL) == REDISMODULE_ERR)
              return REDISMODULE_ERR;
      
          if (RedisModule_LoadConfigs(ctx) == REDISMODULE_ERR) {
              if (tls_cfg) {
                  RedisModule_FreeString(ctx, tls_cfg);
                  tls_cfg = NULL;
              }
              return REDISMODULE_ERR;
          }
          ...
      }
      Co-authored-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      4faddf18
  26. 21 Aug, 2022 1 commit