Repository navigation
Conversation
…redis#4368) In `generateStringArgValue`, the `argName` dispatch chain contains two identical `else if (strcmp(argName, "command") == 0)` branches. The earlier branch already handles every `argName == "command"` case, so the second one, whose body is identical, is unreachable dead code. This removes the duplicate branch; behavior is unchanged. Signed-off-by: latent-9 <296084221+latent-9@users.noreply.github.com>
Improves spelling and grammar across codebase --------- Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Grammar corrections in comments --------- Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com> Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
…s#4381) Stream listpack master entries store separate live and deleted record counts, but integrity validation only verified their sum. A corrupted payload could therefore preserve the total while understating the live count, causing XLEN to disagree with the stored records and allowing XDEL to discard records not accounted for by the header. Count live and deleted records independently while validating stream listpacks and reject payloads when either count differs from its declared value. Signed-off-by: Roshan Khatri <roshanvkhatri@gmail.com>
Consolidating nonexistent spelling Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
…edis#4402) `src/commands/config-info.json` and `src/commands/move.json` still carry `"since": "10.0.0"`. *This was generated by AI but verified, with love, by a human.* Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
…load (redis#4360) moduleUnregisterCleanup did not remove the module's cluster message receivers. The stale entries kept pointing at the freed ValkeyModule, so a later cluster message of that type dereferenced r->module in moduleCallClusterReceivers, causing a use-after-free. Added a MODULE UNLOAD test to verify the fix, and also added type=254 to allow us to verify the correctness of the loop logic. --------- Signed-off-by: Binbin <binloveplay1314@qq.com>
…ndlers (redis#4401) Follow-up of redis#3611. Fixes a regression introduced by that PR. The May 27 On-Demand run on this PR (SET/GET, 96B, io-threads 2/10, pipeline 1/10) showed no significant RPS change. After `17ec23f` removed `post_read_done_postpone_mask`, `processClientIOReadsDone()` started postponing READ and returning `needs_post_read_update = 1` for every non-ACCEPTING completed read. That second phase (`lookupClientByID` + `processPendingCommandAndInputBuffer` + `connUpdateState`) is only required when `update_state` may synchronously invoke handlers. For other transports it is per-completion overhead. This matches the post-merge dashboard drop: small payloads, io-threads, **P1 worse than P10**. This PR restores the original gate without bringing `struct client` into the connection driver (the review concern that led to `17ec23f`): - `ConnectionType.sync_handlers_in_update_state` (0 by default) - Set only where `update_state` can sync-call handlers - Mask is still computed in `networking.c` from IO state - `connUpdateState()` still runs immediately, including ACCEPTING --------- Signed-off-by: quanyeyang <quanyemostima@gmail.com>
Log EXEC in commandlog. Catches cases where there's no individually slow command. --------- Signed-off-by: Michelle Lee <michellee.3104@gmail.com>
…dis#4404) `unit/commandlog` fails on unstable at 917de6a: ``` *** [err]: COMMANDLOG slow - Redaction does not leak to later commands in a MULTI in tests/unit/commandlog.tcl Expected 'set foo bar' to be equal to 'exec' (context: type eval line 12 cmd {assert_equal {set foo bar} [lindex [lindex $slowlog_resp 0] 3]} proc ::test) ``` redis#4267 dropped `SKIP_COMMANDLOG` from EXEC (`src/commands/exec.json:12`), so EXEC is now logged after the commands it ran, which puts it at entry 0 of the newest-first `COMMANDLOG GET`. The redaction test at `tests/unit/commandlog.tcl:212`, added by redis#4323, still reads entry 0 and gets the EXEC instead of the SET. Neither PR was rebased on the other, so this only broke on merge and not in either PR's CI. The log after `MULTI; ACL SETUSER commandlog-test-user +get; SET foo bar; EXEC`: | Entry | Command | |---|---| | 0 | `exec` | | 1 | `set foo bar` | | 2 | `acl setuser (redacted) (redacted)` | Read the SET from entry 1 instead. The other tests redis#4267 touched already assert entry 0 is `exec` and entry 1 is the inner command, so this matches. Also assert entries 0 and 2 rather than only the SET. The point of the test is that the ACL SETUSER redaction stops at the ACL SETUSER, and asserting entry 2 is redacted is what keeps it from passing if redaction breaks entirely. Asserting entry 0 is `exec` makes the index arithmetic fail loudly next time the entry order changes, instead of silently comparing against the wrong entry the way it just did. ## Testing `unit/commandlog` goes from 29 passed, 1 failed to 30 passed. `unit/slowlog` and `unit/multi` are unchanged at 100 passed. Reverting the `src/` hunks of 917de6a confirms that commit is the trigger. The new entry-0 assert fails, along with the two EXEC tests 917de6a added: ``` *** [err]: COMMANDLOG slow - Redaction does not leak to later commands in a MULTI in tests/unit/commandlog.tcl Expected 'exec' to be equal to 'set foo bar' (context: type eval line 13 cmd {assert_equal {exec} [lindex [lindex $slowlog_resp 0] 3]} proc ::test) *** [err]: COMMANDLOG slow - EXEC is logged alongside slow inner commands in tests/unit/commandlog.tcl Expected '1' to be equal to '2' (context: type eval line 8 cmd {assert_equal [r commandlog len slow] 2} proc ::test) *** [err]: COMMANDLOG slow - EXEC records total transaction time when inner commands are individually fast in tests/unit/commandlog.tcl Expected '0' to be equal to '1' (context: type eval line 9 cmd {assert_equal [r commandlog len slow] 1} proc ::test) ``` *This was generated by AI but verified, with love, by a human.* Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
The cluster message receiver array was sized UINT8_MAX (255), so the valid index range was 0..254. Two issues existed: - VM_RegisterClusterMessageReceiver allowed type 255, an out-of-bounds WRITE past the end of the array. - moduleCallClusterReceivers would perform an out-of-bounds READ when a cluster bus packet carried type 255 (UINT8_MAX), which can be sent by any cluster member. Grow the array to MAX_CLUSTER_MESSAGE_TYPES (UINT8_MAX + 1) so the full uint8_t type range (0..255) is covered, and use the explicit constant in both dispatch and unregister paths. type 255 is now a valid, dispatchable message type. Signed-off-by: Binbin <binloveplay1314@qq.com>
The current approach of manually inspecting logs to determine how long a full sync took is highly cumbersome, hence introducing a new metric called `last_successful_sync_duration_ms `. The new metric `last_successful_sync_duration_ms `, which measures the total time (in milliseconds) taken to complete the last full synchronization, will be reported by replicas in the `INFO replication` output. ``` 127.0.0.1:6380> info replication # Replication role:slave master_host:localhost master_port:6379 master_link_status:up master_last_io_seconds_ago:1 last_successful_sync_duration_ms:15537 <<<<<<<<<-------------===== I AM HERE! master_sync_in_progress:0 slave_read_repl_offset:5345000149 slave_repl_offset:5345000149 replicas_repl_buffer_size:0 replicas_repl_buffer_peak:0 slave_priority:100 slave_read_only:1 replica_announced:1 connected_slaves:0 replicas_waiting_psync:0 master_failover_state:no-failover master_replid:51005812f8730e42095a20c1fe9397c08abd608e master_replid2:0000000000000000000000000000000000000000 master_repl_offset:5345000149 second_repl_offset:-1 repl_backlog_active:1 repl_backlog_size:10485760 repl_backlog_first_byte_offset:5345000122 repl_backlog_histlen:28 ``` --------- Signed-off-by: Satheesha Gowda <satheesha.balaji@gmail.com>
Linux exposes signal masks as 64-bit hexadecimal values, including to 32-bit processes. Parsing them as unsigned long overflowed on Alpine 32-bit, preventing the crash handler from collecting thread stack traces. Parse signal masks as unsigned long long and add coverage for values above the 32-bit range and overflow handling. Failed in [Daily CI](https://github.com/valkey-io/valkey/actions/runs/31798198043/job/94759856769). Verified with 100 runs of the logging tests on Alpine 32-bit: [1200 passed, 0 failed](https://github.com/sarthakaggarwal97/valkey/actions/runs/31821271902/job/94834820008). Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
`DEBUG SLEEP 0.03` uses `nanosleep()`, while command duration is measured as wall-clock elapsed time around each `call()`. On a contended runner, an inner command can therefore take more than the 100 ms threshold and be correctly added to SLOWLOG and COMMANDLOG. The tests assumed that the entire log would contain exactly one entry. That assumption is timing-sensitive and is not required to verify the intended behavior: EXEC is recorded with the total transaction duration and is the newest entry. Signed-off-by: Avi Fenesh <aviarchi1994@gmail.com>
Vendor lz4, lz4hc, lz4frame and xxhash from https://github.com/lz4/lz4 (v1.10.0) under deps/lz4 with build wiring for make and cmake. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Add Valkey Compressed Stream (VCS) support for RDB persistence with LZ4 as the first whole-stream codec. The RDB is wrapped in a VCS envelope and compressed as a single stream at the rio layer. On load, the RDB path probes for the envelope and transparently decompresses when present, falling back to the plain RDB path otherwise. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Permissions must not be checked when replaying commands from an AOF because all that matters is that the commands were allowed at the time they were originally performed. Checking permissions can result in silent data loss. Fixes redis#3983 --------- Signed-off-by: Luke Palmer <luke@lukepalmer.net>
…expire (redis#3118) This affect active expire, when we deleting the expired keys or hash fields from active expire, now we will incr the dirty counter. Otherwise, it will affect the `save` configuration, for example, when active expire removes all expired keys/fiedls, we can trigger a new bgsave. Note that it does change the behavior in some sense, as now the save frequency might increase for some workloads. Note that we don't do this for lazy expire and key eviction since we don't want to propagate read commands during the command process. Theoretically, both lazy expire and key eviction should incr the dirty since it is indeed making changes in the DB, but we do currently have the limitations. Note that this change may cause the SAVE frequency to increase. In most cases, users configure SAVE only on the replica side. When the primary deletes a expired key, it propagates a `DEL` command to the replica, which then delete the key as well; in this scenario, since the replica is already incrementing its "dirty" counter, so the replica might not exhibit any behavioral changes. Expirations are propagated as DEL commands, so the replica increments the dirty counter when it receives DEL. This is a difference between primary and replica behaviour. This PR removes that difference and treats expirations as writes, same on primary and replica. Signed-off-by: Binbin <binloveplay1314@qq.com>
…nvokes handlers (redis#4440) Follow-up of redis#3611 and redis#4401. redis#3611 reordered `processClientIOWriteDone()` to call `postWriteToClient()` before `connUpdateState()`, and added a `lookupClientByID()` re-fetch after the update so the flow survives the client being freed. That re-fetch is only required when `connUpdateState()` may synchronously invoke handlers (e.g. RDMA) and free the client; for other transports it is per-write-completion overhead. redis#4401 restored this gate on the read-done path (`processClientIOReadsDone`). This PR mirrors it on the write-done path with the same `connUpdateStateMayInvokeHandlers()` helper: record whether the transport may invoke handlers, always call `connUpdateState()`, and re-fetch the client by id only when it may have been freed. Unlike the read path, `processClientIOWriteDone()` cannot run while the connection is `CONN_STATE_ACCEPTING` — accepts complete on the read path (`ioThreadAccept` → `JOB_RES_READ_CLIENT` → `processClientIOReadsDone`), and no reply is queued before the handshake finishes — so no accept-state guard is needed here. This removes one `lookupClientByID()` (radix-tree lookup) per write completion for socket/TLS clients. SET is unaffected because its regression was read-side and already addressed by redis#4401. Signed-off-by: Daniel Gershkovich <dgershko@gmail.com>
When a replica disconnects from its primary, replicationCachePrimary() calls unlinkClient(), which sets c->conn to NULL. freeClient() only rolled back the client's memory from stat_clients_type_memory[] when c->conn was non-NULL, so the cached primary's contribution leaked. It causes the replica side to see a high value for the `mem_clients_normal` INFO field. A client is only accounted while connected: updateClientMemoryUsage() (the only place that sets c->last_memory_usage) asserts c->conn. Use c->last_memory_usage as the rollback condition so a previously counted client is always subtracted, regardless of c->conn. Add a debug assert that either c->conn is set or we are releasing a cached_primary. Signed-off-by: Binbin <binloveplay1314@qq.com>
…lution (redis#4469) `custom_matchers.hpp` included `wrappers.h` (which pulls in server.h) before `gmock/gmock.h`. Since `server.h` defines `C` macros `#define min(a,b)` and `#define max(a,b)`, these macros were active when the preprocessor parsed gmock. When the generated files are fresh, gmock's header guard is already set by the time `custom_matchers.hpp` tries to include it again, which masking the bug. When generated files are stale or the include graph changes, `custom_matchers.hpp` becomes the first to pull in gmock, exposing the latent ordering issue. Signed-off-by: harrylin98 <harrylin980107@gmail.com>
…ld (redis#4454) Add a "replica-priority" aux field so a node's non-default cluster-replica-priority survives restarts and is restored from nodes.conf instead of waiting for the next gossip round. Trigger CLUSTER_TODO_SAVE_CONFIG when a node learns a new replica priority via gossip (updateReplicaPriority) and when the local node's priority is updated via CONFIG SET (clusterUpdateMyselfReplicaPriority), so nodes.conf is rewritten. It was introduced in redis#2204, but was omitted. Signed-off-by: Binbin <binloveplay1314@qq.com>
`CPPFLAGS` is the usual place for preprocessor flags, but `deps/fpconv`,
`deps/hdr_histogram`, `deps/linenoise` and `src/Makefile` use explicit
recipes and never
pass it to the compiler, so anything set there is dropped for those
objects. The rest of
the tree picks it up from make's implicit rules.
This mostly shows up in packaging: Debian's `dpkg-buildflags` and RPM's
`%{optflags}` both
put hardening definitions in `CPPFLAGS`, so today those components are
built without them.
Debian carries this as `0002-Add-CPPFLAGS-to-upstream-makefiles.patch`,
and
`valkey-io/valkey-release-automation` carries a copy as well, so taking
it upstream would
let both drop it. Original patch by lamby — I'm forwarding it.
Discussed in redis#1882.
---------
Signed-off-by: Feng Ruohang <rh@vonng.com>
Co-authored-by: Chris Lamb <lamby@debian.org>
…edis#4488) Update valkey.conf to use the real config name cluster-node-timeout (instead of node-timeout) in the failover formula and use its default value (15s) in the example, matching default behavior. Also fix the error log in valkey-cli.c to print the correct config name. Signed-off-by: Binbin <binloveplay1314@qq.com>
Fixes redis#4484 Fixes redis#4485 Fixes redis#4486 The tests counted cumulative error messages in a shared stdout file. Clear the shared server stdout before each affected test so log assertions only count messages produced by the current server instance. This avoids cumulative-count flakes and works with all supported log formats. --------- Signed-off-by: quanyeyang <quanyemostima@gmail.com>
…edis#4496) Some CI environments run slowly, so the paused old primary can stay disconnected from its primary long enough to exceed the cluster-replica-validity-factor limit. In that case the old primary is disqualified from the second auto failover. Fixes redis#4406 Signed-off-by: Binbin <binloveplay1314@qq.com>
One test name spells it `writeable`. Outside `deps/` that is the only occurrence left in the tree, everywhere else is `writable`. Current releases of `typos` flag it, and the Spellcheck job installs `typos` through `taiki-e/install-action` without pinning a version, so the job can turn red without anyone touching this repo. With this word changed, `typos --config=./.config/typos.toml` is clean across the tree. It is a test name, so nothing changes behaviour, and nothing else refers to the old one. Signed-off-by: melancholictheory <selimvhorst@gmail.com>
Fixes redis#4458 ### Description Both slot-stat comparators return a `uint64_t` difference as `int`: ```c return entry_a.stat - entry_b.stat; ``` The subtraction is done in `uint64_t` arithmetic and then truncated to `int`, so `ORDERBY` sorts incorrectly once two slots' counters differ by more than `INT_MAX` — and a difference of exactly 2³² truncates to zero, making two very different slots compare equal. This affects the cumulative counters `cpu-usec`, `network-bytes-in` and `network-bytes-out`, which grow without bound until `CONFIG RESETSTAT` or a slot ownership change. Replaced with an explicit three-way comparison, keeping the existing tie-break on slot number (safe, since slots are bounded at 16383): ```c return (entry_a.stat > entry_b.stat) - (entry_a.stat < entry_b.stat); ``` The values are only compared, never subtracted, so nothing can overflow. This idiom is already used at `src/vset.c:869`. Surfaced by the review of redis#4455, which adds a fourth cumulative byte counter that would inherit this. ### Testing Added a test to `tests/unit/cluster/slot-stats.tcl` that drives one slot's `network-bytes-in` past `INT_MAX` (repeatedly overwriting one large key, so memory stays flat) and asserts the ordering in both directions. It fails without the fix — the heavy slot lands last in a `DESC` sort — and passes with it. Tagged `slow`: the test moves ~2.2 GB and takes ~150s. --------- Signed-off-by: Zuoying Jiang <jackjiang14@hotmail.com> Signed-off-by: Madelyn Olson <madelyneolson@gmail.com> Co-authored-by: Madelyn Olson <madelyneolson@gmail.com> Co-authored-by: Binbin <binloveplay1314@qq.com>
Rework handleClientsBlockedOnKey to re-walk the live blocking list each round and advance via listRotateHeadToTail, instead of snapshotting client ids and re-resolving them. The original fix (redis#4212) was somewhat complex and difficult to read, rewrite it in a simpler way. Signed-off-by: Binbin <binloveplay1314@qq.com>
…edis#4492) Internal refactor to support redis#4076. No functional change. Signed-off-by: Jim Brunner <brunnerj@amazon.com>
…exes (redis#4823) ## Problem On 64-bit builds, `LSET` parses its index as `long`, but `listTypeReplaceAtIndex()` narrowed it to `int`. An out-of-range index such as `4294967296` became `0`, causing `LSET` to return `OK` and modify the first element instead of returning `ERR index out of range`. This is a correctness issue affecting both listpack and quicklist lists. ## Fix Preserve the `long` index through `listTypeReplaceAtIndex()` so an out-of-range index returns an error without modifying the list. Add regression tests for large positive and negative indexes in both encodings. ## Tests - `./runtest --single unit/type/list --quiet`: 289 passed, 0 failed. - Confirmed the command returns `ERR index out of range` and leaves the list unchanged after the fix. On 32-bit builds, these values are rejected when parsing the `long` index; the narrowing bug occurs on 64-bit builds. --------- Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
**Summary** Fixes [GHSA-f4q9-j22m-xqq9](GHSA-f4q9-j22m-xqq9), a heap buffer overflow in cluster Pub/Sub message construction reported through ZDI `clusterCreatePublishMsgBlock()` calculated message lengths using `size_t` but passed the result through a 32-bit length field. An oversized channel and message could therefore cause a much smaller buffer to be allocated before copying the full payload. **Changes** - Validate the complete serialized message length before allocation or 32-bit conversion. - Reject oversized `PUBLISH` and `SPUBLISH` requests before publishing locally or propagating to the cluster. - Adds focused tests for valid and oversized payloads. Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
) Since redis#3402, `valkey-cli` reads environment variables before it parses command line options, so options can override them. The "Using a password with '-a' or '-u' option on the command line interface may not be safe" warning only checks that a password is set, so a password from `VALKEYCLI_AUTH` or `REDISCLI_AUTH` alone now prints it, even though nothing was on the command line. The fix warns only when `-a` or `-u` replaced the password the environment provided. `-a` still warns and still overrides the environment variable. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
…is#4954) Fixes redis#4950 On a btree zset with more than one distinct score, the lex range functions in `ordered_index.c` packed the bounds with the score of the smallest member, so they only looked at the lowest score bucket. `ZLEXCOUNT`, `ZREMRANGEBYLEX`, `ZRANGEBYLEX`, `ZREVRANGEBYLEX`, `ZRANGE ... BYLEX` and `ZRANGESTORE ... BYLEX` could all return something different from listpack. `ZREMRANGEBYLEX` and `ZRANGESTORE` are replicated as is, so a primary and replica holding the key in different encodings could diverge. For keys with more than one score, these now follow listpack: - count and delete use the run in score order starting at the first element >= min, up to the first element after it > max (delete removes it by rank) - forward range seeks start at the first element >= min, reverse at the last element <= max scanning from the end (like `zzlLastInLexRange`), then apply the LIMIT offset; the caller already stops at the far bound Single-score keys keep the existing O(log N) paths. I matched listpack rather than the 9.1 skiplist where they differ: 9.1's skiplist can disagree with itself, e.g. `ZADD z 0 b 0 d 1 a 1 c 2 e` with `[a [c` gives `ZLEXCOUNT` 4 but removes 1, while listpack gives 1 for both. Known trade-off: on mixed-score keys, finding the start of a lex range is a linear walk in score order, so it costs O(N) in the worst case, while the 9.1 skiplist and unstable did an O(log N) descent (with inconsistent results on mixed scores). It shows up most when members that are next to each other in lex order share a score, e.g. @charsyam measured `ZRANGEBYLEX ... LIMIT 0 10` from the middle of 20k/30k-member keys going from about 0.7 µs to 59/88 µs. Stepping one score at a time instead of one member at a time would cut this down and can be a follow-up. Tests: fixed mixed-score cases for count/delete and for forward/reverse ranges with LIMIT and ZRANGESTORE on both encodings, plus a fuzz test checking listpack and btree agree on random mixed-score sets for all of these. They fail without the change. I also ran 5000 random cases locally with sets up to 600 members. --------- Signed-off-by: Aman Kumar <amankumar020003@gmail.com>
Missed this test improvement when submitting valkey-io/valkey#4996 --------- Signed-off-by: Jim Brunner <brunnerj@amazon.com> Signed-off-by: Rain Valentine <rsg000@gmail.com> Co-authored-by: Rain Valentine <rsg000@gmail.com> Co-authored-by: valkey-review-bot[bot] <282604435+valkey-review-bot[bot]@users.noreply.github.com>
…redis#4655) The election bound subtracted a voter twice if it NACKed and was then marked FAIL, resetting elections that were still winnable. Track ACK/NACK per voter per election instead and add unit tests. Fixes redis#4626 --------- Signed-off-by: Dante Knowles <xdk@amazon.com>
moduleBlockClient() treated every subscribed client as unblockable because getClientType() returns CLIENT_TYPE_PUBSUB. A RESP3 client can still run commands while subscribed, so ValkeyModule_BlockClient and ValkeyModule_BlockClientOnAuth returned NULL with ENOTSUP and queued no reply. Allow blocking those clients again, matching the processCommand rule that only RESP2 Pub/Sub clients are restricted. Keyspace notifications still cannot block them. Introduced by PR 1819 (8.1.1). Restores the pre-8.1.1 behavior for RESP3 subscribed clients; no change for RESP2 clients or keyspace-notification contexts. Fixes redis#4862 --------- Signed-off-by: quanyeyang <quanyemostima@gmail.com> Signed-off-by: Rain Valentine <rsg000@gmail.com> Co-authored-by: Rain Valentine <rsg000@gmail.com> Co-authored-by: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com>
Repeated `BYPOLYGON` options in `GEOSEARCH` and `GEOSEARCHSTORE` overwrite the previous vertex array without freeing it. Only the final array is released, leaking memory even when the source key does not exist. Free the previous vertices before allocating a replacement, preserving the existing behavior of using the last polygon. Add regression tests for both commands, invalid `COUNT` cleanup, and retained memory across repeated requests. redis#3807 renames `geoPolygonPointsFree()` to `geoVerticesFree()`. If redis#3807 is merged first, I'll rebase this PR and use the new name. Validation: - GEO suite: 75 passed. - New tests with 4 I/O threads and `io-threads-always-active yes`: 2 passed. - The leak regression fails against the parent commit and passes with this fix. Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
GitHub is moving `ubuntu-latest` from 24.04 to 26.04, which mounts
`/tmp` as tmpfs. The `test-ubuntu-reclaim-cache` job writes `dump.rdb`
under `/tmp/master`, so the file stays fully resident in page cache and
the "SAVE doesn't increase cache" check fails.
This moves the job's data directories to `${RUNNER_TEMP}`, which is
disk-backed, and renames them to `primary`/`replica`.
The same change is already on the 7.2-9.1 backport sweep branches.
- [Backport sweep for 7.2
failure](https://github.com/valkey-io/valkey/actions/runs/40362847376/job/121712607489)
- [Backport sweep for 8.0 passing on
26.04](https://github.com/valkey-io/valkey/actions/runs/40383833511/job/121787504093)
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
For large key lists, `ValkeyModule_GetCommandKeysWithFlags` allocates temporary storage in `getKeysResult` and returns without freeing it. This adds the missing cleanup after copying the positions and flags into the returned arrays. Fixes redis#5042 Signed-off-by: Christopher Lim <41635847+christopherlim98@users.noreply.github.com>
Fixes redis#4138, redis#4137, redis#4136, redis#4135, redis#4134, redis#4133, redis#4132, redis#4131, redis#858, redis#4978 ### Root cause Starts at R1. ``` R0 00:55:54.481 * Migrating slot 609 to node 8b6df7cd (R1) ``` R1 to R4, on purpose. ``` R0 00:55:57.358 * Failover auth granted to 27ba0c0f (R4) for epoch 7 R0 00:55:57.774 - Failover occurred in migration target. Slot 609 is now being migrated to node 27ba0c0f (R4) ``` R4 back to R1, on purpose. ``` R1 00:56:02.028 * Failover election won: I'm the new primary. R0 00:56:02.398 - Failover occurred in migration target. Slot 609 is now being migrated to node 8b6df7cd (R1) ``` R1 to R4 again, by mistake. - R4 has not finished demoting, so it keeps gossiping its epoch 7 slot claim. - Each of those packets makes R1 and R0 walk all 16384 slots. - R1 goes quiet long enough to be declared dead. - R2's fail report from the deliberate freeze is still valid, so PFAIL becomes FAIL at once. - R4 is the only replica in that shard, so it skips the election delay and takes the shard back. ``` R0 00:56:03.878 * NODE 8b6df7cd (R1) possibly failing. R0 00:56:03.878 * Marking node 8b6df7cd (R1) as failing (quorum reached). R4 00:56:04.812 * This is the best ranked replica and can initiate the election immediately. R4 00:56:04.812 * Start of election delayed for 0 milliseconds (rank #0, primary rank #0, offset 175, ...) R4 00:56:04.812 * Starting a failover election for epoch 9, node config epoch is 8 R4 00:56:05.154 * Failover auth ACK from 8b6df7cd (R1) for epoch 9 (ACKs 2, quorum 2) R4 00:56:05.154 * Failover election won: I'm the new primary. R0 00:56:05.535 - Failover occurred in migration target. Slot 609 is now being migrated to node 27ba0c0f (R4) ``` The state never comes back to R1, which causes a failure `expected [609->-8b6df7cd]; got [609->-27ba0c0f]`. Followup tests inherit the broken topology which causes cascading failures. ### Fix - `cluster-node-timeout` is now 5000 ms instead of 1000 ms, so an event loop that is merely busy is no longer read as a dead node. - Restoring a node now goes through a `failback` helper that first waits for every node to clear that node's `fail?` and `fail` flags. - A `normalize_cluster` helper runs at at test failures that could inherit a broken layout so a followup test doesn't inherit a broken state - The empty-shard migration tests disabled in redis#858 are enabled again. That issue reports the same failure signature, and they pass with this change. --------- Signed-off-by: Bara' Hasheesh <bara.hasheesh@gmail.com>
The clang-format check fails on PRs that land on an ubuntu-24.04 runner. ubuntu-latest is mid-migration to 26.04, and since redis#5029 the job installs Ubuntu's own `clang-format-18`: 18.1.3 on 24.04, 18.1.8 on 26.04. The tree is formatted with 18.1.8, so whether a PR passes depends on which image it gets. This installs clang-format 18.1.8 from PyPI, the same package CONTRIBUTING.md tells contributors to use, so the runner image no longer matters. Replaces redis#5064. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
`objectComputeSize()` samples a hashtable-encoded hash with a validating iterator, which skips fields whose TTL has passed but that have not been reaped yet. Those fields still occupy memory, and `hashtableSize()`, which the sampled average is multiplied by, still counts them. The result is under-reported whenever expired fields are larger than live ones. **Example:** one small live field plus 99 expired 1 KB fields, with active expiry disabled. `MEMORY USAGE` reports ~4 KB for a hash that holds ~100 KB. **Fix:** iterate with `HASHTABLE_ITER_SKIP_VALIDATION`. This matches the rest of `objectComputeSize()`, where the listpack branch uses `zmalloc_size()` and sets and sorted sets have no validator, and it matches `rdbSaveObject()`, which already skips validation when walking the hash. It also removes a dependency on expiry evaluation, which reads server state a size estimate should not need. Once expired fields are reaped the reported value is unchanged. Existing `hashexpire.tcl` tests that check memory drops after expiry wait for reaping first, so they are unaffected. Adds a regression test to `tests/unit/hashexpire.tcl`. Signed-off-by: Sushil Paneru <sushil.paneru1@gmail.com>
Calling `ValkeyModule_Call` with `S` but without `E` crashes on validation errors because cleanup calls `sdslen()` on a NULL error message. Use a generic ERR message for error accounting when no reply message was requested. --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com> Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Previously in valkey-io/valkey#3122 it was decided not to make this API public in 9.1 version. Making it public for 9.2 so other Lua modules would be able to use less overhead API (e.g. LuaJIT https://github.com/valkey-io/valkey-luajit and later Lua 5.5 module) --------- Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com> Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
…tu-ARM, and fix macOS build
…ocksDB v11.12+ (C++20) - Merge latest valkey/unstable while preserving full commit history - Adapt Swap engine (kvstore, dict, robj, commandlog, replication, rordb, eviction, expire, persist, ttl_compact, blob_list_gc) to Valkey architecture - Upgrade deps/rocksdb to webc-fork/rocksdb (v11.12+ main) and C++ standard to C++20 - Update README.md, ror.conf, valkey.conf, and todo.md for Valkey-On-Rocks - Pass all 39 swap unit test suites, all 19 swap integration test suites, and valkey-benchmark checks with zero compiler warnings (-Werror)
…d unit-test to CI build-job
…lease, and clean dead declarations
…ncodeMetaScanKey/decodeMetaScanKey
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景与目标
本 PR 将 Redis-On-Rocks 的基线从 Redis 迁移到 Valkey（
valkey/unstable），在完整保留 swap/RocksDB 自研能力的前提下适配 Valkey 架构，并将 RocksDB 依赖升级到 v11.12+ / C++20。合并后main即为「Valkey-On-Rocks」。deps/rocksdb的 gitlink 由e8d3b44（ctrip-v8.5.4）升级为3b84f021f，该提交即 [feat] 同步上游官方 RocksDB main（v11.12+）并启用 C++20 rocksdb#10（[feat] 同步上游官方 RocksDB main（v11.12+）并启用 C++20，headwebc-fork:ctrip-main）的 head。3b84f021f目前只存在于 ctripcorp/rocksdb 的 PR ref 上，因此请先合并 Poc reduce lock content #10，并且请使用 merge commit 方式（不要 squash / rebase），否则该 SHA 会被重写，本 PR 的子模块指针将悬空。.gitmodules的deps/rocksdbURL 仍指向webc-fork/rocksdb（其ctrip-main分支与本 PR gitlink 完全一致）。Poc reduce lock content #10 合并后我们会补一个小提交把该 URL 改回ctripcorp/rocksdb；若评审希望现在就是ctripcorp/rocksdb，请告知，我们立即改（需 Poc reduce lock content #10 先以 merge commit 合并）。deps/xredis-gtid未改动（仍为ctripcorp/xredis-gtid，gitlinke4c64ef3不变）。与 #461 的关系（重叠提示）
#461（
[ci] configure multi-platform CI ...）的 4 个提交已包含在本 PR 的历史中：55d1803ee、734ffd1f6、15c9a5969、885e4ec11。建议先合 #461，或在本 PR 中一并评审，避免重复讨论。变更内容
3b84f021f）and C++ standard to C++20src/*（Valkey 架构适配）、src/commands（452）、deps/*（jemalloc、libvalkey、hiredis、lua 等）、tests/{unit,integration,swap,modules}、.github/workflows规模
ctripcorp:main：ahead 4024 / behind 0，merge-base =227336a7a（无冲突）885e4ec11：2073 files changed, +478454 / −139464（4020 commits）测试
valkey-benchmark校验通过-Werror）