1. 20 Mar, 2023 1 commit
    • polaris-alioth's avatar
      passwords printed in the crash log (#11930) · 56eef6fb
      polaris-alioth authored
      When the server crashes during the AUTH command, or another command with
      an AUTH argument, the password was recorded in the log.
      
      Now, when the `auth` keyword is detected (could be in HELLO or MIGRATE, etc),
      the loop exits before printing any additional arguments.
      56eef6fb
  2. 12 Mar, 2023 1 commit
    • Binbin's avatar
      Fix the bug that CLIENT REPLY OFF|SKIP cannot receive push notifications (#11875) · 416842e6
      Binbin authored
      This bug seems to be there forever, CLIENT REPLY OFF|SKIP will
      mark the client with CLIENT_REPLY_OFF or CLIENT_REPLY_SKIP flags.
      With these flags, prepareClientToWrite called by addReply* will
      return C_ERR directly. So the client can't receive the Pub/Sub
      messages and any other push notifications, e.g client side tracking.
      
      In this PR, we adding a CLIENT_PUSHING flag, disables the reply
      silencing flags. When adding push replies, set the flag, after the reply,
      clear the flag. Then add the flag check in prepareClientToWrite.
      
      Fixes #11874
      
      Note, the SUBSCRIBE command response is a bit awkward,
      see https://github.com/redis/redis-doc/pull/2327
      
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      416842e6
  3. 19 Feb, 2023 1 commit
  4. 16 Feb, 2023 1 commit
    • Oran Agra's avatar
      Cleanup around script_caller, fix tracking of scripts and ACL logging for RM_Call (#11770) · 233abbbe
      Oran Agra authored
      * Make it clear that current_client is the root client that was called by
        external connection
      * add executing_client which is the client that runs the current command
        (can be a module or a script)
      * Remove script_caller that was used for commands that have CLIENT_SCRIPT
        to get the client that called the script. in most cases, that's the current_client,
        and in others (when being called from a module), it could be an intermediate
        client when we actually want the original one used by the external connection.
      
      bugfixes:
      * RM_Call with C flag should log ACL errors with the requested user rather than
        the one used by the original client, this also solves a crash when RM_Call is used
        with C flag from a detached thread safe context.
      * addACLLogEntry would have logged info about the script_caller, but in case the
        script was issued by a module command we actually want the current_client. the
        exception is when RM_Call is called from a timer event, in which case we don't
        have a current_client.
      
      behavior changes:
      * client side tracking for scripts now tracks the keys that are read by the script
        instead of the keys that are declared by the caller for EVAL
      
      other changes:
      * Log both current_client and executing_client in the crash log.
      * remove prepareLuaClient and resetLuaClient, being dead code that was forgotten.
      * remove scriptTimeSnapshot and snapshot_time and instead add cmd_time_snapshot
        that serves all commands and is reset only when execution nesting starts.
      * remove code to propagate CLIENT_FORCE_REPL from the executed command
        to the script caller since scripts aren't propagated anyway these days and anyway
        this flag wouldn't have had an effect since CLIENT_PREVENT_PROP is added by scriptResetRun.
      * fix a module GIL violation issue in afterSleep that was introduced in #10300 (unreleased)
      233abbbe
  5. 12 Feb, 2023 1 commit
    • Tian's avatar
      Reclaim page cache of RDB file (#11248) · 7dae142a
      Tian authored
      # Background
      The RDB file is usually generated and used once and seldom used again, but the content would reside in page cache until OS evicts it. A potential problem is that once the free memory exhausts, the OS have to reclaim some memory from page cache or swap anonymous page out, which may result in a jitters to the Redis service.
      
      Supposing an exact scenario, a high-capacity machine hosts many redis instances, and we're upgrading the Redis together. The page cache in host machine increases as RDBs are generated. Once the free memory drop into low watermark(which is more likely to happen in older Linux kernel like 3.10, before [watermark_scale_factor](https://lore.kernel.org/lkml/1455813719-2395-1-git-send-email-hannes@cmpxchg.org/) is introduced, the `low watermark` is linear to `min watermark`, and there'is not too much buffer space for `kswapd` to be wake up to reclaim memory), a `direct reclaim` happens, which means the process would stall to wait for memory allocation.
      
      # What the PR does
      The PR introduces a capability to reclaim the cache when the RDB is operated. Generally there're two cases, read and write the RDB. For read it's a little messy to address the incremental reclaim, so the reclaim is done in one go in background after the load is finished to avoid blocking the work thread. For write, incremental reclaim amortizes the work of reclaim so no need to put it into background, and the peak watermark of cache can be reduced in this way.
      
      Two cases are addresses specially, replication and restart, for both of which the cache is leveraged to speed up the processing, so the reclaim is postponed to a right time. To do this, a flag is added to`rdbSave` and `rdbLoad` to control whether the cache need to be kept, with the default value false.
      
      # Something deserve noting
      1. Though `posix_fadvise` is the POSIX standard, but only few platform support it, e.g. Linux, FreeBSD 10.0.
      2. In Linux `posix_fadvise` only take effect on writeback-ed pages, so a `sync`(or `fsync`, `fdatasync`) is needed to flush the dirty page before `posix_fadvise` if we reclaim write cache.
      
      # About test
      A unit test is added to verify the effect of `posix_fadvise`.
      In integration test overall cache increase is checked, as well as the cache backed by RDB as a specific TCL test is executed in isolated Github action job.
      7dae142a
  6. 09 Feb, 2023 1 commit
    • Binbin's avatar
      When DEBUG LOADAOF fails, return an error instead of exiting (#11790) · 66bed3f2
      Binbin authored
      Return an error when loadAppendOnlyFiles fails instead of
      exiting. DEBUF LOADAOF command is only meant to be used by
      the test suite, and only by tests that generated an AOF file
      first. So this change is ok (considering that the caller is
      likely to catch this error and die).
      
      This actually revert part of the code in #9012, and now
      DEBUG LOADAOF behaves the same as DEBUG RELOAD (returns an
      error when the load fails).
      
      Plus remove a `after 2000` in a test, which can save times (looks like copy paste error).
      66bed3f2
  7. 02 Feb, 2023 1 commit
    • Harkrishn Patro's avatar
      Propagate message to a node only if the cluster link is healthy. (#11752) · fd397568
      Harkrishn Patro authored
      Currently while a sharded pubsub message publish tries to propagate the message across the cluster, a NULL check is missing for clusterLink. clusterLink could be NULL if the link is causing memory beyond the set threshold cluster-link-sendbuf-limit and server terminates the link.
      
      This change introduces two things:
      
      Avoids the engine crashes on the publishing node if a message is tried to be sent to a node and the link is NULL.
      Adds a debugging tool CLUSTERLINK KILL to terminate the clusterLink between two nodes.
      fd397568
  8. 11 Jan, 2023 1 commit
    • Viktor Söderqvist's avatar
      Make dictEntry opaque · c84248b5
      Viktor Söderqvist authored
      Use functions for all accesses to dictEntry (except in dict.c). Dict abuses
      e.g. in defrag.c have been replaced by support functions provided by dict.
      c84248b5
  9. 02 Jan, 2023 1 commit
  10. 07 Dec, 2022 1 commit
    • Harkrishn Patro's avatar
      Optimize client memory usage tracking operation while client eviction is disabled (#11348) · c0267b3f
      Harkrishn Patro authored
      
      
      ## Issue
      During the client input/output buffer processing, the memory usage is
      incrementally updated to keep track of clients going beyond a certain
      threshold `maxmemory-clients` to be evicted. However, this additional
      tracking activity leads to unnecessary CPU cycles wasted when no
      client-eviction is required. It is applicable in two cases.
      
      * `maxmemory-clients` is set to `0` which equates to no client eviction
        (applicable to all clients)
      * `CLIENT NO-EVICT` flag is set to `ON` which equates to a particular
        client not applicable for eviction.  
      
      ## Solution
      * Disable client memory usage tracking during the read/write flow when
        `maxmemory-clients` is set to `0` or `client no-evict` is `on`.
        The memory usage is tracked only during the `clientCron` i.e. it gets
        periodically updated.
      * Cleanup the clients from the memory usage bucket when client eviction
        is disabled.
      * When the maxmemory-clients config is enabled or disabled at runtime,
        we immediately update the memory usage buckets for all clients (tested
        scanning 80000 took some 20ms)
      
      Benchmark shown that this can improve performance by about 5% in
      certain situations.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      c0267b3f
  11. 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
  12. 02 Oct, 2022 1 commit
  13. 29 Sep, 2022 1 commit
    • 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
  14. 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
  15. 06 Sep, 2022 1 commit
  16. 18 Aug, 2022 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Fix replication inconsistency on modules that uses key space notifications (#10969) · 508a1388
      Meir Shpilraien (Spielrein) authored
      Fix replication inconsistency on modules that uses key space notifications.
      
      ### The Problem
      
      In general, key space notifications are invoked after the command logic was
      executed (this is not always the case, we will discuss later about specific
      command that do not follow this rules). For example, the `set x 1` will trigger
      a `set` notification that will be invoked after the `set` logic was performed, so
      if the notification logic will try to fetch `x`, it will see the new data that was written.
      Consider the scenario on which the notification logic performs some write
      commands. for example, the notification logic increase some counter,
      `incr x{counter}`, indicating how many times `x` was changed.
      The logical order by which the logic was executed is has follow:
      
      ```
      set x 1
      incr x{counter}
      ```
      
      The issue is that the `set x 1` command is added to the replication buffer
      at the end of the command invocation (specifically after the key space
      notification logic was invoked and performed the `incr` command).
      The replication/aof sees the commands in the wrong order:
      
      ```
      incr x{counter}
      set x 1
      ```
      
      In this specific example the order is less important.
      But if, for example, the notification would have deleted `x` then we would
      end up with primary-replica inconsistency.
      
      ### The Solution
      
      Put the command that cause the notification in its rightful place. In the
      above example, the `set x 1` command logic was executed before the
      notification logic, so it should be added to the replication buffer before
      the commands that is invoked by the notification logic. To achieve this,
      without a major code refactoring, we save a placeholder in the replication
      buffer, when finishing invoking the command logic we check if the command
      need to be replicated, and if it does, we use the placeholder to add it to the
      replication buffer instead of appending it to the end.
      
      To be efficient and not allocating memory on each command to save the
      placeholder, the replication buffer array was modified to reuse memory
      (instead of allocating it each time we want to replicate commands).
      Also, to avoid saving a placeholder when not needed, we do it only for
      WRITE or MAY_REPLICATE commands.
      
      #### Additional Fixes
      
      * Expire and Eviction notifications:
        * Expire/Eviction logical order was to first perform the Expire/Eviction
          and then the notification logic. The replication buffer got this in the
          other way around (first notification effect and then the `del` command).
          The PR fixes this issue.
        * The notification effect and the `del` command was not wrap with
          `multi-exec` (if needed). The PR also fix this issue.
      * SPOP command:
        * On spop, the `spop` notification was fired before the command logic
          was executed. The change in this PR would have cause the replication
          order to be change (first `spop` command and then notification `logic`)
          although the logical order is first the notification logic and then the
          `spop` logic. The right fix would have been to move the notification to
          be fired after the command was executed (like all the other commands),
          but this can be considered a breaking change. To overcome this, the PR
          keeps the current behavior and changes the `spop` code to keep the right
          logical order when pushing commands to the replication buffer. Another PR
          will follow to fix the SPOP properly and match it to the other command (we
          split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if
          we chose to).
      
      #### Unhanded Known Limitations
      
      * key miss event:
        * On key miss event, if a module performed some write command on the
          event (using `RM_Call`), the `dirty` counter would increase and the read
          command that cause the key miss event would be replicated to the replication
          and aof. This problem can also happened on a write command that open
          some keys but eventually decides not to perform any action. We decided
          not to handle this problem on this PR because the solution is complex
          and will cause additional risks in case we will want to cherry-pick this PR.
          We should decide if we want to handle it in future PR's. For now, modules
          writers is advice not to perform any write commands on key miss event.
      
      #### Testing
      
      * We already have tests to cover cases where a notification is invoking write
        commands that are also added to the replication buffer, the tests was modified
        to verify that the replica gets the command in the correct logical order.
      * Test was added to verify that `spop` behavior was kept unchanged.
      * Test was added to verify key miss event behave as expected.
      * Test was added to verify the changes do not break lazy expiration.
      
      #### Additional Changes
      
      * `propagateNow` function can accept a special dbid, -1, indicating not
        to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands`
        function. The side effect of this change is that now the `select` command
        will appear inside the `multi/exec` block on the replication stream (instead of
        outside of the `multi/exec` block). Tests was modified to match this new behavior.
      508a1388
  17. 04 Aug, 2022 1 commit
    • Binbin's avatar
      errno cleanup around rdbLoad (#11042) · 4505eb18
      Binbin authored
      This is an addition to #11039, which cleans up rdbLoad* related errno. Remove the
      errno print from the outer message (may be invalid since errno may have been overwritten).
      
      Our aim should be the code that detects the error and knows which system call
      triggered it, is the one to print errno, and not the code way up above (in some cases
      a result of a logical error and not a system one).
      
      Remove the code to update errno in rdbLoadRioWithLoadingCtx, signature check
      and the rdb version check, in these cases, we do print the error message.
      The caller dose not have the specific logic for handling EINVAL.
      
      Small fix around rdb-preamble AOF: A truncated RDB is considered a failure,
      not handled the same as a truncated AOF file.
      4505eb18
  18. 26 Jul, 2022 1 commit
    • Binbin's avatar
      Change the return value of rdbLoad function to enums (#11039) · 00097bf4
      Binbin authored
      The reason we do this is because in #11036, we added error
      log message when failing to open RDB file for reading.
      In loadDdataFromDisk we call rdbLoad and also check errno,
      now the logging corrupts errno (reported in alpine daily).
      
      It is not safe to rely on errno as we do today, so we change
      the return value of rdbLoad function to enums, like we have
      when loading an AOF.
      00097bf4
  19. 06 Jun, 2022 1 commit
  20. 26 Apr, 2022 1 commit
    • Madelyn Olson's avatar
      Set replicas to panic on disk errors, and optionally panic on replication errors (#10504) · 6fa8e4f7
      Madelyn Olson authored
      * Till now, replicas that were unable to persist, would still execute the commands
        they got from the master, now they'll panic by default, and we add a new
        `replica-ignore-disk-errors` config to change that.
      * Till now, when a command failed on a replica or AOF-loading, it only logged a
        warning and a stat, we add a new `propagation-error-behavior` config to allow
        panicking in that state (may become the default one day)
      
      Note that commands that fail on the replica can either indicate a bug that could
      cause data inconsistency between the replica and the master, or they could be
      in some cases (specifically in previous versions), a result of a command (e.g. EVAL)
      that failed on the master, but still had to be propagated to fail on the replica as well.
      6fa8e4f7
  21. 28 Mar, 2022 1 commit
  22. 22 Mar, 2022 1 commit
  23. 01 Mar, 2022 1 commit
    • ranshid's avatar
      Introduce debug command to disable reply buffer resizing (#10360) · 9b15dd28
      ranshid authored
      In order to resolve some flaky tests which hard rely on examine memory footprint.
      we introduce the following fixes:
      
      # Fix in client-eviction test - by @yoav-steinberg 
      Sometime the libc allocator can use different size client struct allocations.
      this may cause unexpected memory calculations to fail the test.
      
      # Introduce new DEBUG command for disabling reply buffer resizing
      In order to eliminate reply buffer resizing during specific tests.
      we introduced the ability to disable (and enable) the resizing cron job
      
      Co-authored-by: yoav-steinberg yoav@redislabs.com
      9b15dd28
  24. 22 Feb, 2022 1 commit
    • ranshid's avatar
      introduce dynamic client reply buffer size - save memory on idle clients (#9822) · 47c51d0c
      ranshid authored
      
      
      Current implementation simple idle client which serves no traffic still
      use ~17Kb of memory. this is mainly due to a fixed size reply buffer
      currently set to 16kb.
      
      We have encountered some cases in which the server operates in a low memory environments.
      In such cases a user who wishes to create large connection pools to support potential burst period,
      will exhaust a large amount of memory  to maintain connected Idle clients.
      Some users may choose to "sacrifice" performance in order to save memory.
      
      This commit introduce a dynamic mechanism to shrink and expend the client reply buffer based on
      periodic observed peak.
      the algorithm works as follows:
      1. each time a client reply buffer has been fully written, the last recorded peak is updated: 
      new peak = MAX( last peak, current written size)
      2. during clients cron we check for each client if the last observed peak was:
           a. matching the current buffer size - in which case we expend (resize) the buffer size by 100%
           b. less than half the buffer size - in which case we shrink the buffer size by 50%
      3. In any case we will **not** resize the buffer in case:
          a. the current buffer peak is less then the current buffer usable size and higher than 1/2 the
            current buffer usable size
          b. the value of (current buffer usable size/2) is less than 1Kib
          c. the value of  (current buffer usable size*2) is larger than 16Kib
      4. the peak value is reset to the current buffer position once every **5** seconds. we maintain a new
         field in the client structure (buf_peak_last_reset_time) which is used to keep track of how long it
         passed since the last buffer peak reset.
      
      ### **Interface changes:**
      **CIENT LIST** - now contains 2 new extra fields:
      rbs= < the current size in bytes of the client reply buffer >
      rbp=< the current value in bytes of the last observed buffer peak position >
      
      **INFO STATS** - now contains 2 new statistics:
      reply_buffer_shrinks = < total number of buffer shrinks performed >
      reply_buffer_expends = < total number of buffer expends performed >
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarYoav Steinberg <yoav@redislabs.com>
      47c51d0c
  25. 16 Feb, 2022 1 commit
  26. 10 Feb, 2022 1 commit
  27. 08 Feb, 2022 1 commit
    • Wen Hui's avatar
      Make INFO command variadic (#6891) · 2e1bc942
      Wen Hui authored
      
      
      This is an enhancement for INFO command, previously INFO only support one argument
      for different info section , if user want to get more categories information, either perform
      INFO all / default or calling INFO for multiple times.
      
      **Description of the feature**
      
      The goal of adding this feature is to let the user retrieve multiple categories via the INFO
      command, and still avoid emitting the same section twice.
      
      A use case for this is like Redis Sentinel, which periodically calling INFO command to refresh
      info from monitored Master/Slaves, only Server and Replication part categories are used for
      parsing information. If the INFO command can return just enough categories that client side
      needs, it can save a lot of time for client side parsing it as well as network bandwidth.
      
      **Implementation**
      To share code between redis, sentinel, and other users of INFO (DEBUG and modules),
      we have a new `genInfoSectionDict` function that returns a dict and some boolean flags
      (e.g. `all`) to the caller (built from user input).
      Sentinel is later purging unwanted sections from that, and then it is forwarded to the info `genRedisInfoString`.
      
      **Usage Examples**
      INFO Server Replication   
      INFO CPU Memory
      INFO default commandstats
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      2e1bc942
  28. 06 Feb, 2022 1 commit
  29. 04 Feb, 2022 1 commit
    • Binbin's avatar
      Fix SENTINEL SET config rewrite test (#10232) · d7fcb3c5
      Binbin authored
      Change the sentinel config file to a directory in SENTINEL SET test.
      So it will now fail on the `rename` in `rewriteConfigOverwriteFile`.
      
      The test used to set the sentinel config file permissions to `000` to
      simulate failure. But it fails on centos7 / freebsd / alpine. (introduced in #10151)
      
      Other changes:
      1. More error messages after the config rewrite failure.
      2. Modify arg name `force_all` in `rewriteConfig` to `force_write`. (was rename in #9304)
      3. Fix a typo in debug quicklist-packed-threshold, then -> than. (#9357)
      d7fcb3c5
  30. 18 Jan, 2022 1 commit
    • Wang Yuan's avatar
      Use const char pointer in redismodule.h as far as possible (#10064) · d697daa7
      Wang Yuan authored
      When I used C++ to develop a redis module. i  used `string.data()` as the second parameter `ele`
      of  `RedisModule_DigestAddStringBuffer`, but there is a warning, since we never change the `ele`,
      i think we should use `const char` for it.
      
      This PR adds const to just a handful of module APIs that required it, all not very widely used.
      The implication is a breaking change in terms of compilation error that's easy to resolve, and no ABI impact.
      The affected APIs are around Digest, Info injection, and Cluster bus messages.
      d697daa7
  31. 03 Jan, 2022 2 commits
    • chenyang8094's avatar
      Implement Multi Part AOF mechanism to avoid AOFRW overheads. (#9788) · 87789fae
      chenyang8094 authored
      
      
      Implement Multi-Part AOF mechanism to avoid overheads during AOFRW.
      Introducing a folder with multiple AOF files tracked by a manifest file.
      
      The main issues with the the original AOFRW mechanism are:
      * buffering of commands that are processed during rewrite (consuming a lot of RAM)
      * freezes of the main process when the AOFRW completes to drain the remaining part of the buffer and fsync it.
      * double disk IO for the data that arrives during AOFRW (had to be written to both the old and new AOF files)
      
      The main modifications of this PR:
      1. Remove the AOF rewrite buffer and related code.
      2. Divide the AOF into multiple files, they are classified as two types, one is the the `BASE` type,
        it represents the full amount of data (Maybe AOF or RDB format) after each AOFRW, there is only
        one `BASE` file at most. The second is `INCR` type, may have more than one. They represent the
        incremental commands since the last AOFRW.
      3. Use a AOF manifest file to record and manage these AOF files mentioned above.
      4. The original configuration of `appendfilename` will be the base part of the new file name, for example:
        `appendonly.aof.1.base.rdb` and `appendonly.aof.2.incr.aof`
      5. Add manifest-related TCL tests, and modified some existing tests that depend on the `appendfilename`
      6. Remove the `aof_rewrite_buffer_length` field in info.
      7. Add `aof-disable-auto-gc` configuration. By default we're automatically deleting HISTORY type AOFs.
        It also gives users the opportunity to preserve the history AOFs. just for testing use now.
      8. Add AOFRW limiting measure. When the AOFRW failures reaches the threshold (3 times now),
        we will delay the execution of the next AOFRW by 1 minute. If the next AOFRW also fails, it will be
        delayed by 2 minutes. The next is 4, 8, 16, the maximum delay is 60 minutes (1 hour). During the limit
        period, we can still use the 'bgrewriteaof' command to execute AOFRW immediately.
      9. Support upgrade (load) data from old version redis.
      10. Add `appenddirname` configuration, as the directory name of the append only files. All AOF files and
        manifest file will be placed in this directory.
      11. Only the last AOF file (BASE or INCR) can be truncated. Otherwise redis will exit even if
        `aof-load-truncated` is enabled.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      87789fae
    • Madelyn Olson's avatar
      Implement clusterbus message extensions and cluster hostname support (#9530) · 5460c100
      Madelyn Olson authored
      Implement the ability for cluster nodes to advertise their location with extension messages.
      5460c100
  32. 02 Jan, 2022 1 commit
    • yoav-steinberg's avatar
      Generate RDB with Functions only via redis-cli --functions-rdb (#9968) · 1bf6d6f1
      yoav-steinberg authored
      
      
      This is needed in order to ease the deployment of functions for ephemeral cases, where user
      needs to spin up a server with functions pre-loaded.
      
      #### Details:
      
      * Added `--functions-rdb` option to _redis-cli_.
      * Functions only rdb via `REPLCONF rdb-filter-only functions`. This is a placeholder for a space
        separated inclusion filter for the RDB. In the future can be `REPLCONF rdb-filter-only
        "functions db:3 key-patten:user*"` and a complementing `rdb-filter-exclude` `REPLCONF`
        can also be added.
      * Handle "slave requirements" specification to RDB saving code so we can use the same RDB
        when different slaves express the same requirements (like functions-only) and not share the
        RDB when their requirements differ. This is currently just a flags `int`, but can be extended to
        a more complex structure with various filter fields.
      * make sure to support filters only in diskless replication mode (not to override the persistence file),
        we do that by forcing diskless (even if disabled by config)
      
      other changes:
      * some refactoring in rdb.c (extract portion of a big function to a sub-function)
      * rdb_key_save_delay used in AOFRW too
      * sendChildInfo takes the number of updated keys (incremental, rather than absolute)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      1bf6d6f1
  33. 21 Dec, 2021 2 commits
    • Meir Shpilraien (Spielrein)'s avatar
      Change FUNCTION CREATE, DELETE and FLUSH to be WRITE commands instead of MAY_REPLICATE. (#9953) · 3bcf1084
      Meir Shpilraien (Spielrein) authored
      The issue with MAY_REPLICATE is that all automatic mechanisms to handle
      write commands will not work. This require have a special treatment for:
      * Not allow those commands to be executed on RO replica.
      * Allow those commands to be executed on RO replica from primary connection.
      * Allow those commands to be executed on the RO replica from AOF.
      
      By setting those commands as WRITE commands we are getting all those properties from Redis.
      Test was added to verify that those properties work as expected.
      
      In addition, rearrange when and where functions are flushed. Before this PR functions were
      flushed manually on `rdbLoadRio` and cleaned manually on failure. This contradicts the
      assumptions that functions are data and need to be created/deleted alongside with the
      data. A side effect of this, for example, `debug reload noflush` did not flush the data but
      did flush the functions, `debug loadaof` flush the data but not the functions.
      This PR move functions deletion into `emptyDb`. `emptyDb` (renamed to `emptyData`) will
      now accept an additional flag, `NOFUNCTIONS` which specifically indicate that we do not
      want to flush the functions (on all other cases, functions will be flushed). Used the new flag
      on FLUSHALL and FLUSHDB only! Tests were added to `debug reload` and `debug loadaof`
      to verify that functions behave the same as the data.
      
      Notice that because now functions will be deleted along side with the data we can not allow
      `CLUSTER RESET` to be called from within a function (it will cause the function to be released
      while running), this PR adds `NO_SCRIPT` flag to `CLUSTER RESET`  so it will not be possible
      to be called from within a function. The other cluster commands are allowed from within a
      function (there are use-cases that uses `GETKEYSINSLOT` to iterate over all the keys on a
      given slot). Tests was added to verify `CLUSTER RESET` is denied from within a script.
      
      Another small change on this PR is that `RDBFLAGS_ALLOW_DUP` is also applicable on functions.
      When loading functions, if this flag is set, we will replace old functions with new ones on collisions. 
      3bcf1084
    • zhugezy's avatar
      Remove EVAL script verbatim replication, propagation, and deterministic execution logic (#9812) · 1b0968df
      zhugezy authored
      
      
      # Background
      
      The main goal of this PR is to remove relevant logics on Lua script verbatim replication,
      only keeping effects replication logic, which has been set as default since Redis 5.0.
      As a result, Lua in Redis 7.0 would be acting the same as Redis 6.0 with default
      configuration from users' point of view.
      
      There are lots of reasons to remove verbatim replication.
      Antirez has listed some of the benefits in Issue #5292:
      
      >1. No longer need to explain to users side effects into scripts.
          They can do whatever they want.
      >2. No need for a cache about scripts that we sent or not to the slaves.
      >3. No need to sort the output of certain commands inside scripts
          (SMEMBERS and others): this both simplifies and gains speed.
      >4. No need to store scripts inside the RDB file in order to startup correctly.
      >5. No problems about evicting keys during the script execution.
      
      When looking back at Redis 5.0, antirez and core team decided to set the config
      `lua-replicate-commands yes` by default instead of removing verbatim replication
      directly, in case some bad situations happened. 3 years later now before Redis 7.0,
      it's time to remove it formally.
      
      # Changes
      
      - configuration for lua-replicate-commands removed
        - created config file stub for backward compatibility
      - Replication script cache removed
        - this is useless under script effects replication
        - relevant statistics also removed
      - script persistence in RDB files is also removed
      - Propagation of SCRIPT LOAD and SCRIPT FLUSH to replica / AOF removed
      - Deterministic execution logic in scripts removed (i.e. don't run write commands
        after random ones, and sorting output of commands with random order)
        - the flags indicating which commands have non-deterministic results are kept as hints to clients.
      - `redis.replicate_commands()` & `redis.set_repl()` changed
        - now `redis.replicate_commands()` does nothing and return an 1
        - ...and then `redis.set_repl()` can be issued before `redis.replicate_commands()` now
      - Relevant TCL cases adjusted
      - DEBUG lua-always-replicate-commands removed
      
      # Other changes
      - Fix a recent bug comparing CLIENT_ID_AOF to original_client->flags instead of id. (introduced in #9780)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      1b0968df
  34. 01 Dec, 2021 1 commit
    • meir@redislabs.com's avatar
      Redis Functions - Move Lua related variable into luaCtx struct · e0cd580a
      meir@redislabs.com authored
      The following variable was renamed:
      1. lua_caller 			-> script_caller
      2. lua_time_limit 		-> script_time_limit
      3. lua_timedout 		-> script_timedout
      4. lua_oom 			-> script_oom
      5. lua_disable_deny_script 	-> script_disable_deny_script
      6. in_eval			-> in_script
      
      The following variables was moved to lctx under eval.c
      1.  lua
      2.  lua_client
      3.  lua_cur_script
      4.  lua_scripts
      5.  lua_scripts_mem
      6.  lua_replicate_commands
      7.  lua_write_dirty
      8.  lua_random_dirty
      9.  lua_multi_emitted
      10. lua_repl
      11. lua_kill
      12. lua_time_start
      13. lua_time_snapshot
      
      This commit is in a low risk of introducing any issues and it
      is just moving varibales around and not changing any logic.
      e0cd580a
  35. 30 Nov, 2021 1 commit
  36. 24 Nov, 2021 1 commit
    • sundb's avatar
      Replace ziplist with listpack in quicklist (#9740) · 45129059
      sundb authored
      
      
      Part three of implementing #8702, following #8887 and #9366 .
      
      ## Description of the feature
      1. Replace the ziplist container of quicklist with listpack.
      2. Convert existing quicklist ziplists on RDB loading time. an O(n) operation.
      
      ## Interface changes
      1. New `list-max-listpack-size` config is an alias for `list-max-ziplist-size`.
      2. Replace `debug ziplist` command with `debug listpack`.
      
      ## Internal changes
      1. Add `lpMerge` to merge two listpacks . (same as `ziplistMerge`)
      2. Add `lpRepr` to print info of listpack which is used in debugCommand and `quicklistRepr`. (same as `ziplistRepr`)
      3. Replace `QUICKLIST_NODE_CONTAINER_ZIPLIST` with `QUICKLIST_NODE_CONTAINER_PACKED`(following #9357 ).
          It represent that a quicklistNode is a packed node, as opposed to a plain node.
      4. Remove `createZiplistObject` method, which is never used.
      5. Calculate listpack entry size using overhead overestimation in `quicklistAllowInsert`.
          We prefer an overestimation, which would at worse lead to a few bytes below the lowest limit of 4k.
      
      ## Improvements
      1. Calling `lpShrinkToFit` after converting Ziplist to listpack, which was missed at #9366.
      2. Optimize `quicklistAppendPlainNode` to avoid memcpy data.
      
      ## Bugfix
      1. Fix crash in `quicklistRepr` when ziplist is compressed, introduced from #9366.
      
      ## Test
      1. Add unittest for `lpMerge`.
      2. Modify the old quicklist ziplist corrupt dump test.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      45129059
  37. 21 Nov, 2021 1 commit
    • Oran Agra's avatar
      Fix false positive leak reported by GCC ASAN (#9816) · 183b90a6
      Oran Agra authored
      Leak found by the corrupt-dump-fuzzer when using GCC ASAN, which seems
      to falsely report leaks on pointers kept only on the stack when calling exit.
      Instead we now use _exit on panic / assert to skip these leak checks.
      
      Additionally, check for sanitizer warnings in the corrupt-dump-fuzzer between iterations,
      so that when something is found we know which test to relate it too (and it prints reproduction command list)
      183b90a6
  38. 11 Nov, 2021 1 commit
    • Ozan Tezcan's avatar
      Add sanitizer support and clean up sanitizer findings (#9601) · b91d8b28
      Ozan Tezcan authored
      - Added sanitizer support. `address`, `undefined` and `thread` sanitizers are available.  
      - To build Redis with desired sanitizer : `make SANITIZER=undefined`
      - There were some sanitizer findings, cleaned up codebase
      - Added tests with address and undefined behavior sanitizers to daily CI.
      - Added tests with address sanitizer to the per-PR CI (smoke out mem leaks sooner).
      
      Basically, there are three types of issues : 
      
      **1- Unaligned load/store** : Most probably, this issue may cause a crash on a platform that
      does not support unaligned access. Redis does unaligned access only on supported platforms.
      
      **2- Signed integer overflow.** Although, signed overflow issue can be problematic time to time
      and change how compiler generates code, current findings mostly about signed shift or simple
      addition overflow. For most platforms Redis can be compiled for, this wouldn't cause any issue
      as far as I can tell (checked generated code on godbolt.org).
      
       **3 -Minor leak** (redis-cli), **use-after-free**(just before calling exit());
      
      UB means nothing guaranteed and risky to reason about program behavior but I don't think any
      of the fixes here worth backporting. As sanitizers are now part of the CI, preventing new issues
      will be the real benefit. 
      b91d8b28