mirror of
https://github.com/redis/redis.git
synced 2026-10-28 08:25:04 +08:00
unstable
13382
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Vitah Lin
|
33c2a2a145 |
Fix page cache reclaim test when /tmp is tmpfs (#16067)
### Issue `test_reclaimFilePageCache` fails when `/tmp` is mounted as tmpfs, as on Ubuntu 26.04. `POSIX_FADV_DONTNEED` does not evict tmpfs file pages, so the cache reclaim assertions fail. The daily `test-ubuntu-reclaim-cache` job has the same problem with RDB files under `/tmp`. ### Change Use `/var/tmp`, which normally uses a disk filesystem, for the unit test file and the daily job's master and replica directories. Update the related pidfile and RDB paths. Keep the existing cache reclaim checks. |
||
|
debing.sunandClaude Opus 5.5
|
79d3af9350 |
Fix Daily workflow not triggering for PRs to two-digit release branches (#16077)
The Daily workflow's `pull_request` branch filter is `'[0-9].[0-9]'`, which only matches a single digit on each side of the dot. As a result, PRs targeting release branches such as `8.10` do not trigger the Daily workflow (e.g. #16076 only ran CI, while the 8.8/8.6/8.4/8.2/7.4/7.2 release PRs opened at the same time all ran Daily). Change the filter to `'[0-9]+.[0-9]+'` so it matches release branches with any number of digits. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
hristostaykov-del
|
7716edb651 |
Verify the TLS server identity by default in redis-cli, redis-benchmark and Sentinel (#16033)
Followup #15492 ## Verify the TLS server identity by default in redis-cli, redis-benchmark and Sentinel - Breaking change for TLS users of these tools - see "Impact on existing deployments" ## What this PR contains 1. **Verify the TLS server identity by default** in the three tools (the change). 2. **Reject IP addresses in `tls-expected-peer-name`.** The server option is defined as DNS names only; an IP address configured there matched nothing and rejected every peer, so it is now refused when the configuration is loaded or set. 3. **Share the TLS peer identity primitive.** `tls.c`, `cli_common.c` and `sentinel.c` each had a copy of the OpenSSL calls that bind an expected identity; they are consolidated into `tlshelpers.h`. No behavior change. 4. **Sentinel: retry the Pub/Sub link when TLS setup fails.** Pre-existing: a failed TLS setup left the Pub/Sub link dead until restart. 5. **Add a positive control to the cluster-bus impersonation test**, so its log-absence assertion cannot pass vacuously. ## Background A TLS client checks two things about a server certificate: that a trusted CA issued it (the "chain"), and that it was issued for the server being contacted (the "identity": the host name or IP in the certificate's Subject Alternative Name). `redis-cli`, `redis-benchmark` and Sentinel's connections to the instances it monitors checked only the chain. So anyone holding any certificate from the same CA — under a shared or public CA — could impersonate a Redis server to them and receive the `AUTH` credentials they send. Sentinel makes these connections on its own, continuously, for every master, replica and sentinel it monitors. ## What changes The three tools now also verify the identity, as a browser or `curl` would. The identity they expect is, in order of preference: 1. **An explicitly configured name**: `--tls-expected-peer-name` for the tools (DNS names, or a single IP address), `tls-expected-peer-name` for Sentinel (DNS names only, as for the rest of the server). Meant for deployments where every node's certificate carries one shared name, including cluster mode, where node addresses come from the cluster itself. 2. **The name given to `--sni`**, which is now both sent to the server and verified (before, it was only sent). 3. **The host that was dialed**: an IP is matched against the IP entries of the certificate, a DNS name against its DNS entries. A dialed DNS name is also sent as SNI by default. Redis itself serves one certificate and ignores SNI; this is for a TLS proxy or load balancer in front of it that routes by SNI or presents the certificate for that name. Two opt-outs: `--no-verify-hostname` keeps the chain check but skips the identity check; `--insecure` skips both, as before. Neither may be combined with an explicit expected name, as it could not be honored. A rejected certificate produces a message naming the identity that was expected and these remedies, instead of OpenSSL's `certificate verify failed`. ## Sentinel New option `sentinel tls-verify-hostnames yes|no` (default `yes`; changeable at runtime with `SENTINEL CONFIG SET`, applying to links made afterwards). Sentinel verifies the address it knows an instance by, and sends it as the server name when it is a DNS name. The option governs only that address check: when `tls-expected-peer-name` is configured, that name is verified instead, on every link, whatever the option says. A mismatch is logged as a WARNING event, `-tls-identity-mismatch`, that names the expected identity and the remedies; a link failing at DEBUG level only would otherwise leave a Sentinel that lost every link with nothing visible in its log. ## Impact on existing deployments This is a breaking change for TLS users of these tools, to be listed as such in the release notes. Affected are `redis-cli` and `redis-benchmark` run with `--tls`, and Sentinel with `tls-replication yes`; plaintext connections are untouched. Certificates must now carry the identity these tools dial. Most Redis deployments connect by IP — Sentinel always does unless `resolve-hostnames` is enabled — and an IP is matched only against the certificate's `IP Address` SAN entries: DNS entries are not consulted and there is no fallback to the Common Name. A certificate whose SAN lists only DNS names, or that has no SAN and relies on its Common Name, is therefore rejected when the tool connects by IP, even though it is accepted when the tool connects by a DNS name it carries. Operators must reissue certificates with the IP in the SAN, pin a shared DNS name with `tls-expected-peer-name`, or set the opt-out `--no-verify-hostname` for `redis-cli` and `redis-benchmark`, `sentinel tls-verify-hostnames no` for Sentinel. `tls-expected-peer-name` now refuses IP addresses, from the config file and from `CONFIG SET`. This also affects replication, the cluster bus and `MIGRATE`. Such a value never matched a certificate, so any deployment that had one was already failing every connection with only `certificate verify failed` to show for it; it now fails to load with a message that says why. The test certificates change accordingly: `server.crt` now carries `127.0.0.1`, `::1` and `localhost`, so `utils/gen-test-certs.sh` must be re-run; `redis.crt` keeps no identity on purpose, as the fixture for the negative tests. ## Unchanged on purpose Connections between servers (replication, cluster bus, `MIGRATE`) keep the existing behaviour: chain check by default, identity only with `tls-expected-peer-name`. Their peer addresses are learned from other nodes, so pinning a configured name fits them better than trusting the dialed address. The only change on that path is the IP rejection above. The bundled hiredis library is also untouched; these tools do not use its TLS context API. ## Also fixed As the new failure mode exposed them: - `redis-benchmark` looped forever on a fatal TLS error on a non-blocking connection (the error was reported as "try again"). - `redis-cli` printed a TLS failure twice because its silent probe connection did not honor the quiet flag. - Sentinel never retried its Pub/Sub link after a TLS setup failure: the dead context stayed in place and counted as connected, so the instance lost Hello traffic until restart. The command link already closed its context in that case; the Pub/Sub link now does the same. ## Build The check uses OpenSSL's `X509_VERIFY_PARAM` API (OpenSSL 1.0.2+). The availability test and the one function that binds an expected identity to a connection (`tlsVerifyParamAddIdentity()`) live in the new `src/tlshelpers.h`, shared by `tls.c`, `sentinel.c` and the tools, so OpenSSL's subtleties — IP versus DNS matching, explicit lengths for BoringSSL, wildcard flags — are in one place. Building against an older OpenSSL fails unless `TLS_NO_PEER_NAME_VERIFICATION` is defined, which compiles the check out; the tools then verify the chain only and say so once, and an explicit expected name is refused rather than ignored. ## Tests - `tests/unit/tls.tcl`: the default by IP and by DNS name, `--sni`, expected-name precedence, rejection of a trusted certificate lacking the address, both opt-outs, invalid and conflicting options, the check running before `AUTH` is sent, `redis-benchmark`, and the config-time rejection of IP addresses in `tls-expected-peer-name`. - `tests/sentinel/tests/17-tls-verify-hostnames.tcl`: the default, rejection and its WARNING event, the runtime opt-out and its persistence, and the `tls-expected-peer-name` override. - `tests/unit/cluster/tls-peer-impersonation.tcl`: a positive control that replays the forged FAIL through a certificate carrying the cluster SAN and sees it processed, so the negative test's log-absence assertion is meaningful. |
||
|
Vitah Lin
|
e87dda2a47 |
Remove the timing assertion from the Roaring bitmap digest test (#15990)
## Issue The fragmented Roaring bitmap digest test can fail when CI runner load changes. It compares digest time for a 1MB fragmented bitmap against a 16MB string baseline and requires the ratio to stay below 25x. Scheduling noise can skew this comparison. ## Change Remove the timing assertion and keep the digest correctness check |
||
|
Marcin PoźniakandClaude Opus 5.5
|
7028976793 |
Remove redundant per-step range assert in lpFindCbInternal() (#16039)
## Summary
`lpFindCbInternal()` is the scan behind `lpFind()` and `lpFindCb()`, so
every listpack member/field lookup goes
through it: ZSCORE, ZMSCORE, HGET, HMGET, HGETEX, SISMEMBER and so on.
After each step it validates the
new position:
```c
if (p + 8 >= lp + lp_bytes)
lpAssertValidEntry(lp, lp_bytes, p);
else
assert(p >= lp + LP_HDR_SIZE && p < lp + lp_bytes);
```
On 64-bit builds the `else` assert cannot fire:
- `p` starts at a valid entry (from `lpFirst()`/`lpNext()`/`lpSeek()`)
and only moves forward: each step adds a
positive entry size. Those sizes are not bounds-checked before the move
(the `lpSkip()` path and integer
entries), but every position within 8 bytes of the end, or past it,
takes the `p + 8 >= lp + lp_bytes` branch
and is checked by `lpAssertValidEntry()`.
- So the `else` branch only sees positions with `p + 8 < lp + lp_bytes`,
which already implies
`p < lp + lp_bytes`, and `p >= lp + LP_HDR_SIZE` holds because `p` never
moves backwards.
It runs on every step, so twice per member/score or field/value pair.
That makes it pure overhead in the
hottest loop of every listpack lookup. This PR removes it on 64-bit
builds and documents why the position is in
range.
**32-bit builds keep the lower-bound half.** There, `lpSkip()` advances
`p` by a 32-bit entry length, so a corrupt
32-bit-string length close to 2³² (an unsanitized payload) wraps `p` to
just below the listpack instead of past
its end, which the near-end check would not catch. The guard uses the
`#if UINTPTR_MAX == 0xffffffff` idiom
already used in `object.c`, `defrag.c` and `fast_float_strtod.c`:
```c
if (p + 8 >= lp + lp_bytes)
lpAssertValidEntry(lp, lp_bytes, p);
#if UINTPTR_MAX == 0xffffffff
/* On 32-bit a corrupt entry length can wrap `p` below the header. */
else
assert(p >= lp + LP_HDR_SIZE);
#endif
```
Integrity checking is unchanged:
- the full `lpAssertValidEntry()` still runs for every position within 8
bytes of the end (or past it);
- the per-match bound check (`p + entry_size < lp + lp_bytes`) before
the comparator reads a string value is
kept.
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Vitah Lin
|
ad49ff0d92 |
Fix GCC maybe-uninitialized warning in zset B+ tree (#16058)
Introduced in #15635 ## Issue The Ubuntu 26.04 CI build fails with `-Werror=maybe-uninitialized` in `zbtScoreLeafReverseForPop()`. GCC cannot prove that the loop initializes `scores[0]` before use. ## Change Assert that the leaf count is positive and within `ZBT_SCORE_LEAF_MAX` before filling the array. This makes the existing nonempty leaf requirement explicit. |
||
|
Lazizbek Ergashev
|
c039dd784c |
Fix double free of reply in redis-cli --cluster add-node (#16053)
Fixes #16051 After the function sync step, `clusterManagerCommandAddNode()` frees `reply` but doesn't clear the pointer. If resolving the first node's address for `CLUSTER MEET` then fails, the `goto cleanup` frees the same reply again. The pointer is now set to NULL after the free, so cleanup skips it. |
||
|
|
81d653f5dc |
Reduce large sorted set memory with a packed B+ tree (#15635)
Large sorted sets (beyond `zset-max-listpack-entries` / `zset-max-listpack-value`) now use a new `btree` encoding instead of skiplist + dict. The skiplist and the dict keep separate copies of the same index information. The new encoding uses one packed B+ tree plus a hash index built for this purpose, and they share information. Small sets still use listpack. There are no new config options. ### Design **Score tree.** A B+ tree ordered by (score, member) that owns all member data. Leaves are single allocations of at most 4095 bytes holding at most 96 elements, laid out as packed scores, u16 record offsets, one hash tag byte per member, then member records growing from the end. Each double is mapped to an order-preserving integer, and a leaf stores a base value plus bit-packed differences, so dense or integer-like scores take a few bits each. Members of 320 bytes or more live in their own allocation; their record keeps the pointer, length and hash, so a score change does not copy them. Inner nodes store child pointers, the max score below each child and per-child element counts. The counts make rank queries a tree descent. **Member index.** An open-addressing hash table with 8-slot buckets. A slot holds only an 8-bit hash tag and a 16-bit leaf ID; IDs widen to 32 bits before the 16-bit range runs out. A lookup goes from the tag to the leaf, then compares members inside the leaf. Each bucket has a small per-home summary of tags, which rejects most misses without probing. Resizes are incremental and move whole leaves from the old table to the new one. **ZSCAN.** The cursor is a reverse-binary counter over groups of hash buckets, as in `dictScan()`. It depends only on member hashes, so it stays valid across resizes. Like SCAN, a member may be returned more than once. ### Behavior changes - `OBJECT ENCODING` reports `btree`. Keys are no longer stored as skiplists; skiplist is only used as a temporary structure inside set-algebra commands. - ZSCAN reply order and cursor values differ from the skiplist encoding. A member can be returned more than once even when no resize happened. - `MEMORY USAGE` reports the smaller footprint. - RDB and AOF formats are unchanged. Btree sets are saved as `RDB_TYPE_ZSET_2`, so old and new versions (including mixed-version replication) exchange data without migration. ### Implementation - `src/zset_btree.c`, `src/zset_btree.h`: the new encoding. - `t_zset.c`: dispatch for every command, plus bulk paths. Listpack-to-btree conversion and ZRANGESTORE append sorted input directly into leaves. ZUNIONSTORE builds the result btree directly instead of a temporary skiplist. ZMSCORE and multi-element pops are batched. - `rdb.c`: loads large sets into the btree and validates listpack scores before conversion. - Adapted to the new encoding: `aof.c`, `db.c` (ZSCAN), `defrag.c`, `lazyfree.c`, `object.c` (free, dup, dismiss in fork child), `geo.c`, `sort.c`, `module.c` (range iteration, `RM_ScanKey`), `debug.c`. - `dict.c`: `dictFindLinkWithHash()`, so ZUNIONSTORE can reuse a member hash it already computed. --------- Co-authored-by: fcostaoliveira <filipe@redis.com> Co-authored-by: debing.sun <debing.sun@redis.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Moti Cohen <moti.cohen@redis.com> |
||
|
Lazizbek Ergashev
|
9e19096803 |
Fix ebuckets defrag assert when resuming after the remaining buckets were removed (#15950)
When an incremental ebuckets defrag resumes, `ebDefragRax()` seeks to the saved `next` key with `>=` and then asserts `raxNext()`. If every bucket at or after that key was removed between the two runs (fields expired, deleted or persisted), the seek still succeeds but leaves the iterator at the end, so the assert fires and the server crashes. Now a failed `raxNext()` there ends the scan the same way reaching the last bucket already does. Added an ebuckets unit test that defrags one bucket, removes all the later ones and resumes. Fixes #15947 |
||
|
hristostaykov-del
|
9a95a8913a |
Redis cli cmddocs hardening (#16034)
## Issue redis-cli builds its hint table from the `COMMAND DOCS` reply in two passes: `cliCountCommands()` sizes the `helpEntries` array, then `cliInitCommandHelpEntries()` fills it. It trusted the server to make the two passes agree. A malicious or compromised server can make them disagree: * **Nested subcommands**, one level deeper than Redis ever emits. The counter walks one level, while the filler recursed without limit, writing one entry per nested subcommand past the allocation. * **Odd-length maps.** Both passes, and `cliAddCommandDocArg()`, walked replies in `i += 2` steps without checking that the last pair is complete. They read `element[i + 1]` past the end of the element array and dereferenced it. The first is a heap buffer overflow (ASan: `WRITE of size 4` in `cliFillInCommandHelpEntry`). The second crashes an unfixed client even in a normal build. `cliSwitchProto()` has the same odd-length problem when it reads the server version from the `HELLO 3` reply. Reaching any of these requires pointing redis-cli at a hostile or MITM'd endpoint. ## Fix * Guard every paired access in the `COMMAND DOCS` parsing and the `HELLO 3` reply walk with `i + 1 < elements`. * Recurse into subcommands only for top-level commands, keeping allocation and fill in lockstep. * As a defensive bound, stop filling once `next` reaches `helpEntries + helpEntriesLen`. * Trim `helpEntriesLen` to what was actually filled, so no uninitialized entries reach `qsort()`. ## Tests The tests reuse the existing `tests/helpers/fake_redis_node.tcl` without modifying it, with RESP fixtures built alongside the tests in `tests/integration/redis-cli.tcl`. There is one malformed-input mode per guarded site in the `COMMAND DOCS` parsing: `nested`, `oddlen`, `oddspec`, `oddsubs`, `oddargs`. Each case asserts that redis-cli neither crashes nor trips a sanitizer, exits successfully, and reports a passing hint suite. Each malformed reply also carries a synthetic `SYNTEST` command whose distinctive argument hint is checked with `--test_hint_file`. This proves the supplied docs were parsed rather than silently replaced by the built-in fallback table. A well-formed case verifies that ordinary commands and first-level subcommands still produce the expected argument hints. A separate test sends a `HELLO 3` reply that ends in a dangling `version` key and checks that `redis-cli -3 PING` still prints `PONG`. A shared runner forces RESP2 and database 0, temporarily clears and restores `REDISCLI_AUTH`, and cleans up on failure without masking the original error. The tests are skipped under TLS, which the helper does not support. AddressSanitizer prints its report and exits without raising a signal, so the tests check the output and the exit status rather than relying on a fatal signal. Against the unfixed client, all five `COMMAND DOCS` modes fail under `SANITIZER=address`, and four of them also fail in a normal build. The `HELLO 3` test fails under `SANITIZER=address` with a heap-buffer-overflow read. |
||
|
Slavomir KaslevandSergei Georgiev
|
788eb04400 |
Keep XTRIM cursor valid after listpack flags re-encode (#16044)
## Problem Stream entries store a small flags integer in a listpack. A crafted DUMP/RESTORE payload can encode that flags element as a 64-bit integer instead of the 7-bit encoding that actually fits the value. Integrity checks still accept the listpack, so the key loads even with `sanitize-dump-payload yes`. `XTRIM` walks the node, marks entries deleted via `lpReplaceInteger()`, and then used to restore the scan cursor as `lp + (p - lp)`. Replacing a padded encoding with the minimal one shrinks the listpack (and may realloc it), so that saved offset no longer points at the next entry — the trim walk continues from the wrong place and can crash. ## Solution `streamTrim()` no longer saves `p` as an offset from the start of the listpack. It saves the distance from the element after the flags slot (`p - lpNext(lp, pcopy)`) and restores the cursor with `lpNext(lp, pcopy) + delta` after `lpReplaceInteger()`, so the walk continues at the next entry even if the flags rewrite reallocates or shrinks the listpack. Adds `corrupt-dump.tcl` coverage that RESTOREs the same non-minimal-flags payload and runs `XTRIM MAXLEN 1`. (cherry picked from commit c9c9dd76d47725cc088ebf1f361ab2291e270280) Co-authored-by: Sergei Georgiev <s_ggeorgiev@yahoo.com> |
||
|
Slavomir KaslevandMincho Paskalev
|
e4eb89782d |
Fix lua shebang/function metadata RESP injection (#16032)
Parsing lua shebang (during SCRIPT LOAD) or function metadata (during FUNCTION LOAD) may cause RESP injection into the reply stream of the client which may cause desync. Fix the parsing so it returns proper errors in error cases (cherry picked from commit c90c984474bfeb28836fa501f6dd9d4fd24cac34) Co-authored-by: Mincho Paskalev <minchopaskal@gmail.com> |
||
|
Slavomir KaslevandMincho Paskalev
|
b3c4bbaa46 |
Fix lua tostring exposing heap addresses space (#16031)
## Summary Prevent Lua’s default `tostring` from exposing process memory addresses. Object values now return strings such as `table: <opaque>` and `function: <opaque>`. ## Behavior - Covers tables, functions, threads, and userdata in EVAL and Redis Functions. - Preserves scalar conversions and custom `__tostring` behavior. - Leaves debugger address output unchanged. - Different objects of the same type now produce identical default strings. ## Testing - Added regression coverage for address suppression, scalar conversions, and custom metamethods. - Server build succeeded. - All 12 targeted scripting and debugger checks passed. (cherry picked from commit 3d18eb1b71daa112918a312fc7a4b9b36677dce0) Co-authored-by: Mincho Paskalev <minchopaskal@gmail.com> |
||
|
Shubham S Taple
|
a013fd9207 |
Fix use-after-free in cluster bus FORGOTTEN_NODE handling (#15685)
## Issue A cluster-bus `PING` packet can include a `FORGOTTEN_NODE` extension naming the node that sent the packet. Processing it can delete the active cluster link and free its receive buffer while the packet parser still holds pointers into that buffer, causing a use-after-free. ## Fix Ignore `FORGOTTEN_NODE` entries that refer to the active sender or its current link node. Also save the next extension pointer before processing each entry, so the parser doesn’t reread extension metadata after a handler may have changed it. |
||
|
LiranAbirandClaude Opus 5.5
|
fdd319f8b3 |
RED-213286: Add a BLESS test for the repl-diskless-load swapdb path (#16010)
Test-only change: one new test at the end of `tests/unit/bless.tcl` (+56 lines). It covers the `blessed_keys` swap in `swapMainDbWithTempDb()` (`src/db.c`), which runs when a replica loads a full sync with `repl-diskless-load swapdb`. No server code changes. ## Why this test Mutation check on `unstable` @ |
||
|
Oran AgraandClaude Opus 5.5
|
847ff3503a |
CLUSTER RESET on a replica: propagate the flush to the AOF and sub-replicas (#16038)
CLUSTER RESET turns a replica into an empty master: clusterReset() calls
replicationUnsetMaster() and then emptyData(). emptyData() only empties
memory; nothing is written to the AOF or to the replication stream. So:
- The AOF still holds the old dataset. When the node next restarts from
it, the old keys come back, although the node was reset to empty (and,
being a master now, it may later own slots those keys don't belong to).
- Sub-replicas keep their link across replicationUnsetMaster() (the
replid is shifted, not reset), so they keep the old dataset too.
Fix: after emptyData(), propagate a FLUSHALL (AOF + replication), as
FLUSHALL itself would. CLUSTER RESET runs as a command, so it is
propagated when the command completes (also inside MULTI/EXEC).
Tests:
- tests/unit/cluster/cluster-reset-aof.tcl: a replica with an AOF is
reset and restarted from its AOF; its dataset must stay empty. Without
the fix all its keys come back.
- tests/unit/cluster/resharding.tcl: verify that the masters hold
exactly the test's keys (sum of DBSIZE), not just that those keys hold
the right values, before and after the AOF restart.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches cluster reset and replication/AOF propagation paths; incorrect
propagation could affect dataset durability or replica state during
failover/reset scenarios.
>
> **Overview**
> Fixes **CLUSTER RESET** on a replica promoted to master: after
`emptyData()` clears memory only, the change **propagates `FLUSHALL`**
via `alsoPropagate` with **AOF and replication** so restarts from AOF
and chained sub-replicas do not keep the old dataset.
>
> **Tests:** `resharding.tcl` adds **`verify_no_extra_keys`** (total
`DBSIZE` across masters must match the logical key count) in the pre-
and post-AOF-restart consistency checks, so ghost keys from a bad AOF
replay are caught.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
LiranAbirandClaude Opus 5.5
|
194a599a50 |
RED-213286: Extend BLESS eviction tests to every maxmemory policy (#15996)
Test-only change to `tests/unit/bless.tcl`. It covers the eviction policies and paths that the BLESS suite added in #15649 didn't exercise yet. No server code changes. ## What's added | Area | Before | After | | --- | --- | --- | | Blessed key survives an eviction flood | 6 of 9 evicting policies | All 9 (adds `volatile-lfu`, `volatile-lrm`, `allkeys-lrm`) | | All-blessed bounded overshoot (tolerated within 1.25x `maxmemory`, OOM past it, `BLESS CLEAR` relieves it) | `allkeys-random` only, plus a separate `allkeys-lru` case | All 9 evicting policies; keys get a TTL under `volatile-*` | | Time-sliced eviction | Not covered | `lazyfree-lazy-eviction yes` with `maxmemory-eviction-tenacity 0` | | No overshoot when no blessed key is an eviction candidate | Not covered | `noeviction`, and `volatile-lru` with blessed keys that have no TTL: the plain `maxmemory` limit applies | Most of the additions wrap existing tests in `foreach` loops rather than copying them. Per review, the loops sit at column 0 so the wrapped tests keep their original indentation (+51 / −9). A few existing test names gain a `($policy)` or `(lazyfree-lazy-eviction ...)` suffix. ## Note on the "no overshoot" case The eviction docs say the overshoot tolerance "applies uniformly across every `maxmemory-policy` value". In the code, the tolerance only applies when eviction sampled keys and found that all of them were blessed (`blessed_blocked`). With `noeviction`, or under `volatile-*` when the blessed keys have no TTL, nothing is sampled, so writes get a plain OOM. The new test pins the current code behavior. If that's the intended behavior, the docs wording could be narrowed. ## Testing `./runtest --single unit/bless` on `unstable` @ |
||
|
Mincho Paskalev
|
2ac3f66dd5 |
Add config filtering which clients can be processed by IO threads (#15913)
# Description Add `io-threads-client-types` to select which client connections use the configured IO workers. It accepts one or more values: - `normal`: ordinary client connections. - `replica`: downstream replica connections, including compressed connections. - `primary`: upstream primary connections, including compressed connections. - `replication`: both `replica` and `primary`. - `replica-compressed`: only compressed downstream replica connections. - `primary-compressed`: only compressed upstream primary connections. - `replication-compressed`: both compressed replication types. - `all`: all client types. This is the default. Clients excluded by the selection stay on the main thread. The config is immutable and available in both compression and non-compression builds. Compression-specific selections require compression support and `repl-compression > 0`. Enabling replication compression requires a selection that permits replication clients. ## Why Compression is CPU-intensive, so offloading compressed replication connections to IO workers can be useful while keeping ordinary clients on the main thread. The config also allows selecting replication connections independently by direction. ## Usage ```sh redis-server \ --io-threads 2 \ --io-threads-client-types replication-compressed \ --repl-compression 7 ``` This starts the main thread and one IO worker. Only replication connections that negotiate compression use the worker; all other clients stay on the main thread. Multiple types can be combined: ```sh redis-server \ --io-threads 4 \ --io-threads-client-types "normal replication-compressed" \ --repl-compression 7 ``` This allows ordinary clients and compressed replication connections to use the IO workers. `io-threads` controls the thread count. With `io-threads 1`, no IO worker is started. |
||
|
Lazizbek Ergashev
|
6ea763ceb8 |
Allow HOTKEYS START DURATION 0 as documented by HOTKEYS HELP (#16026)
Fixes #16025 `HOTKEYS HELP` says `DURATION 0` keeps tracking until `HOTKEYS STOP`, and that's already what happens when DURATION is omitted, but the parser rejected 0 with `DURATION must be between 1 and 1000000`. The lower bound is now 0, so an explicit `DURATION 0` behaves like leaving it out (the auto-stop in serverCron only runs for duration > 0). |
||
|
debing.sunandClaude Sonnet 5
|
913be9cdba |
Make client unshared reply memory accounting incremental in cron (#15761)
## Problem `getClientsSharedMemoryUsage()` refreshed `reply_bytes_unshared` by scanning every pending reply buffer whenever `INFO MEMORY` or `MEMORY STATS` collected memory overhead. `CLIENT LIST` / `CLIENT INFO` performed the same potentially expensive scan for each reported client. For clients with large output buffers, these synchronous scans add latency to commands that only need memory statistics. ## Solution Keep `reply_bytes_unshared` as a cached per-client value, and track the global shared/unshared reply memory incrementally instead of scanning on demand: - `updateClientMemoryUsage()` applies the delta of each client's `reply_bytes_shared` / `reply_bytes_unshared` to `server.stat_clients_shared_memory` / `server.stat_clients_unshared_memory`, remembering its last contribution in `last_memory_shared` / `last_memory_unshared`; `freeClient()` subtracts it. `INFO MEMORY` and `MEMORY STATS` just read these counters. - The cached unshared reply bytes are refreshed from `clientsCronRunClient()` roughly once per second per client. `CLIENT LIST` / `CLIENT INFO` report the cached value without scanning. - When client eviction is enabled, keep the existing real-time refresh in `updateClientMemUsageAndBucket()`, so eviction bucket placement uses an up-to-date value whenever the unshared bytes can affect the selected bucket. ## Behavior changes - The INFO fields `mem_clients_normal_shared` / `mem_clients_normal_unshared` (released since 8.10), and `clients.normal.shared` / `clients.normal.unshared` in `MEMORY STATS`, are no longer instantaneous: - shared lags until `clientsCron()` visits the client again, about 0.1s (one cron tick at the default `hz`) with few clients, up to about 1s with many clients; - unshared also waits for the per-second rescan, so it lags by about 1s. `omem-unshared` in `CLIENT LIST` lags the same way. - Fix for `FLUSHALL ASYNC` with pending reply references: `protectClientReplyObjects()` duplicates the referenced objects and removes the clients from `clients_with_pending_ref_reply`. The base computed the global counters by iterating that list, so after `FLUSHALL ASYNC` it reported 0 shared/unshared bytes although the duplicated objects are solely owned by the clients' output buffers. With incremental counters, these bytes are now reported as unshared. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
Marcello CostagliolaandClaude Opus 5.5
|
da47cd5bf4 |
Fix MSETEX writes lost during atomic slot migration (#15937)
**Summary**
MSETEX is missing a key-extraction callback, causing some internal paths
to treat it as having no keys. This PR adds the missing callback.
Impact of the bug:
- Atomic slot migration: acknowledged MSETEX writes can be lost during
migration. MSETEX also bypasses the write-blocking check during active
trimming on replicas, which may cause the master and replica to diverge
in an ASM edge case.
- Scripts, functions and module calls: in cluster mode, MSETEX called
from Lua scripts, functions or `RedisModule_Call()` can write to keys in
different slots or to a slot owned by another node.
- Module APIs: `RedisModule_GetCommandKeys()` and
`RedisModule_GetCommandKeysWithFlags()` incorrectly return no keys for
MSETEX.
- Slot migration module API:
`RedisModule_ClusterPropagateForSlotMigration()` does not reject an
MSETEX whose keys are in different slots or outside the migrating slot
range.
- Metrics: in standalone mode with `cluster-compatibility-sample-ratio`,
a cross-slot MSETEX is not counted in `cluster_incompatible_ops`,
whether a client sends it or a script runs it.
Affects version 8.4 and later.
**Detailed explanation**
An `MSETEX` executed on the source node while one of its slots is being
migrated with `CLUSTER MIGRATION IMPORT` is never sent to the
destination. The client receives `1`; after the handoff the destination
is the only owner of the slot and does not have the write, and the
source trims its copy.
```
node B: CLUSTER MIGRATION IMPORT 1 16382 (the slot of {t} is served by node A)
node A: SET c1{t} v -> OK reaches B
MSET c2{t} v -> OK reaches B
MSETEX 1 m1{t} hello -> 1 never sent
MSETEX 1 old{t} new-value -> 1 never sent (old{t} was in the snapshot)
after the migration, on B: GET m1{t} -> (nil) GET old{t} -> old-value
```
`MONITOR` on the destination shows the `SET` and the `MSET` and no
`MSETEX`. Nothing about it is logged and the task ends as `completed`. A
master with a replica outside cluster mode is not affected: the
replication stream carries the command, it is the slot filter of the
migration that drops it.
`asmFeedMigrationClient()` gets the slot of every propagated command
from `getSlotFromCommand()` and skips the commands without keys.
`getSlotFromCommand()` relies on `getKeysFromCommand()`, which asks the
module API, then the getkeys function of the command, then the legacy
`(first, last, step)` range. `MSETEX` declares its keys with a `keynum`
key spec, which that range cannot express, and has no getkeys function,
so there it has no keys. It is the only command in this position: of the
28 commands whose key specs are not a plain index and range, the other
27 have a `get_keys_function`.
The same function backs the checks made on each command of a script in
cluster mode (`getNodeByQuery()` when it is not handed the keys found by
`preprocessCommand()`, which is how `script.c` and `RM_Call()` call it):
```
on a node that does not serve the slot of {t}:
EVAL "return redis.call('SET','k{t}','v')" 0 -> ERR Script attempted to access a non local key in a cluster node
EVAL "return redis.call('MSETEX',1,'k{t}','v')" 0 -> 1
on the node that serves {t} and {u}:
EVAL "return redis.call('MSET','a{t}','v','b{u}','v')" 0 -> ERR Command 'mset' in script attempted to access keys that don't hash to the same slot
EVAL "return redis.call('MSETEX',2,'a{t}','v','b{u}','v')" 0 -> 1
```
In the first case the key stays on the node that does not serve its
slot: `DBSIZE` counts it, `KEYS *` and `CLUSTER COUNTKEYSINSLOT` do not
show it. A client that sends `MSETEX` itself is routed correctly
(`MOVED`, `CROSSSLOT`, also inside `MULTI`): that path reads the key
specs.
The same on 8.4.7, 8.6.7, 8.8.3, 8.10.2 and `unstable` (
|
||
|
Ozan Tezcan
|
c38dcdabf4 |
Set TCP_NODELAY and keepalive on the master connection after PSYNC (#16020)
After PSYNC, the cached master client is reused on the new connection, which never goes through createClient(). So TCP_NODELAY and keepalive are not set, and Nagle can delay REPLCONF ACK by ~40ms (WAIT latency). |
||
|
Lazizbek Ergashev
|
f2a17a8dca |
Fix XADD/XTRIM MAXLEN with ACKED stopping early at a node with referenced entries (#16006)
Fixes #16004 Introduced since #14623 ### Problem `streamTrim()` walks the stream nodes from the head. A node that isn't entirely inside the trim range is the boundary node: its entries are trimmed one by one, and then the loop ends, assuming enough was deleted: ``` if (node_eligible_for_remove) continue; break; /* there was enough to delete in the current node */ ``` That assumption holds for `KEEPREF` and `DELREF`, which delete every entry they visit. It doesn't hold for `ACKED`, which skips entries that are still referenced by a consumer group. When the boundary node holds such entries, `MAXLEN n ACKED` stops with the stream still longer than `n`, even though later nodes hold acked entries that could be removed. How far the trim gets therefore depends on where the node boundaries fall (`stream-node-max-entries` / `stream-node-max-bytes`, tombstones, AOF rewrite repacking). Exact trims are propagated verbatim, and `~` trims are rewritten to exact ones. So a replica or an AOF replay with a different node layout deletes a different set of entries than the master. For example, with 6 entries where `3-0` and `4-0` are pending, `XTRIM s MAXLEN = 3 ACKED` deletes 2 entries with `stream-node-max-entries 2` but 3 entries with `stream-node-max-entries 100`. ### Fix Keep the `stop` flag of the per-entry loop outside of it, and move on to the next node whenever the loop finished a node without reaching the trim target (`!stop`), instead of deciding based on `node_eligible_for_remove`. - `MAXLEN` + `ACKED` (exact or `~`): keeps going past nodes with referenced entries, until `s->length <= maxlen` (already checked at the top of the loop) or the end of the stream. The result no longer depends on the node layout. - `MINID` (any strategy): `!stop` after a node means all of its entries are `< minid`, which is the same as `node_eligible_for_remove`. No behavior change. - `MAXLEN` + `KEEPREF` / `DELREF`: every visited entry is deleted, so the boundary node always reaches `stop`. |
||
|
Tom Gabsow
|
13056451a5 |
Update RedisTimeSeries to version 8.11.90 (#15969)
## Summary Updates the bundled RedisTimeSeries pin in `modules/modules.yaml` from **v8.10.0 to v8.11.90**, the **8.12 release candidate**. | Module | From | To | |---|---|---| | RedisTimeSeries | v8.10.0 | v8.11.90 | The upstream tag resolves to `ef84d314675a693998092db24b5adf8cc314d004`, the current `8.12` release-branch head. Its source version is `8.11.90`. RedisJSON and RedisBloom are handled separately by #15964 and #15966. ## Release notes — RedisTimeSeries 8.12 release candidate Based on the published [`v8.11.90` tag](https://github.com/RedisTimeSeries/RedisTimeSeries/tree/v8.11.90) at [`ef84d314`](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/ef84d314675a693998092db24b5adf8cc314d004). These notes describe the actual changes since **8.10.0**, excluding features already backported into that release. ### Bug fixes and robustness - **Correct replication and AOF replay for increments with labels:** Fixed `TS.INCRBY` and `TS.DECRBY` inserting an automatically generated `TIMESTAMP` after the `LABELS` clause, where it could be interpreted as an extra label. Also corrected keyword detection when keys or labels contain `TIMESTAMP` or `LABELS`, and prevented failed sample updates from being replicated. ([#2160](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2160)) - **Correct updates to large uncompressed chunks:** Fixed sample-count truncation when inserting or updating samples in uncompressed chunks containing more than 32,767 samples, including chunks loaded through `RESTORE`. ([#2172](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2172)) - **Safer compressed-chunk decoding:** Added bounds checks and read-ahead padding to prevent heap over-reads when decoding malformed compressed chunks. Validation applies to both RDB loading and inter-shard deserialization. ([#2150](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2150)) - **Stricter RDB consistency validation:** Reject serialized series with no chunks, duplicate chunk start timestamps, overflowing sample-count totals, or a total sample count that disagrees with the loaded chunks. ([#2189](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2189)) - **Safe duplicate-policy parsing:** Fixed a potential stack overflow when an excessively long duplicate-policy string is supplied. Valid policy names retain their existing case-insensitive behavior. ([#2205](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2205)) ### Cluster reliability - Improved cluster-type detection and inter-shard authentication handling for Redis Enterprise deployments using OSS cluster mode. ([#2149](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2149), [LibMR #123](https://github.com/RedisGears/LibMR/pull/123)) - Fixed an SSL-object memory leak when inter-shard TLS initialization fails. ([LibMR #124](https://github.com/RedisGears/LibMR/pull/124)) - Changed distributed execution-ID initialization to avoid reusing IDs after a shard restarts. ([#2199](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2199), [LibMR #126](https://github.com/RedisGears/LibMR/pull/126)) ### Command metadata - Added the missing `TS.QUERYLABELS` entry to `commands.json` for client-generation and documentation tools. The command itself was already available in 8.10.0. ([#2135](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2135)) - Corrected `TS.CREATERULE`’s advertised minimum argument count. ([#2136](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2136)) - Updated `TS.INCRBY` and `TS.DECRBY` command specifications to describe their supported creation options accurately. ([#2160](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2160)) ### Testing and build maintenance - Expanded replication-integrity, failover, topology-convergence, TLS-failure, and corrupted-RDB regression coverage. - Improved cluster-test startup and timeout handling, updated RLTest to 0.7.31, and removed Debian Bullseye CI jobs. - **Updated the Homebrew OpenSSL dependency to OpenSSL 4 for macOS builds.** ([#2213](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2213), [8.12 backport #2215](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2215)) - Added release-tag/version validation and improved artifact naming for nightly and manual builds. - Reduced the TLS migration-test fixture to improve CI reliability; this change does not alter production query timeouts. ([#2212](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2212)) - Set the release-candidate version to **8.11.90**. ([#2214](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2214)) [Full comparison: v8.10.0 → v8.11.90](https://github.com/RedisTimeSeries/RedisTimeSeries/compare/v8.10.0...v8.11.90) ## Validation - [x] Verified the published tag points to the intended 8.12 release commit and matches `src/version.h`. - [x] Verified the manifest parser resolves RedisTimeSeries to `v8.11.90` as a tag. - [x] Verified the diff changes only the RedisTimeSeries pin; `git diff --check` passes. - [ ] Upstream [tag build/test/package workflow](https://github.com/RedisTimeSeries/RedisTimeSeries/actions/runs/38559907930) completes (running when this PR was opened; tag-version validation passed). - [ ] Redis core CI validates the bundled module. A full local module build/test was not run for this manifest-only change. <details> <summary>Upstream commit history for audit</summary> This is the commit-history range, not a list of newly introduced features: some master commits have equivalent backports in 8.10.0. The release notes above were checked against the net source diff. - Use dedicated redis_ref field in ramp.yml for CI git ref ([#2059](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2059)) ([ccdcd46e](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/ccdcd46e47dec2a9b57fcf72bce9460cf9c3c88f)) - pass VG=1 flag to the build as well ([#2060](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2060)) ([062f3691](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/062f3691d830e6dea22956eb71a7edbe56b87d07)) - MOD-16160 - wake parked TS.BGET clients on key deletion ([#2062](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2062)) ([b1c4085d](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b1c4085dbdec055ef58bb0b59e8b3c04d68d40d2)) - ci: bump GitHub Actions to Node24-compatible versions ([#2035](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2035)) ([04cb2fce](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/04cb2fcec96f3ac5772e54f2f4cd25e06dfd22e4)) - MOD-15749 Short-form CLUSTERSET: RAMP capability + LibMR error-return ([#2065](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2065)) ([8f2a3a6b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/8f2a3a6bb4e01f48d1f04686ed70433eda8bdcd6)) - ci: speed up Event CI (lean PR gate) and shard slow nightly jobs ([#2068](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2068)) ([cfcd4e73](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/cfcd4e73ef110261b89d6b20a3412d36fb538547)) - MOD-6409 - TS.INFO memoryUsage (and MEMORY USAGE, via the mem_usage callback) ([#2067](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2067)) ([00c646b4](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/00c646b4412e1406eab1883995f1dc95e3e265b8)) - MOD-16266 - fix ci hang ([#2066](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2066)) ([a0f71da6](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a0f71da64e23ef3c3637895c97ffa479d6803ba5)) - MOD-15565 - mrange aggregation execution in inner shards ([#2074](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2074)) ([badcf8ac](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/badcf8acc4f022e8ae8641f4eeb0a8e8eb943315)) - MOD-16505 - change TS.BGET command to TS.READ ([#2081](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2081)) ([22c471ef](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/22c471efe5ff702d2080d0e1ed4f20f3d2989e02)) - MOD-14615: run valgrind test suite serially to avoid CPU oversubscription ([#2083](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2083)) ([a48abf37](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a48abf374508fca85fe56f97378683cd986f529f)) - MOD-14615: skip test_asm migration-query test on macos-15-intel ([#2084](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2084)) ([c94501f2](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/c94501f21b12e76498513018f3b09548dc05665c)) - MOD-16299 - NRANGE support multiple aggregations ([#2079](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2079)) ([7d16dbd6](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/7d16dbd663cde28d77742cd6be0d166f389b72b4)) - MOD-16514 - fix bootstrap & build for all os ([#2082](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2082)) ([049d00dc](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/049d00dcc9e8611b677d5a5589e5bd7bfdc894c7)) - MOD-16653 Prevent more heap overflow + other rdb load asserions ([#2086](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2086)) ([b54cdcef](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b54cdcef3ace43be379295bc26889038a0213eb7)) - MOD-16172 - support exclude empty in mrange commands ([#2072](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2072)) ([0d46cacd](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/0d46cacd8ee5257ceda2517790e6d7788a89e0f0)) - ci: add code coverage to CI — PR + nightly (lcov -> Codecov) ([#2070](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2070)) ([dbb7ce4f](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/dbb7ce4fd202c4163959a9fa40cd8e2d1d627690)) - fix(alpine): use distro python3 for the test venv (avoid uv musl clang standalone) ([#2089](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2089)) ([0576b6ed](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/0576b6ede8e94a70f424aca869cf513665f32758)) - MOD-16370 Introduce `TS.QUERYLABELS`: Get a list of labels and label-… ([#2090](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2090)) ([77b634dd](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/77b634ddd785a9d21b4099520acf4db1df6075e9)) - MOD-16873 `TS.INCRBY` replicate master timestamp when timestamp is lo… ([#2109](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2109)) ([ca3375a9](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/ca3375a9f00e84733f244640c278688d1005b0ed)) - MOD-16306 - use registry for pulling / pushing docker images ([#2098](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2098)) ([8c2f591b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/8c2f591b502b34c583cac3fd0019a17f309c9d33)) - MOD-16364 - update requirements deps ([#2108](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2108)) ([93ccac0e](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/93ccac0ef9e070bf035a74177d0091ddaaf1d109)) - fix ubuntu hangs ([#2115](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2115)) ([b8ff52e5](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b8ff52e542e3fdc11c06a50b4538ab7774e6a365)) - MOD-16382 Handle topology-change notifications ([#2104](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2104)) ([ce8be8aa](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/ce8be8aae4524ab535904b6b3629d24f49c1faef)) - MOD-16382 Add topology-change flow tests (consolidated) ([#2116](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2116)) ([01d59afb](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/01d59afb14680532439eac2885f80730ae14ca13)) - MOD-16265 - retry the dumpAndReload() for max 2 sec so no "Background save already in progress to give enough time to existing rdb save to finish ([#2124](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2124)) ([d74df641](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/d74df6411e530609f64c6d94ac4d5f0fc2ea486c)) - Merge pull request #2125 from RedisTimeSeries/claude/redistimeseries-pr-2116-tests-0wmm4d ([7ddf2ae8](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/7ddf2ae8f99426df30057e823faf7d83a5873665)) - MOD-17016 Disable topology events handling in ts under RE (for now) ([#2130](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2130)) ([40748fdb](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/40748fdb5a430c639f223f86b7ded308c382074c)) - MOD-17095 Add tls cluster tests leg ([#2126](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2126)) ([3fbfae15](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/3fbfae152ef889fd27045ac088220c6933df366d)) - MOD-16382 Add flow tests for topology events ([#2121](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2121)) ([fae002e8](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/fae002e8a1fae4e16234a8454569624353e3c595)) - MOD-16382 Test CLUSTERSET after topology events ([#2131](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2131)) ([a50d901a](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a50d901af7d1fd7553f38ad82a46573120423a75)) - squash ([#2128](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2128)) ([5944c928](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/5944c9283caa37935777eb26d8788a37610cb4d7)) - MOD-16399 Bump LibMR: skip rebuilds for unchanged CLUSTERSET topologies ([#2088](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2088)) ([008f5a9b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/008f5a9b692d466c0e78fc58c6be52deb2e500a0)) - MOD-17289 Suppress x86-only Valgrind false positive in MR_BuildCluster ([#2134](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2134)) ([bb16996d](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/bb16996dddc53ad76adc17450410e653013f94fb)) - MOD-17349 Add TS.QUERYLABELS to commands.json ([#2135](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2135)) ([b278d809](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b278d80937f98eec89ad61607f27cb38efedfcf1)) - MOD-17350 Fix TS.CREATERULE advertised arity (-5 -> -6) ([#2136](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2136)) ([b01a5426](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b01a5426a37d925dc509a9197f191e158664e81f)) - MOD-17281 Fix test_failover timeouts and hang ([#2138](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2138)) ([b04f067a](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b04f067aeed10075da5950379ea6d4eb259985af)) - MOD-16600 Read INFOCLUSTER status from the end, not index 17 ([#2144](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2144)) ([9e24dabd](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/9e24dabde2aa463af7ccfd2ce79f3a6f6550e849)) - MOD-17518 Use refined cluster types (allow RE+oss bdbs) ([#2149](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2149)) ([0cc05c78](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/0cc05c78f364fd61f9a6d4df674073715a309e89)) - MOD-17239 gorilla decoder guard over-read ([#2150](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2150)) ([36c13128](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/36c131288e3568ed66c6f7019b838af71ae894e5)) - MOD-17209 Port missing topology-event tests and hunt for UAFs ([#2137](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2137)) ([44d103be](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/44d103be80753bf947b14e8a1dcfc34f917b2bc7)) - smarter soloution ([#2151](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2151)) ([77270d3a](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/77270d3a7baaab019674af0dccfc44aea9851a85)) - MOD-17566 - add new stage to tag flow to verify tag version miss match ([#2152](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2152)) ([f7d32579](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/f7d32579e00a9a8d06b17eda6def1b9dfdd626b9)) - MOD-16873 Fix TS.INCRBY/TS.DECRBY replicating a bogus TIMESTAMP label with LABELS ([#2160](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2160)) ([4f26b4cb](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/4f26b4cbec3ff7edda66821342b0d456b2bc4d47)) - MOD-17281 Tolerate connection reset during demotion in test_failover ([#2162](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2162)) ([6863942c](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/6863942c381a3acdf000e4a57e951face93bc6ce)) - MOD-18192 - leak ssl in cluster err ([#2163](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2163)) ([dc0f1a00](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/dc0f1a0040f5f977c5c05ab2e58b6dc8c66954c1)) - MOD-17439 Better handling of timeouts ([#2164](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2164)) ([783eebfb](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/783eebfbca2bda15d6771b3cef3f7a9c76dcc42b)) - MOD-18488 - Fix signed truncation in uncompressed upsert after RESTORE ([#2172](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2172)) ([7c06d521](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/7c06d521e4d61b0c832e6c3d4bef35cb7c789bd7)) - Remove Bullseye from automation testing matrices ([#2181](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2181)) ([70a37c2b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/70a37c2ba4d0825ebbdea9ed88ca8b51fbd77f96)) - MOD-18589 - Reject inconsistent sample counts in RDB loads ([#2189](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2189)) ([a65a01a7](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a65a01a7b3f52dfa869e7889bc6791f3e25df237)) - MOD-18751 / MOD-18906 / MOD-18913: bring the oss-cluster flow tests back ([#2194](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2194)) ([9487ceef](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/9487ceef503381acc6517f02fb485fee074d5550)) - MOD-18948: verify every write command replicates to the replica ([#2201](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2201)) ([0f34e70c](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/0f34e70c7aa3f9d6a0ed8ed3bbb1345841c41e2e)) - build: pin Homebrew OpenSSL 3 on macOS ([#2200](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2200)) ([853c9905](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/853c9905d79cdc76d05cbbae4d4dcdbd6258e111)) - MOD-18801 Bump deps/LibMR to its latest version ([#2199](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2199)) ([fbcbd40b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/fbcbd40ba1ba367c46d7ffbda875142cb0438ae8)) - MOD-17236 - dup policy string length capping ([#2205](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2205)) ([feaf6bc2](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/feaf6bc2b3402ae0f609c987a9f63a31142af431)) - MOD-18948: check clock rows by what the replica receives, not by timing ([#2207](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2207)) ([d47c979c](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/d47c979c8d1e5faa35e43e0aef0804793ac0e910)) - MOD-18475 Check promoted nodes against current Redis topology ([6060574b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/6060574b6274c46151baafb8081457ff9213f42e)) - MOD-18443 Enable server logs for TLS initialization regression ([b9f97109](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/b9f971097697d05951c5b68d9a68375127d1ae84)) - MOD-18475 Refresh all node views throughout failover convergence ([11390b56](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/11390b5699233915ced95fc0c240c50c8a6ffce6)) - MOD-19365 Use released RLTest 0.7.31 for module tests ([28cd329c](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/28cd329c6c63d570dd02fcd21ff1e946f6b8db19)) - CI: bound test-container loopback MSS with MTU 1500 ([e3c403ad](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/e3c403ad086f85b751ddcf1e8aa5248e73622df7)) - MOD-17216: replace CI MTU workaround with 60-second query idle timeout ([a01ca70a](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a01ca70a0d3d69501141c3274ce484635558c2f4)) - MOD-17216: use requested 10-second distributed query idle limit ([386d2c9b](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/386d2c9b6a815439b173dc31e4517c9dba5f41ae)) - MOD-17216 Use response progress timeouts for distributed range queries ([00ad114f](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/00ad114fadab34248d4435cfc8db3e20d319ea17)) - MOD-17216: reduce migration query fixture instead of changing timeouts ([1f0daea0](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/1f0daea09ac657148afe98a050f88ce496b48cbc)) - MOD-17216: reuse smaller migration fixture for TLS runs ([51adf485](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/51adf48569bc485165093c37720f0d9c8b779d97)) - Prepare RedisTimeSeries 8.12 release candidate 8.11.90 ([a2448196](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/a2448196480b7bb4882b324db506d1e231b18f41)) - update openssl dep to version 4 on brew ([#2213](https://github.com/RedisTimeSeries/RedisTimeSeries/pull/2213)) ([4bb5b630](https://github.com/RedisTimeSeries/RedisTimeSeries/commit/4bb5b630ce28f999903ed1765e1036816b0a4bf3)) </details> |
||
|
redis-release-automation-app[bot]andgithub-actions[bot]
|
4d04dfabb6 |
Update RedisBloom to version 8.11.90 (#15966)
## Module Update This PR updates **RedisBloom** to version **8.11.90**. ## Changes - Updated `modules/redisbloom/Makefile` to set `MODULE_VERSION = v8.11.90` ## Release Information - **Module**: RedisBloom - **Version**: 8.11.90 - **Tag Date**: 2026-10-14 - **Makefile Path**: `modules/redisbloom/Makefile` ## Verification - [ ] Version updated correctly in Makefile - [ ] Module builds successfully - [ ] Tests pass - [ ] No breaking changes --- *This PR was automatically created by the Redis Module Release Orchestrator.* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
redis-release-automation-app[bot]andgithub-actions[bot]
|
2ee1ce462d |
Update RedisJSON to version 8.11.90 (#15964)
## Module Update This PR updates **RedisJSON** to version **8.11.90**. ## Changes - Updated `modules/redisjson/Makefile` to set `MODULE_VERSION = v8.11.90` ## Release Information - **Module**: RedisJSON - **Version**: 8.11.90 - **Tag Date**: 2026-10-14 - **Makefile Path**: `modules/redisjson/Makefile` ## Verification - [ ] Version updated correctly in Makefile - [ ] Module builds successfully - [ ] Tests pass - [ ] No breaking changes --- *This PR was automatically created by the Redis Module Release Orchestrator.* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> |
||
|
GuyAv46andClaude Opus 5.5
|
89791b3677 |
MOD-19657: Vector Sets: salt the deserialization ID table hash (#15999)
### Summary `hnsw_deserialize_index()` resolves serialized node IDs to nodes through a linear probing table during RDB load / `RESTORE`. - Hash node IDs with the salt already passed to the function, instead of a fixed unsalted hash. - Stop probing at the first empty slot. The table is insert-only, so an empty slot means the ID is not present: this is only an early exit (optimization), not a bug fix, and the result is unchanged. Valid RDB files are unaffected; no format change. ### Release notes Vector Sets: harden the RDB load / `RESTORE` path by salting the hash used to resolve serialized node IDs. Also a minor load-time optimization when resolving node links. No behavior or format change. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Mincho Paskalev
|
9d6e098a1b |
Fix Lua DoS because of long running string find (#1561) (#16017)
Lua's `string.find` method calls a C function that does the pattern matching. For patterns with humongous number of combinations like `(.)(.)(.)(.)(.)(.)(.)(.)(.)(.)` the function may run virtually indefinitely. Since lua only exposes a hook for the upstream application during the main loop of the VM, we can't possibly check for timed-out script since the script is running a C-fucntion. To fix this a counter was added inside Lua's `match` function, and the lua's hook is called whenever this counter is reached (similar to what lua does in its VM loop). This way we can catch the script is long-running and also kill it. Thanks to @skaslev for the idea. |
||
|
Tom Gabsow
|
559d143b4e |
RED-220725: Preserve BLESS across module value replacement (#15995)
Fixes RED-220725 by preserving BLESS `NO-EVICT` when
`RM_ModuleTypeSetValue` replaces a module value.
The API deletes the old key before installing the replacement, which
clears its BLESS state. The change captures the old flag, installs the
replacement, then looks up the live key after keyspace notifications and
restores the flag only if that key still contains the module value being
installed. This also covers module commands such as `TDIGEST.MERGE`,
which use `RM_ModuleTypeSetValue` for their destination.
Regression tests cover flag preservation and eviction protection,
including a notification callback that attaches metadata and reallocates
the new key. The suite is registered in `runtest-moduleapi`.
`make -j8` passed. The focused Tcl suite could not run locally because
the test harness reported all candidate ports busy; CI will validate the
registered suite.
`MIGRATE` flag transfer and the expired-key difference between `INFO`
and `BLESS SCAN` remain unchanged, consistent with the design
discussion.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes module key replacement and BLESS/eviction behavior for module
types; scoped fix with regression tests but affects memory policy
semantics.
>
> **Overview**
> **`RM_ModuleTypeSetValue`** used to drop **BLESS `NO-EVICT`** because
it deletes the key before installing the new module value. The
implementation now reads the no-evict flag on the existing key, performs
the replace, and reapplies **`blessSetNoEvict`** on the new value when
needed (e.g. module commands that update in place via
**`TDIGEST.MERGE`**).
>
> A new **`bless_datatype`** module API suite checks that **`NO-EVICT`**
survives **`datatype.set`** on a blessed key and that the key is not
evicted under **`allkeys-lru`** after the value is replaced. The suite
is wired into **`runtest-moduleapi`**.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
GuyAv46andClaude Opus 5.5
|
2138dd66df |
MOD-19660: Vector Sets: bound serialized node params count on RDB load (#15998)
### Summary `VectorSetRdbLoad()` read each node's `params_count` into a `uint32_t` and allocated `params_count * sizeof(uint64_t)` before validating it. - Load `params_count` as 64 bit. - Reject it before allocating when it exceeds `HNSW_MAX_SERIALIZED_NODE_PARAMS`, the maximum a single serialized node can encode. - `HNSW_MAX_LEVEL` moves to `hnsw.h` so the bound can be derived there. Valid RDB files are unaffected. ### Tests - New `modules/vector-sets/tests/rdb_params_count_cap.py`: a node padded up to the bound still loads; counts above the bound, above `UINT32_MAX`, and `UINT64_MAX` are rejected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Vitah Lin
|
64a436ed0f |
Wait for RDB child before DEBUG RELOAD in hash template tests (#16001)
The `large template-based key` test can fail with `ERR Background save already in progress`. Replica sync may finish before the master reaps the RDB child created for synchronization. PR #15949 added a guard that rejects `DEBUG RELOAD` while a background RDB save is active. The guard prevents reload from loading a stale snapshot. The hash template test, introduced by PR #15364, only waits for replica sync and can call `DEBUG RELOAD` before the RDB child exits. |
||
|
Filipe Oliveira (Redis)
|
c4706ba71a |
Update HdrHistogram to 0.12.1 and optimize INFO statistics (#15367)
Update the vendored HdrHistogram_c core to [0.12.1](https://github.com/HdrHistogram/HdrHistogram_c/commit/30e9ce6ef5dcdc60f9270e1ca03c0966eaaf71d0) and reduce the cost of `INFO latencystats` with its single-pass batch percentile API. Preserve configured percentile order and INFO formatting, including command statistics. The dependency is generated from release commit `30e9ce6ef5dcdc60f9270e1ca03c0966eaaf71d0` using upstream's `script/amalgamate.py --malloc-include hdr_redis_malloc.h`. The generated files have no manual edits. Only the core is vendored; Redis's allocator shim is retained. Both dependency READMEs include the source pin and regeneration instructions. HdrHistogram remains compiled with `-O3`. GCC 4.8 retains the upstream scalar scan fallback (`HDR_DISABLE_AVX2`), while other supported compilers retain runtime AVX2 dispatch where available. The iterator extension is now supplied upstream, and private headers are inlined by the generator. ## INFO changes - Initialize one percentile context per INFO section, shared across command and subcommand histograms. Prepare labels and sorted input once. - Keep up to 16 percentiles in inline stack storage; use overflow-checked heap arrays above 16 and free them once after traversal. All configuration sizes use `qsort`, with the original index breaking ties between equal percentiles. - Query every configured percentile in one histogram pass and emit results in the original order, preserving duplicates and empty configurations. - Use upstream 0.12.1's consistent p0 and empty-histogram behavior, removing the former Redis workarounds. The dependency also fixes p100 rank calculation at very large counts, unsafe rank conversion, and empty statistics. - Render latency values with integer microseconds and three fractional digits. Build commandstats lines without slowlog fields using `sdscatfmt`, retaining `usec_per_call` formatting. |
||
|
Filipe Oliveira (Redis)
|
d1a17fad78 |
ZSCAN: format skiplist scores with d2string(), consistent with ZSCORE (#15821)
### Problem For the same sorted set member, `ZSCAN` on a skiplist-encoded key returns a different score string than `ZSCORE` — and than `ZSCAN` itself on a listpack-encoded key: ``` redis> ZADD k 3.3 m (plus >128 more members, so k is skiplist-encoded) redis> ZSCORE k m "3.3" redis> ZSCAN k 0 MATCH m 1) "0" 2) 1) "m" 2) "3.2999999999999998" ``` ### Root cause #10587 (7.2) moved `d2string()` / `addReplyDouble()` to the fpconv shortest round-trip formatter, but `scanCallback()`'s `OBJ_ZSET` branch in `db.c` kept formatting the score with `ld2string(..., LD_STR_AUTO)` — i.e. the C library's `"%.17Lg"` on a `double` widened to `long double`. It is the only client-reply `ld2string()` call fed a `double`; every other client-facing score reply already uses `d2string()` (ZSCORE, ZMSCORE, the `WITHSCORES` variants, ZPOPMIN/ZPOPMAX, and listpack-encoded sorted sets, which store the `d2string()` form and echo it from ZSCAN as-is). The result is that since 7.2 a key's ZSCAN score strings change form when it converts from listpack to skiplist. ### Fix Two lines: format the skiplist ZSCAN score with `d2string()` into a `MAX_D2STRING_CHARS` buffer (128 bytes instead of the 5 KiB `MAX_LONG_DOUBLE_CHARS` stack buffer). After merging current `unstable`, including #15822, these lines are in `scanGenericCommand()` reply construction. The borrowed-node collection and queued-deletion regression test from #15822 are preserved. ### Behaviour change (release notes) This is a formatting consistency improvement, not a numeric correctness fix: the old and new strings parse to the same double. Since the reply text changes, this should be release-noted and does not warrant a bug-fix backport. ZSCAN on skiplist-encoded sorted sets now returns the same score string as ZSCORE for the same value. Relative to `"%.17Lg"` the notation can differ in either direction — `1.2345678912345679e+8` where it printed `123456789.12345679`, `-123456789012345670000000` where it printed `-1.2345678901234567e+23` — and integral scores of 18+ digits up to 2^62 print exactly (`1152921504606846976` instead of `1.152921504606847e+18`). Every form re-parses to the same `double` through ZADD. Notes for reviewers: - ZSCAN is a read-only reply; nothing here is replicated, written to AOF or persisted. AOF rewrite has already emitted the fpconv form since 7.2 (`rioWriteBulkDouble`). - No new syntax class appears: `"%.17Lg"` already produced exponent notation (`1e+100`, `9.8813129168249309e-323`); fpconv writes minimal exponent digits (`e+8` vs `e+08`), exactly as ZSCORE / ZMSCORE / `WITHSCORES` have since 7.2, so any client parsing those parses this. - Listpack entries written before 7.2 and loaded from an RDB keep their stored `"%.17g"` text (unchanged by this PR). - The precision requirement from #2175 (never lose information for tiny values) is preserved; that regression test still passes and its denormal now prints `1e-322`, matching ZSCORE. ### Out of scope `RM_ScanKey` (module scan callback) keeps its current formatting — it is a separate API surface and is left for a follow-up. |
||
|
|
5e332cabd5 |
deps/fpconv: remove every integer division from Grisu digit generation (#15787)
`generate_digits()` divides `part1` by a divisor loaded from the static `tens[]` table. Because the table is mutable, the compiler can't assume its contents and emits a real 64-bit `divq` for every fractional double formatted into a RESP reply. ## Change 1. `deps/fpconv/fpconv_dtoa.c`: `static uint64_t tens[]` → `static const uint64_t tens[]` (plus the two pointers into it). 2. `deps/fpconv/Makefile`: `OPT= -Os` → `OPT= -O3`. `fpconv_dtoa.o` goes from 5 `div` instructions to 0. Output is byte-for-byte identical. ## Why both changes are needed 1. The loop must be unrolled so each divisor is a known constant. GCC only does that at `-O3`. 2. The compiler must be able to prove the divisor values. With a mutable table, plain `-O3` is a regression (10 divides). 3. The compiler must choose to strength-reduce. At `-Os` it declines, so `const` alone changes nothing. `deps/*/Makefile` files carry their own `OPT` and `src/Makefile`'s `-O3` doesn't propagate, so `deps/fpconv` has been building at `-Os`. `div` instructions in `fpconv_dtoa.o` (gcc 11.4 and 13.3): | build | `-Os` | `-O2` | `-O3` | |---|---|---|---| | before | 5 | 1 | 10 | | this PR | 5 | 1 | **0** | clang 18/19 already reach 0 at `-O3` without `const`, so there it is documentation only. ## Results `redis-benchmarks-specification`, Xeon 6972P, server and client on separate NUMA nodes, arms interleaved, 5 reps per cell. | test | before → after (ops/sec) | Δ | |---|---|---| | `zset-1K zrange-all-elements` | 6,797 → 9,616 | **+41.5%** | | `zset-100 zrange-all-elements` | 42,181 → 48,555 | **+15.1%** | | `zrangebyscore long-scores` (integer scores, control) | 69,321 → 69,420 | +0.14% | The control takes the `double2ll()` fast path and never reaches Grisu, so the gain comes from the float-formatting path. With `io-threads 16` the numbers are similar (+46.3% / +22.2% / +0.02%). Every RESP double reply benefits: `ZSCORE`, `ZINCRBY`, `ZRANGE ... WITHSCORES`, `GEODIST`, `INCRBYFLOAT`, etc. Integer values are unaffected. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: fcostaoliveira <filipe@redis.com> |
||
|
Paulo Sousa
|
e0bb931684 |
Fix BITOP performance regression by looking up each source key only once (#15970)
## Summary #15331 made `bitopCommandGeneric()` look every source up twice (once for the lookup side effects, once to collect the values) and look the target up before reading the sources. For 10 sources that is 21 lookups per call instead of 10, and for `BITOP` over many short strings the lookups dominate the work. This collects each source in the same pass that looks it up, as before #15331: one `lookupKeyRead()` per source. Two behaviours follow from that: - **Module callbacks that write.** Writing from a keyspace notification callback is documented as dangerous and highly discouraged (`RM_SubscribeToKeyspaceEvents`, `RM_AddPostNotificationJob`). `BITOP` no longer guards against it: if a callback writes to a source that was already looked up while a later source is being looked up, the result may not reflect that write, and a master and its replica may differ. Roaring operands are borrowed from the keyspace and not refcounted, so a callback that deletes or replaces one of them would leave a dangling pointer. The set commands (`src/t_set.c`) already hold their source pointers across later lookups in the same way. - **The target is still looked up right before a native result is stored.** Without it a logically expired target would expire last, so its `DEL` would be queued after the `BITOP`, and replicas and the AOF would delete the result. The string result path does not need it. Thanks @sundb for the review that led to dropping the two-pass sequence. ## Behaviour differences from #15331 These match the behaviour before #15331: - A wrong-type source together with a logically expired target: the target is no longer expired (and no `DEL` is propagated) before the `WRONGTYPE` error. - An empty result into a logically expired target now emits `del`, not `expired`, and propagates no `DEL`. ## Tests The two `keyspace_events` tests that asserted the previous contract are removed ("BITOP reads sources after lookup callbacks change them", "BITOP expires its destination before reading sources"). `unit/bitops`, `unit/bitroar`, `unit/bitfield`, `unit/expire` and `unit/moduleapi/keyspace_events` pass. Locally the full `moduleapi` suite aborts at an unrelated `KEYMETA - AOF rewrite` test (a Tcl read error on this machine), so CI is the check for the rest. Still to add: module-free tests for the native result into an expired target (AOF and replica), a wrong-type source after decoded strings, a target that is also a source, and duplicate sources. |
||
|
neoLsHanddebing.sun
|
528f02d8fd |
Fix stream MINID ~ trim propagation with DELREF and ACKED (#15936)
Fixes #15934 An approximate `MINID ~` trim (either `XTRIM`, or `XADD` with the trimming options) is rewritten before propagation into an exact `MINID = <id>`, so that replicas and the AOF trim deterministically. `streamRewriteTrimArgument()` picked that `<id>` with `streamGetEdgeID()`, **counting tombstones**. Since #14623 an approximate trim with a non-`KEEPREF` delete strategy no longer stops at node boundaries: it deletes single entries inside the head node and leaves tombstones (`STREAM_ITEM_FLAG_DELETED`). So the very first entry is often one the master has just deleted. Propagating its ID makes replicas / AOF run `MINID = <deleted id>`, which removes nothing, so they keep entries (and with `DELREF` also the pending references) that the master dropped. `DEBUG LOADAOF` then brings the deleted entries back on the master too. ``` 127.0.0.1:6379> XADD s 1-0 f v 127.0.0.1:6379> XADD s 2-0 f v 127.0.0.1:6379> XADD s 3-0 f v 127.0.0.1:6379> XTRIM s MINID ~ 3-0 DELREF (integer) 2 127.0.0.1:6379> XRANGE s - + # master: only 3-0 survives 1) 1) "3-0" 2) 1) "f" 2) "v" 127.0.0.1:6379> DEBUG LOADAOF # replays the propagated "XTRIM s MINID = 1-0 DELREF" 127.0.0.1:6379> XRANGE s - + # ... which deletes nothing, so 1-0/2-0 are back 1) 1) "1-0" ... ``` ### The fix Picking the first *live* entry is enough for `DELREF`, but not for `ACKED`: it keeps the still-referenced entries below the threshold, and `LIMIT` (or the default approximate budget) can stop the trim early. Replicas / AOF replay the propagated command with `LIMIT` disabled, so they need to know exactly how far the master got. So instead of deriving the ID from what is left in the stream, `streamTrim()` reports the largest ID it removed through a new `max_deleted_id` out-parameter (callers that do not need it pass `NULL`). The caller propagates its successor as the exact `MINID`; for `XADD` when nothing was deleted it passes `NULL`, and the first live ID of the stream is propagated so replay is a no-op. This works for `ACKED` too: entries are examined in ID order and replicas apply the same reference checks. When a whole node is dropped its last ID is used even if it is an old tombstone, since nothing live lies between it and the last entry actually deleted. The boundary is tracked while entries are removed, so no extra pass over the stream is needed and `LIMIT` keeps bounding the work. --------- Co-authored-by: debing.sun <debing.sun@redis.com> |
||
|
Vitah Lin
|
24cb291304 |
CI upgrade FreeBSD to 15.1 (#15989)
## Issue The FreeBSD CI job still runs on FreeBSD 13.2. This version is no longer available for the current CI setup, so the job needs to move to a supported FreeBSD release. On FreeBSD 15.1, the `unit/networking` prefetch test creates 300 Tcl clients. Each client requests an ephemeral local port. The FreeBSD 15 port allocation and lookup changes can sometimes return `EADDRINUSE` during client creation before the Redis assertion runs. ## Change - Use `cross-platform-actions/action@v1.6.0` with FreeBSD 15.1. - Disable randomized ephemeral-port selection in the CI VM with `net.inet.ip.portrange.randomized=0` to avoid the observed port collision. |
||
|
debing.sun
|
4e4c65bc71 |
MOD-19794: Fix macOS module builds to link against the selected SDK (#15976)
Redis test modules, example modules, and vector sets hard-code the CommandLineTools SDK library directory on macOS. When the selected Xcode SDK is 26.5 but CommandLineTools has SDK 27, module linking can fail with `tapi error: malformed file` / `unknown architecture`. Resolve the selected SDK through `xcrun --show-sdk-path`, accepting SDKROOT as a name or a path, and stop with a clear error if resolution returns no path. Linux behavior is unchanged. Validation: all test modules, example modules, and vector sets built locally against macOS SDK 26.5; link commands checked with unset, named, and absolute SDKROOT; simulated xcrun failure stops before linking; git diff --check passed. The local linker accepts SDK 27, so the original older-linker failure was confirmed in CI logs but not reproduced locally. Affected-toolchain verification remains pending. The change done by all these previous commits: |
||
|
Tom Gabsow
|
c9e1f74604 | Use selected macOS SDK for example and vector-set modules | ||
|
Tom Gabsow
|
56afae5439 | Resolve SDK names with xcrun and reject empty SDK paths | ||
|
Tom Gabsow
|
ce9333df56 | Fix macOS test modules to link against the selected SDK | ||
|
Vitah Lin
|
f6fd2877a9 |
Fix CRC64 test build on ARM (#15974)
Fix CRC64 test with #ifdef HAVE_PCLMUL |
||
|
Vitah Lin
|
f57612b12f |
Fix MSan false positive in crash watchdog (#15972)
### Issue Daily `test-sanitizer-memory` fails after #15891. MSan reports `use-of-uninitialized-value` in `crashWatchdogArmIfIdle()`, so Redis exits before printing the expected crash report. The code reads `cur` only when `timer_gettime()` succeeds. However, older MSan runtimes lack an interceptor for this function, so they do not track the output buffer as initialized. |
||
|
Ozan TezcanandClaude Opus 4.8
|
b8dfc63b3b |
Optimize LZF decompression to speed up RDB load (#15765)
Optimize LZF decompression to speed up RDB load (#15765) Redis's LZF decoder is branch-misprediction bound (two computed-jump switch statements plus variable-length byte loops). Add lzf_decompress2(), an LZ4-style decoder used for RDB loading: it replaces those branches with a fast loop that copies in fixed 8/16/32-byte chunks, plus a byte-safe tail for the last few bytes. The original decoder stays in lzf_d.c (compiled under REDIS_TEST) as the oracle for a new randomized fuzz test (`redis-server test lzf`), which requires byte-identical output from both decoders. RDB load is ~16-49% faster depending on value size (~32% on a mixed workload); isolated decode is ~2.5-3x on x86-64 and ARM. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Moti CohenandClaude Fable 5
|
5019bc9af1 |
Accelerate CRC64 checksumming for RDB load with PCLMULQDQ (#15732)
## Summary Three changes that together accelerate RDB checksumming and loading: 1. **PCLMULQDQ crc64** (`crc64.c`/`config.h`/`crc64.h`): ~6x faster carry-less-multiplication `crc64()` for buffers >= 64 bytes, runtime-dispatched, with the existing table implementation as fallback. 2. **Batch checksum computation for RDB file loading** (`rio.c`/`rio.h`/`rdb.c`): `crc64()` was called once per *parsed field*, often just a few bytes, never reaching PCLMUL's 64-byte cutoff. `rioInitWithFileLoad()` reads the fd directly in 4KB chunks (bypassing stdio) and checksums each chunk once, so tiny fields still hit PCLMUL-sized buffers. 3. **Read large fields straight into the destination** (`rio.c`): (2)'s 4KB buffer meant even a 128KB or 20MB value got copied through it in dozens of round trips. When the buffer is empty and the request is >= one buffer's worth, `rioFileLoadRead()` now reads the aligned bulk directly into the destination in one syscall (same trick glibc's `fread()` uses for large reads), checksumming it in place - no extra copy. (1) alone helped value-heavy RDBs but did nothing for small-key datasets. (2) is what makes it apply broadly. (3) is an independent win on top, and helps even with `rdb-checksum no` since it's pure copy avoidance. ## Design notes - crc64 dispatch: PCLMUL only for buffers >= 64 bytes on x86-64 with the `pclmul` CPUID flag; everything else keeps using crcspeed unchanged. Fold constants are derived at init, not hardcoded. No Barrett reduction - the final 128-bit remainder is reduced via the table path instead. `crc64_init()` self-checks PCLMUL against the table implementation on 48 cases and disables it on any mismatch, so a broken build loses the speedup, never produces a wrong checksum. - The RDB's trailing 8-byte checksum footer must never be folded into the running checksum, but a read (buffered or direct) routinely spans both the last content bytes and the footer. `content_end` (`filesize - 8`) clips the checksum to content only - this is the bug I actually hit building this: without the clip, a read straddling the boundary corrupts every load, caught immediately by the RDB's own checksum verification. - `rdbLoadProgressCallback()` skips its own checksum step via `RIO_FLAG_CKSUM_AT_SOURCE` when the rio's `read()` already accounted for it. - 4KB buffer size chosen from a sweep (64B-4MB): syscall overhead dominates below ~4-16KB, flat within ~1% from 16KB-1MB, larger buffers give a little back to cache eviction. **Not touched, deliberately:** - `redis-check-rdb` - same shape, could use the same fix, left simple since it's not a hot path. - AOF RDB-preamble loading - unsafe: it hands off to `fgets()`/`ftello()` on the same `FILE*` right after the RDB portion, and `rioInitWithFileLoad()` bypassing stdio would desync that. Needs a different design. - Diskless replication full sync - prototyped an analogous batching scheme; benchmarks showed it made small-key diskless syncs ~4% *slower* (three fix attempts, no improvement) with no benefit for large values either. Dropped; left as a follow-up. ## Testing - `crc64Test`: known vectors, PCLMUL-vs-table cross-validation across lengths/alignments/seeds; both crc64 safety nets mutation-tested. - `DEBUG RELOAD`/`redis-check-rdb` round-trips: tiny-key, value-heavy, empty DB, checksum disabled, and values straddling every 4KB boundary in both directions - all byte-identical. - `valgrind --tool=memcheck`: clean across small-key, value-heavy, and truncated/corrupt files. - Full suites passing: `integration/rdb`, `corrupt-dump`, `corrupt-dump-fuzzer`, `aof`, `aof-multi-part`, `convert-ziplist-*-on-load`, `replication`, `replication-psync`. ## Benchmarks (AWS Xeon Platinum 8488C) Datasets sized to ~5s load time on `unstable` to keep round-to-round noise small. 6 alternating rounds each. | Dataset | unstable | this branch | delta | |---|---|---|---| | Small keys (11M keys, 284MB) | 5.303s | 4.572s | **-13.8%** | | Big keys (51,000 x 128KB, 6.3GB) | 4.985s | 3.673s | **-26.3%** | | Huge values (370 x 20MB, 7.3GB) | 5.508s | 4.061s | **-26.3%** | | Cold page cache (EBS-bound) | - | - | ~0% (I/O-bound) | crc64 microbenchmark: 4.2 GB/s (best table path) -> 25.9 GB/s (PCLMUL), 6.2x. **Isolating PCLMUL's contribution** (commit 1 is the one with review-worthy complexity; commits 2+3 are a straightforward buffered-I/O change). Built commits 2+3 cherry-picked directly onto `unstable`, skipping commit 1 entirely, and compared all three variants: | Dataset | unstable | 2+3 only, no PCLMUL | all three | |---|---|---|---| | Small keys (11M keys, 284MB) | 5.124s | 4.382s (**-14.5%**) | 4.336s (**-15.4%**) | | Big keys (51,000 x 128KB, 6.3GB) | 5.016s | 5.033s (**+0.3%, noise**) | 3.671s (**-26.8%**) | For small keys, batching alone captures 94% of the total win - PCLMUL's own incremental contribution on top is only ~1%, since once tiny fields are consolidated into 4KB chunks, the existing `crcspeed` dual/tri-stream table path is already fast enough. For large values, PCLMUL is responsible for essentially the entire win - a large value was already read via one big `rioRead()` call that already exceeded `crcspeed`'s own 2048-byte tri-stream cutoff even in `unstable`, so there was no "many tiny calls" problem for batching to fix, and direct-read's copy-avoidance only becomes proportionally significant once PCLMUL has already shrunk the checksum cost. So whether commit 1 is worth its review complexity depends on which workload shape matters more; commits 2+3 alone are a much simpler change that still captures most of the benefit for small-key-dominated workloads. ## Follow-ups AOF preamble loading, diskless replication (see above), ARM PMULL, VPCLMULQDQ (AVX-512) - all deliberately left out to keep this diff focused. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
b261feaf22 |
Add an INFO stream section with per-consumer-group PEL histograms (#15682)
## Add an `INFO streams` section with stream and consumer-group histograms ### What Introduces a new `INFO` section, `Streams`, carrying per-database base-2 logarithmic histograms of stream and consumer-group properties — same shape as `INFO keysizes`: ``` # Streams db0_stream_distrib_streams_cgroups:0=12,1=4,2=1 db0_stream_distrib_cgroups_pel:1=4,8=2,64=1 db0_stream_distrib_cgroups_consumers:1=5,2=2 ``` - **`stream_distrib_streams_cgroups`** — one sample per stream, valued by its number of consumer groups. Every stream is a sample from birth, so bin `0` counts the streams with no groups and the row totals the stream keys in the database. - **`stream_distrib_cgroups_pel`** — one sample per consumer group, valued by its pending-entry-list size. - **`stream_distrib_cgroups_consumers`** — one sample per consumer group, valued by its number of consumers. The section is not part of the default set: it is printed by `INFO all`, `INFO everything` and `INFO streams`, and gated on a new `stream-stats` boolean, default `no`, `MODIFIABLE`. ### Why `XINFO` answers "how is *this* group doing" for a known key. It cannot answer "does *any* group in this database have a runaway PEL" without `SCAN` + `XINFO` over every stream and every group — O(keys × groups), blocking, and unusable as a scrape target. These histograms answer that in O(1), cheaply enough to poll continuously. They are lossy by design: they tell you a problem exists, not which key. Diagnosis still needs a scan. The other two rows put the PEL in context. A consumer count that keeps growing is the classic leak of per-connection consumer names that are never deleted, and read against the PEL row it separates a group whose backlog is spread over many consumers from one where a single consumer is accumulating unacknowledged work. The groups-per-stream row shows how groups are spread over streams — one stream with hundreds of groups versus hundreds of streams with one — and, through bin `0`, how many streams have no groups at all. Those correlations survive the per-database aggregation, which is what makes the set more useful than the PEL row alone. ### Design Each metric is a **gauge maintained incrementally** — one sample per object, moved between bins as the property changes. `streamUpdateStat()` is the single update primitive for both streams and groups: it moves one sample from the bin of the old value to the bin of the new one, with `-1` meaning "no sample" (an object entering or leaving), and reads the object's stamp (explained below) to know whether the old bin really holds the sample. Hooks reach it through four thin wrappers — update or remove, for a stream or for a group — which name the object and the metric and take the sample from that metric's sampler (`streamStreamSample()` or `streamCGroupSample()`), so a hook cannot pair a metric with the wrong value. The wrappers are always inlined, so with `stream-stats` off every hook is one inlined compare. Metrics come in two units, per-stream and per-group, each with its own sampler; the samplers' switches list every enumerator so that adding a metric without a sampler case is a build warning. Adding a metric is one enum value, one entry in `streamDistribMetricNames[]`, one sampler case, and one line in each of the two walkers that add, remove or tally a whole stream's samples. The `INFO` renderer, the reset path and the ASM delta are metric-agnostic. A command samples the affected object before its own change and updates after it. PEL: XREADGROUP (delivery grows it), XACK/XACKDEL, XCLAIM/XAUTOCLAIM, XNACK (FORCE adds entries), and XGROUP CREATE/DESTROY/DELCONSUMER. Consumers: XREADGROUP, XCLAIM and XAUTOCLAIM when they create a consumer, XGROUP CREATECONSUMER/DELCONSUMER, and XGROUP CREATE/DESTROY for the group entering or leaving. Groups per stream: XGROUP CREATE/DESTROY. Two paths are special: - **Cross-group shrink.** A `DELREF` delete can drop one entry from several groups' PELs at once; this is handled at its single chokepoint, `streamCleanupEntryCGroupRefs()`, which required threading `redisDb *` through `streamTrim`/`streamTrimByLength`/`streamTrimByID` and the module trims. - **Whole-stream load/removal.** RDB/replica load, RESTORE, COPY, MOVE, DEBUG RELOAD and key deletion add or drop a stream and every group at once, via the `streamKeyLoaded`/`streamKeyRemoved` hooks. A stream born through the module API (`RM_StreamAdd` on a missing key) goes through the same hook, so it counts from birth like any other. `streamDistribBin()` bins a value as a `size_t`, exactly as the keysizes histogram does, and asserts the row bound in debug builds. All three metrics are rax sizes, bounded by addressable memory like a key size, so the assertion cannot fire for them. ### Correctness outside the normal command path - **Atomic slot migration (ASM).** Background trim frees migrated stream keys on a BIO thread, bypassing `streamKeyRemoved`, so deltas are tallied off-thread, one row per metric, and subtracted on completion in the main thread. Safe without locking because the trimmed slots are detached into a temporary kvstore *before* the job is queued, giving that thread exclusive ownership — confirmed with ThreadSanitizer. A captured flag plus the database's histogram generation ensure a delta is discarded rather than applied if `stream-stats` was toggled, or the histogram otherwise reset, meanwhile; and the tally counts only samples that are actually in the rows, via the stamps below. The same generation check now also guards the keysizes and allocsizes deltas, whose subtraction after a synchronous FLUSH was previously unguarded. - **Runtime toggling is lazy, and safe.** Enabling does not rescan the keyspace (that would block for roughly half a second per million stream keys), so streams and groups that already exist are not counted until a group write command next touches them: every such command — XREADGROUP (whether or not it delivers anything), XACK, XACKDEL, XCLAIM, XAUTOCLAIM, XNACK and the XGROUP subcommands — enters the group it touches, with its PEL size and consumer count, and the group count of its stream. A stable setup therefore converges on its first read or ack; streams whose groups see no command stay out until a reload or until a group is created on them. The gauges under-count until then, never over-count. What makes that safe is a per-object stamp. Each database's rows carry a generation (`kvstoreMetadata.stream_stats_epoch`), assigned from a server-wide counter whenever they are zeroed so that no two databases ever share one, and each stream and group carries the generation its samples belong to plus one bit per metric saying whether that row holds its sample. `streamUpdateStat()` therefore knows whether a sample is really in the old bin: if it is, the move is exact; if not, only the new value is entered. Bins are anonymous counts, so without the stamp a decrement on behalf of a never-counted object, its first update after enable, its deletion, a slot-trim delta, the rollback of a failed multi-setting `CONFIG SET`, would take some other object's tally and never heal; and without the shared counter a stream MOVEd to another database while stats were off could carry a stamp that the next enable makes current there. The stamp lives in the PEL tree's header for groups and in existing padding for streams, so neither struct grows. Disabling stops the bookkeeping and hides the section; the rows are kept, so a re-enable with no stream change in between, including the rollback of a failed multi-setting `CONFIG SET`, is exact, and otherwise a new generation starts. ### Verification `DEBUG STREAM-STATS-ASSERT <0|1>` mirrors `DEBUG KEYSIZES-HIST-ASSERT`: rebuild the histogram from the keyspace after every command and panic on the first disagreeing bin. Enabling `stream-stats` while the assertion is armed primes it with a full rebuild, since the lazy-enable behaviour above would otherwise look like corruption. It's armed in `unit/info-stream`, `unit/type/stream`, `unit/type/stream-cgroups` and `unit/moduleapi/stream` at no measurable cost, so those suites now double as coverage for the bookkeeping. The arming goes through one helper, `arm_stream_stats_assert`, which skips itself where DEBUG is unavailable rather than tagging the blocks `needs:debug` — that tag would make the external-server CI job skip the suites entirely, trading real coverage for an assertion. Tests include bin-boundary walks for all three metrics, every command that touches a PEL or a consumer set, replica propagation, RDB/`DEBUG RELOAD` reconstruction, `createComplexDataset` cross-checked against `XINFO` (`groups`, `pending` and `consumers`), and cluster tests covering both trim methods, the source/destination split during migration, and a synchronous FLUSH racing an in-flight background trim, with the keysizes rows checked alongside. The lazy-enable path is tested with the assertion disarmed, since re-enabling while armed primes a rebuild that would hide exactly what these tests look for: a stale group's first update steals nothing, deleting a never-counted stream steals nothing, a command that updates two metrics on a stale group (XCLAIM, XGROUP DELCONSUMER) is exact, a same-bin change registers the group, a read by an existing consumer, a read that delivers nothing, an ack, XGROUP CREATECONSUMER/DELCONSUMER/SETID and XNACK each fill in every row of the group and its stream, a stream MOVEd to another database while stats were off is counted there, FLUSHDB of one database leaves another's counts intact, a failed multi-setting `CONFIG SET` leaves the histograms intact, and a quick disable and re-enable keeps the counts. Co-authored-by: debing.sun <debing.sun@redis.com> Co-authored-by: Moti Cohen <moticless@gmail.com> Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com> |
||
|
GuyAv46
|
d003e19b00 |
RED-216257 - Add module reply buffers for deferred serialization (#15775)
Modules sometimes need to serialize a reply before deciding whether to publish it, discard it, or compose it with other independently produced reply sections. Existing thread-safe contexts attach serialization directly to a blocked client, so they cannot represent reply data with an independent lifetime or support foreground construction without a blocked-client handle. This PR adds three module APIs for explicitly owned reply buffers: - `RedisModule_CreateReplyBufferContext(ctx)` creates an empty, reply-only context using the source context's RESP version. It accepts command contexts, blocked-client callback contexts, thread-safe contexts bound to blocked clients, and other reply buffers. - `RedisModule_ReplyWithBufferedReply(dst, buffer)` appends the buffered RESP content to another reply destination without adding a wrapper. On success it consumes the content and leaves the source context empty and reusable. - `RedisModule_ResetReplyBuffer(buffer)` discards all accumulated content and leaves the context empty and reusable while preserving its RESP version, module association, and ownership. The module owns each buffer and releases it with the existing `RedisModule_FreeThreadSafeContext` API. A buffer can outlive its source context and any blocked-client handle; Redis prevents module unloading while owned buffers remain live. Freeing a buffer discards unpublished content, including incomplete postponed-length collections and deferred errors. Creation and freeing currently require the server lock because buffers reuse Redis's temporary-client pool and update module lifetime accounting. Reply construction does not require the server lock. Buffers may be handed between threads under module synchronization, and serialization, movement, and cleanup require exclusive access. Publishing follows the normal threading rules of the destination context. Reset does not require the server lock. With exclusive access, a worker can discard static and list-backed reply bytes, incomplete postponed collections, and deferred errors, then immediately reuse the same buffer. Reset errors are not added to server error statistics. Moves transfer large reply blocks without copying their payloads. They reject invalid sources, open postponed collections, self-moves, destinations without a reply target, and any RESP-version mismatch without modifying either accumulator. Successful moves consume the source even when the destination drops replies, such as a closing client. Deferred errors are counted only if they reach a final destination; discarded errors are not reported as sent replies. The implementation reuses `RedisModule_FreeThreadSafeContext` instead of adding a dedicated free API. The remaining lifecycle decisions, including the temporary-client pool and GIL contract for creation and freeing, are documented in the PR discussion. |
||
|
GuyAv46
|
81ebeccccd |
Fix blocked module auth cleanup on disconnect (#15818)
When a client disconnects after blocked module authentication has been processed for unblocking but before `AUTH` is retried, `freeClient()` frees the retained blocked-client handle directly. This skips the module's private-data cleanup callback and leaves its blocked-client count elevated, leaking private data and preventing the module from unloading. Run the disconnected private-data cleanup and the common blocked-client release path for this case. The temporary clients have already been returned to the pool during unblock processing and must not be released again. This fix uses existing module APIs and is independent of the reply-buffer API proposal. The regression test deterministically kills the auth client between unblock processing and command retry. It checks one cleanup callback with the disconnected flag, no auth reply callback, and successful module unloading. Validation: - Before the fix, the test observes no cleanup callback and module unloading fails; LeakSanitizer reports the leaked 8-byte test allocation. - After the fix, the module-auth, block-on-keys, and background-blocking suites pass with AddressSanitizer and leak detection enabled. |
||
|
Stav-LeviandCursor
|
0b1a456dfa |
Bound a stuck crash report so the process still exits. (#15891)
## Problem On a fatal signal, an assertion failure or a panic, Redis writes a bug report and exits only in `bugReportEnd()`, which re-raises the signal so a core dump can be written. Parts of the report are not guaranteed to finish: `writeStacktraces()` runs `backtrace()` on every thread, and `logModulesInfo()` runs module code. If one of them blocks, `bugReportEnd()` is never reached and the process stays up, unresponsive, with no core. In RED-199873 a shard took `SIGSEGV` inside `libcrypto.so.3`. The log printed `EIP:` and did not print `STACK TRACE DONE`. `logStackTrace()` prints `EIP:` before `writeStacktraces()` and `STACK TRACE DONE` after it returns, so the report stopped inside `writeStacktraces()`. The process stayed up until `SIGKILL`, with no core dump, and node_wd and failover did not restart a master whose PID still existed. ## Approach A deadline on the whole report. When a crash report starts, a one-shot timer is armed for 30 seconds. It's tied to a dedicated signal, `SIGRTMIN+4`, which nothing else in Redis uses (`SIGALRM` belongs to the software watchdog). If the report finishes, the process dies as before and the timer never fires. If it doesn't, the timer's signal runs a small handler that makes the process abort, with a core, no matter where the report is stuck. ## Fix - **Timeout:** 30 seconds, hardcoded (no config). Tests shorten it with `DEBUG SET-CRASH-WATCHDOG-TIMEOUT <seconds>`. - **Arming:** in `bugReportStart()`, which signals, asserts and panics all pass through, whenever the timer isn't already counting down (checked with `timer_gettime()`). The first report arms it; a nested report during the memtest pause re-arms it; a running deadline is never extended. A second thread that crashes at the same time waits on `signal_handler_lock` and is covered by the first thread's deadline. - **When it fires:** the handler logs `CRASH WATCHDOG`, sets `SIGSEGV`, `SIGBUS`, `SIGFPE`, `SIGILL` and `SIGABRT` to default, and calls `abort()`. A stray `SIGRTMIN+4` while no report is running is ignored. - **Why `abort()` and not `kill()`:** when Redis is PID 1 of its PID namespace (e.g. the official Docker image), a default-action signal it sends itself is dropped and `kill()` still returns 0. Normal crashes still die there only because the faulting instruction runs again after the handler returns; the watchdog has no faulting instruction to return to. `abort()` dies even as PID 1, because glibc falls back to a faulting instruction. Resetting `SIGSEGV` first keeps that fault out of `sigsegvHandler()`. - **Memory test:** the deadline is paused only around the memtest loop. Its time grows with memory size, but it takes no locks (the memtest code prints only in `--test-memory` mode), so the timeout doesn't depend on instance size. `killThreads()` and `fopen()` before it, and `dumpCodeAroundEIP()` after it, stay covered. A crash during the pause re-arms the deadline for its nested report. - **Fork children:** POSIX timers aren't inherited, so `redisFork()` creates a new timer in the child. A stuck report in a BGSAVE or AOF-rewrite child no longer leaves the parent waiting forever. - **Startup:** installs the handler, unblocks the signal (the mask can be inherited from the parent), and creates the timer. If that fails, it logs `Crash watchdog disabled` and stays off. - **Logs for the next incident:** the thread scan now logs the path and `errno` when `/proc/<pid>/task` can't be opened, and how many threads it saw when none can receive `SIGUSR2`. In the incident, both cases were silent. Regardless of what caused this hang, a stuck report is now cut off within a bounded time, the process dies with a core, and the shard can be restarted automatically. If it happens again, the core will show where it was stuck. This change doesn't explain or fix the `libcrypto` crash itself. ## Not covered - `sigalrmSignalHandler()` (the on-demand/periodic stack dump) calls the same collector and can hang the same way. It's left unbounded on purpose: it isn't a crash path, and bounding it would let a diagnostic signal kill a healthy process. - node_wd should still not rely on Redis to exit: if a shard is unresponsive for longer than the 30-second crash watchdog timeout, take a core (`gcore` or `SIGQUIT`) and then `SIGKILL`. ## Tests - `./runtest --single integration/logging`: a healthy `DEBUG SEGFAULT` with `DEBUG SET-CRASH-WATCHDOG-TIMEOUT 2` finishes, logs `REDIS BUG REPORT END`, and doesn't log `CRASH WATCHDOG`. - `./runtest --single unit/moduleapi/crash`: `MODULE LOAD crash.so hang` makes the module's info callback sleep during the report, with a 2-second timeout. The watchdog line appears, the process dies, and `REDIS BUG REPORT END` is absent. It checks that the process dies, not that a core file was written. Checked by hand: | Case | Result | |---|---| | Stuck report | dies after ~2s, `Aborted (core dumped)` | | Stuck report, Redis as PID 1 (`unshare -Urpf --mount-proc`) | dies, core dumped (used to hang) | | Normal crash, PID 1 | dies, no watchdog line | | Stray `SIGRTMIN+4` on a healthy server | keeps running | | 2 GB instance, memcheck on, 1s timeout | report finishes, no watchdog line | | `BGSAVE` | ok, timer created in the child | ## Answers to the R&D questions **Why does `writeStacktraces()` fail to enumerate the process's threads?** We don't know why the first attempt never returned. The log shows the report starting, the `EIP:` line, and then nothing: no `STACK TRACE DONE`, and no exit. It also doesn't show any of the error messages the thread scan logs when it fails, so we can't confirm that the scan itself failed. The `Failed to get the process's threads` lines that do appear come from later `kill -ALRM` dumps sent by hand during the incident. Each of them returns. They likely find no threads because every thread was still stuck inside the earlier stack collection, where `SIGUSR2` is blocked. That explains those lines, but not the first attempt. The best guess is that the crashing thread got stuck in its own `backtrace()` call. It's the first `backtrace()` in the process, so glibc loads `libgcc_s` there, which takes the loader lock and calls `malloc`, from a signal handler, on every thread at once. A thread interrupted while holding a `malloc` lock would block that, and the crashing thread would wait behind it. This happens before the 2-second `wait_threads()` timeout starts, so nothing stopped it. The log doesn't prove this, and the `libcrypto` crash itself remains undiagnosed: we have one frame and no core. **Harden the crash path so it still escalates to abort()/exit within a bounded time.** Done. A deadline starts the moment any crash report begins (signal, assertion or panic). If the report hasn't finished within 30 seconds, the process is aborted with a core dump, also when Redis runs as PID 1. node_wd and the SM see a dead process and can restart the shard, and the next occurrence can be diagnosed from the core. --------- Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
chenshi
|
3228d82f49 |
Fix bracketed IPv6 cluster CLI address parsing (#15890)
## Summary `redis-cli --cluster` accepts an unbracketed IPv6 endpoint such as `::1:7000`, but it does not accept the standard bracketed form `[::1]:7000`. The Cluster CLI address parser treats `[::1]` as the host name and passes it to the connection layer unchanged. Name resolution then fails before the Cluster command runs. This is a client-side endpoint parsing issue only; it does not change Cluster bus messages, persisted topology, or server-side IPv6 behavior. ## Changes - Recognize `[IPv6]:port` in `parseClusterNodeAddress()`. Signed-off-by: chenshi5012 <chenshi5012@163.com> |