1. 09 Oct, 2022 5 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
    • Rahul Vishwakarma's avatar
    • yancz2000's avatar
      Fix redis-benchmark hang when it fails to connect to redis (#11366) · fe0550a4
      yancz2000 authored
      Forgot to start redis-server when testing performance.
      When opening the benchmark for testing, it will always be stuck,
      and the process cpu will reach 100%.
      fe0550a4
    • 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
  2. 08 Oct, 2022 1 commit
  3. 07 Oct, 2022 2 commits
    • Oran Agra's avatar
      fix arm build warning due to new compiler optimizations (#11362) · 34e70c13
      Oran Agra authored
      Build fails with warnings in ARM CI after adding more aggressive optimizations (#11350)
      probably a result of more aggressive inlining
      
      ```
      ziplist.c: In function ‘pop.constprop’:
      ziplist.c:1770:13: error: ‘vlong’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
                   printf("%lld", vlong);
                   ^~~~~~~~~~~~~~~~~~~~~
      ```
      
      ```
      listpack.c: In function ‘lpInsert.constprop’:
      listpack.c:406:9: error: argument 2 null where non-null expected [-Werror=nonnull]
               memcpy(buf+1,s,len);
               ^~~~~~~~~~~~~~~~~~~
      ```
      34e70c13
    • aradz44's avatar
      Added authentication failure and access denied metrics (#11288) · 8e194153
      aradz44 authored
      Added authentication failure and access denied metrics
      8e194153
  4. 06 Oct, 2022 2 commits
  5. 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
  6. 02 Oct, 2022 4 commits
    • David CARLIER's avatar
      ff808090
    • Maria Markova's avatar
      Change compiler optimizations to -O3 -flto (#11207) · 3469c650
      Maria Markova authored
      
      
      Optimization update from -O2 to -O3 -flto gives up to 5% performance gain
      in 'redis-benchmarks-spec-client-runner' tests geomean where GCC 9.4.0 is used for build
      
      * Fix for false-positive warning in bitops.c
         Warning appeared with O3, on CentOS during inlininig procedure
      * Fixed unitialized streamID within streamTrim() (#1)
      Co-authored-by: default avatarfilipe oliveira <filipecosta.90@gmail.com>
      3469c650
    • Binbin's avatar
      code, typo and comment cleanups (#11280) · 3c02d1ac
      Binbin authored
      - fix `the the` typo
      - `LPOPRPUSH` does not exist, should be `RPOPLPUSH`
      - `CLUSTER GETKEYINSLOT` 's time complexity should be O(N)
      - `there bytes` should be `three bytes`, this closes #11266
      - `slave` word to `replica` in log, modified the front and missed the back
      - remove useless aofReadDiffFromParent in server.h
      - `trackingHandlePendingKeyInvalidations` method adds a void parameter
      3c02d1ac
    • Huang Zhw's avatar
      fix some commands json file (#11201) · 2804eefc
      Huang Zhw authored
      - BITOP: turn argument `operation` from string to oneof
      - CLIENT KILL: turn argument `skipme` from string to oneof
      - COMMAND GETKEYS / GETKEYSANDFLAGS: change arguments to optional, and change arity to -3 (fixes regression in redis 7.0)
      - CLIENT PAUSE: this command was added in v3.0.0
      2804eefc
  7. 30 Sep, 2022 1 commit
  8. 29 Sep, 2022 3 commits
    • Huang Zhw's avatar
      Add redis-cli hints to ACL DRYRUN, COMMAND GETKEYS, COMMAND GETKEYSANDFLAGS (#11232) · f8e2279e
      Huang Zhw authored
      Better redis-cli hints for commands that take other commands as arguments.
      
      ```
      command getkeysandflags hello [protover [AUTH username password]]
      acl dryrun user hello [protover [AUTH username password]]
      ```
      f8e2279e
    • Wen Hui's avatar
      Update sentinel-config.json to consistent with Config Get and Set operation (#11334) · e21c0599
      Wen Hui authored
      The sentinel CONFIG GET command doesn't support multiple arguments, but the json file did.
      remove that.
      e21c0599
    • Meir Shpilraien (Spielrein)'s avatar
      Avoid crash on crash report when a bad function pointer was called (#11298) · 0bf90d94
      Meir Shpilraien (Spielrein) authored
      If Redis crashes due to calling an invalid function pointer,
      the `backtrace` function will try to dereference this invalid pointer
      which will cause a crash inside the crash report and will kill
      the processes without having all the crash report information.
      
      Example:
      
      ```
      === REDIS BUG REPORT START: Cut & paste starting from here ===
      198672:M 19 Sep 2022 18:06:12.936 # Redis 255.255.255 crashed by signal: 11, si_code: 1
      198672:M 19 Sep 2022 18:06:12.936 # Accessing address: 0x1
      198672:M 19 Sep 2022 18:06:12.936 # Crashed running the instruction at: 0x1
      // here the processes is crashing
      ```
      
      This PR tries to fix this crash be:
      1. Identify the issue when it happened.
      2. Replace the invalid pointer with a pointer to some dummy function
         so that `backtrace` will not crash.
      
      I identification is done by comparing `eip` to `info->si_addr`, if they
      are the same we know that the crash happened on the same address it tries to
      accesses and we can conclude that it tries to call and invalid function pointer.
      
      To replace the invalid pointer we introduce a new function, `setMcontextEip`,
      which is very similar to `getMcontextEip` and it knows to set the Eip for the
      different supported OS's. After printing the trace we retrieve the old `Eip` value.
      0bf90d94
  9. 28 Sep, 2022 4 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
    • Phoeniwx's avatar
      fix: redis-cli --memkeys-samples add check lastarg (#11269) · c0725abf
      Phoeniwx authored
      doing redis-cli --memkeys-samples without any additional arguments
      would have lead to a crash of the cli.
      c0725abf
    • guybe7's avatar
      Remove redundant arity checks in XINFO (#11331) · bd40d315
      guybe7 authored
      The arity in the JSON files of the subcommands reneder this
      code unreachable
      bd40d315
  10. 27 Sep, 2022 1 commit
    • Steffen Moser's avatar
      Fixing compilation by removing flock() when compiling on Solaris (#11327) · 6aab4cb7
      Steffen Moser authored
      SunOS/Solaris and its relatives don't support the flock() function.
      While "redis" has been excluding setting up the lock using flock() on the cluster
      configuration file when compiling under Solaris, it was still using flock() in the
      unlock call while shutting down. 
      
      This pull request eliminates the flock() call also in the unlocking stage
      for Oracle Solaris and its relatives.
      
      Fix compilation regression from #10912
      6aab4cb7
  11. 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
  12. 22 Sep, 2022 5 commits
    • Binbin's avatar
      Fix CLUSTER SHARDS showing empty hostname (#11297) · 1de675b3
      Binbin authored
      * Fix CLUSTER SHARDS showing empty hostname
      
      In #10290, we changed clusterNode hostname from `char*`
      to `sds`, and the old `node->hostname` was changed to
      `sdslen(node->hostname)!=0`.
      
      But in `addNodeDetailsToShardReply` it is missing.
      It results in the return of an empty string hostname
      in CLUSTER SHARDS command if it unavailable.
      
      Like this (note that we listed it as optional in the doc):
      ```
       9) "hostname"
      10) ""
      ```
      1de675b3
    • 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 EVAL scripts. see #11231
      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
    • Valentino Geron's avatar
      Replica that asks for rdb only should be closed right after the rdb part (#11296) · e53bf652
      Valentino Geron authored
      The bug is that the the server keeps on sending newlines to the client.
      As a result, the receiver might not find the EOF marker since it searches
      for it only on the end of each payload it reads from the socket.
      The but only affects `CLIENT_REPL_RDBONLY`.
      This affects `redis-cli --rdb` (depending on timing)
      
      The fixed consist of two steps:
      1. The `CLIENT_REPL_RDBONLY` should be closed ASAP (we cannot
         always call to `freeClient` so we use `freeClientAsync`)
      2. Add new replication state `SLAVE_STATE_RDB_TRANSMITTED`
      e53bf652
    • 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
  13. 21 Sep, 2022 1 commit
  14. 19 Sep, 2022 2 commits
    • Binbin's avatar
      Fix Invalid node address specified in redis-cli --cluster create/add-node (#11151) · c2b0c13d
      Binbin authored
      This bug was introduced in #10344 (7.0.3), and it breaks the
      redis-cli --cluster create usage in #10436 (7.0 RC3).
      
      At the same time, the cluster-port support introduced in #10344
      cannot use the DNS lookup brought by #10436.
      c2b0c13d
    • 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
  15. 18 Sep, 2022 1 commit
    • zhaozhao.zz's avatar
      fix infinite sleep in performEvictions when have lazyfree jobs (#11237) · 464aa041
      zhaozhao.zz authored
      This bug is introduced in #7653. (Redis 6.2.0)
      
      When `server.maxmemory_eviction_tenacity` is 100, `eviction_time_limit_us` is
      `ULONG_MAX`, and if we cannot find the best key to delete (e.g. maxmemory-policy
      is `volatile-lru` and all keys with ttl have been evicted), in `cant_free` redis will sleep
      forever if some items are being freed in the lazyfree thread.
      464aa041
  16. 15 Sep, 2022 1 commit
  17. 14 Sep, 2022 1 commit
  18. 13 Sep, 2022 1 commit
  19. 11 Sep, 2022 1 commit
    • Eduardo Semprebon's avatar
      Improve redis.conf documentation on repl-diskless-load (#11213) · 36abc0fa
      Eduardo Semprebon authored
      Just noticed that there are some inaccurate, or at least confusing information about `repl-diskless-load` in `redis.conf`
      It shouldn't scare away users willing to spend the extra memory.
      
      `may mean that we have to flush the contents of the current database before the full rdb was received.`: this is likely related to the time when there was an option `always`, where content on replica was flushed before loading from master.
      36abc0fa
  20. 08 Sep, 2022 1 commit
    • Wen Hui's avatar
      Update group and consumer description in json file for Unifying Stream command format (#11190) · 5389fa62
      Wen Hui authored
      For the stream data type, some commands, such as **XGROUP CREATE, XGROUP DESTROY, XGROUP CREATECONSUMER, 
      XGROUP DELCONSUMER and XINFO CONSUMERS** use groupname and consumername in the command description;
      
      However, for the commands **XREADGROUP GROUP, XPENDING, XACK , XCLAIM  and XAUTOCLAIM**  use term "group and consumer", clients could be confused.
      
      This PR goal is to unify all the commands to groupname and consumername.
      5389fa62