The first test in shutdown.tcl waited a fixed 100ms after
rdb_bgsave_in_progress flipped to 1 before asserting the child's temp
rdb exists. That flag is set by the parent at fork() time, while
temp-<childpid>.rdb is created slightly later by the child, so on
slow/loaded machines the file may not exist yet, making the test flaky.
Replace the fixed sleep + single existence assert with a
wait_for_condition poll. Also harden the post-shutdown deletion check
the same way, since bg_unlink removes the file from a background bio
thread and may not complete immediately.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
copyDbIdArgs assumed the DB token was always at argv[3], so
'COPY src dst REPLACE DB n' or 'COPY src dst DB a DB b' could
bypass db= restrictions. Mirror copyCommand's parser to walk
all argv tokens, collect every DB destination, and let the ACL
checker validate each one.
DB ACL was added in #2309.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Reject any SENTINEL SET argument containing control characters
(0x00-0x1F, 0x7F) to prevent config injection via newline-embedded
values.
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
CLUSTER FLUSHSLOT internally calls delKeysInSlot() which iterates
over all databases and removes the slot's keys from every database,
just like FLUSHALL command will touch all the databases. And it was
missing the ALL_DBS command flag introduced in #2309.
Add test to cover this case, also do a extra cleanup, replace the
manual start_multiple_servers with a single start_cluster call.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Enforce CMake policy CMP0023: Update CMake Linkage To Use `PRIVATE`
Dependencies
The policy for reference:
https://cmake.org/cmake/help/latest/policy/CMP0023.html
With this change, Valkey provides consistent CMake files that can be
safely integrated into other projects.
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
When unregistering a cluster message receiver (callback = NULL) for the
head node of the linked list, the code incorrectly updated
clusterReceivers[type]->next instead of clusterReceivers[type] itself.
This left the array entry pointing to freed memory, causing a
use-after-free on any subsequent list traversal.
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Removing logging key names when hide_user_data_from_log is true in rdb.c
file as well as networking.c, t_hash.c and debug.c files
Made simple if else in order to distinguish for read ability
---------
Signed-off-by: zackcam <zackcam@amazon.com>
On the current unstable branch, this issue does not occur when
`repl-diskless-load` is set to `disabled`, because we now use
a bio thread to save the RDB file, the main thread only triggers
the RDB loading process in `replicationCron`.
This test case is reproducible when `repl-diskless-load` is set
to `flush-before-load`, the replica resets the function context,
leading to a "use-after-free" error within a slow function.
Add test case for #3625 (CVE-2026-23631).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Until now, deep santitization of listpack on load (RDB load, RESTORE)
was configurable. That meant that we had to do validation on access and
traversal of the listpack. This change:
- Force sanitization of listpacks on load and import.
- Deprecates the config `sanitize-dump-payload` and makes it a no-op.
- Deprecates the ACL flags `skip-sanitize-payload` and
`sanitize-payload`, ignoring them when loading old ACL files.
- Removes validation asserts on access.
Benefits:
- Prevents deferred assertions, which would happen if unsanitized
listpacks were loaded and later accessed, leading to server
shutdown.
- Improves read performance.
Replaces: https://github.com/valkey-io/valkey/pull/399
Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com>
Previously, changing the cluster-require-full-coverage config via
CONFIG SET would not trigger an immediate cluster state update, requiring
the cluster to wait for the next clusterCron cycle to reflect the change.
Furthermore, there was a bug when flipping cluster-require-full-coverage
from "no" to "yes" while the cluster was in the OK state. Because
clusterCron only called clusterUpdateState() when:
```
if (update_state || server.cluster->state == CLUSTER_FAIL)
```
The cluster would remain stuck in the OK state despite having uncovered
slots, since neither condition was true (state was OK and update_state
was not set).
The fix schedules CLUSTER_TODO_UPDATE_STATE in beforeSleep whenever
the config changes, so the state is re-evaluated promptly in both
directions (FAIL -> OK and OK -> FAIL).
Add a test that verifies both transitions by pausing a primary with
cluster-require-full-coverage set to "no" (cluster stays OK), then
flipping it back to "yes" (cluster immediately transitions to FAIL).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Add a new `workflow_dispatch` triggered workflow
(`benchmark-on-demand.yml`) that enables admin-triggered, parameterized
benchmarking on any open PR.
Implements: https://github.com/valkey-io/valkey-perf-benchmark/issues/52
### Problem
- Current automated benchmarks (via `run-benchmark` /
`run-cluster-benchmark` labels) only run a fixed SET/GET workload
- PRs targeting other command types cannot be benchmarked automatically
- No way to customize benchmark parameters per PR
### Solution
A new workflow with configurable inputs:
| Input | Description | Default |
|---|---|---|
| `pr_number` | PR number to benchmark (required) | — |
| `commands` | Comma-separated commands (e.g. `SET,GET,HSET,LPUSH`) |
`SET,GET` |
| `data_size` | Comma-separated data sizes in bytes (e.g. `16,96`) |
`16,96` (from config) |
| `io_threads` | Comma-separated io-threads values (e.g. `1,9`) | `1,9`
(from config) |
| `pipelines` | Comma-separated pipeline values (e.g. `1,10`) | `1,10`
(from config) |
| `cluster_mode` | Enable cluster mode | `false` |
Other defaults from config: `duration=180`, `warmup=30`, `clients=1600`,
`benchmark-threads=90`, `keyspacelen=3000000`, `runs=3`.
When `io_threads` is overridden, `server_cpu_range` is automatically
adjusted to match.
Multiple workflow runs can be queued for the same PR with different
configs (no concurrency group).
### How it works
1. `resolve-pr` job fetches the PR merge commit SHA, head SHA, and base
ref.
2. `benchmark` job builds latest `valkey-benchmark` from unstable,
overrides the ARM benchmark config with user-provided parameters, and
runs `benchmark.py` comparing the PR commit against its base branch.
3. Results are posted as a PR comment with commit link, commands tested,
and workflow run link.
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Closes#3220
The Valkey Discord server has been closed. This changes the entry from
the GitHub issue template and the contributing guide.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Document how to install clang-format-18 (via pip or apt) and how to run
it locally to format code before submitting a pull request. This helps
new contributors avoid CI failures from the clang-format check.
Fixes#649
---------
Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com>
Generate stack traces with filenames and absolute line numbers.
Examples, with exceptions inserted in the test suite:
```
% ./runtest --single tests/unit/auth.tcl
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 48510
Testing unit/auth
[ok]: AUTH fails if there is no password configured server side (0 ms)
[exception]: Executing test client: invalid command name "dummy_undefined_function".
in dummy_undefined_function at tests/unit/auth.tcl:6
in start_server at tests/support/server.tcl:766
in execute_test_file at tests/unit/auth.tcl:1
in execute_test_file at tests/test_helper.tcl:146
in test_client_main at tests/test_helper.tcl:645
in test_client_main at tests/test_helper.tcl:1062
in catch at tests/test_helper.tcl:1062
in if at tests/test_helper.tcl:1061
% ./runtest --single tests/unit/auth.tcl
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 52538
Testing unit/auth
[exception]: Executing test client: Bad protocol, 'N' as reply type byte.
in valkey::__dispatch__ at tests/support/valkey.tcl:116
in $fd at tests/unit/auth.tcl:4
in test at tests/support/test.tcl:262
in test at tests/unit/auth.tcl:2
in start_server at tests/support/server.tcl:766
in execute_test_file at tests/unit/auth.tcl:1
in execute_test_file at tests/test_helper.tcl:146
in test_client_main at tests/test_helper.tcl:645
in test_client_main at tests/test_helper.tcl:1062
in catch at tests/test_helper.tcl:1062
in if at tests/test_helper.tcl:1061
```
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
When a replica has cluster-replica-no-failover enabled and the operator
later disables it via CONFIG SET while the primary is failing, the replica
previously had to wait for the next clusterCron (100ms) tick before it
could start an auto election.
Set CLUSTER_TODO_HANDLE_FAILOVER in clusterUpdateMyselfFlags() when the
NOFAILOVER flag transitions from set to cleared, so the failover handler
runs in the next beforeSleep instead.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Tests sometimes fail here. If we add `after 1000` inside the foreach
loop, it becomes very easy to reproduce.
```
test "Use takeover to bring slaves back" {
foreach id $replica_ids {
R $id cluster failover takeover
}
}
[exception]: Executing test client: ERR You should send CLUSTER FAILOVER to a replica.
```
The reason is that the transition from the PFAIL state to the FAIL
state which requires a quorum, may take a little extra time. From a
node's own perspective, it detects PFAIL and consequently views the
cluster as being in a FAIL state; and then, once the replica-no-failover
setting is disabled, auto failover may kick in, thereby causing the
test to fail.
Since we are verifying manual failover takeover, it is best to keep
`cluster-replica-no-failover` disabled at all times.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The numreplicas parameter in WAIT and WAITAOF commands is parsed as
`long` but stored in `blockingState.numreplicas` which is `int`. When a
value larger than INT_MAX (e.g., 2147483648) is passed, the implicit
narrowing conversion from `long` to `int` causes overflow, resulting in
a negative value that breaks the blocking logic.
Example: If `wait 2147483648 0` the client enters a blocked state, it
will unblock immediately after receiving ACKs from the replicas since
`numreplicas` is a negative value, we consider the `numreplicas`
requirement to be satisfied.
It's not a major issue, but it is indeed a bug.
Additionally, the WAIT command used `getLongFromObjectOrReply()` which
accepted negative values for numreplicas. A negative number should be
an invalid value. And in WAITAOF command we will reject negative number.
It is not a major issue, it is an inconsistency in error handling between
WAIT and WAITAOF.
Fix both issues by using `getRangeLongFromObjectOrReply()` with range
[0, INT_MAX] for numreplicas validation in both WAIT and WAITAOF. This
ensures:
- Negative values are rejected with an error.
- Values exceeding INT_MAX are rejected, preventing integer overflow.
- Both commands now have consistent validation for numreplicas.
This could be a **potentially breaking change**. In the past, we allowed:
- WAIT allows negative numreplicas values.
- WAIT and WAITAOF allows numreplicas to exceed INT_MAX.
Add tests for numreplicas boundary validation in both commands, and
add missing parameter validation tests for WAITAOF (numlocal range,
numreplicas range, and timeout range).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Align with `zmadvise_dontneed()`: gate the helper also on
`USE_JEMALLOC` so it becomes a no-op outside jemalloc builds.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The db=, alldbs and resetdbs rules introduced in #2309 were
missing from the ACL self-documenting block in valkey.conf.
Also update the `reset` description to include alldbs and
sanitize-payload, and add ERANGE to the documented errno values.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
In #2309, we ultimately decided not to set `get_dbid_args` for
MIGRATE command and removed the relevant code. But, we missed this
one, just cleanup the dead code.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The safe iterator contract claimed that inserting entries during
iteration was supported. However, inserting into a full bucket triggers
bucketConvertToChained which moves an entry the iterator may have
already returned, causing it to be visited twice. (Closes#2302, and
will resolve a flaky UT failure)
No production code inserts during safe iteration — all safe iterator
usages only read, delete, or gather stats.
Changes:
- Add assert in insert() to catch inserts during safe iteration.
- Update the safe iterator contract comment to document that insert is
not supported.
- Rewrite the safe_iterator test to only test delete-during-iteration
(the supported contract).
---------
Signed-off-by: Rain Valentine <rainval@amazon.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fix#3781
## Description
This PR fixes a crash on 32-bit Alpine 3.23 / time64 environments when
starting `valkey-server` and generating the `INFO server` output.
On 32-bit Alpine 3.23, `time_t` is 64-bit. In `genValkeyInfoString()`,
the `lru_clock` field is formatted with `%u`, but the expression passed
to `sdscatfmt()` was derived from `server.unixtime`, whose type is
`time_t`. This can pass a 64-bit varargs argument to a formatter
expecting an unsigned int-sized value. As a result, later varargs
arguments may be read at the wrong offset, which can lead to a
segmentation fault during startup.
The fix explicitly casts the computed `lru_clock` value to `unsigned
int` before passing it to `sdscatfmt()`, matching the `%u` format
specifier and preserving the intended 24-bit LRU clo
The Alpine 32-bit build fails with `-Werror=format` because a few
`time_t` values are printed with `%ld`. This assumes that `time_t` is
always compatible with `long`, which is not portable across all 32-bit
and 64-bit targets. In particular, 32-bit Linux environments may use a
`time_t` representation that does not match `long`, causing format
checking failures when warnings are treated as errors.
## Root Cause
The crash is caused by a varargs width mismatch on 32-bit time64
systems:
```c
"lru_clock:%u\r\n",
(server.unixtime / LRU_CLOCK_RESOLUTION) & LRU_CLOCK_MAX,
```
On 32-bit Alpine 3.23, server.unixtime is backed by 64-bit time_t, so
the expression may be passed as a 64-bit value. Since %u expects an
unsigned int, sdscatfmt() consumes only part of the argument and
subsequent arguments can become misaligned.
# Fix
Cast the value passed for %u to unsigned int:
server.c
```c
(unsigned int)((server.unixtime / LRU_CLOCK_RESOLUTION) & LRU_CLOCK_MAX)
```
updates the affected `time_t` formatting sites to use `%jd` together
with an explicit `(intmax_t)` cast. This keeps the output portable
across architectures without changing the `time_t` ABI or making
assumptions about its underlying typedef.
# Test
This PR also added a new daily CI "test-alpine-32bit"
Signed-off-by: chenshi <chenshi5012@163.com>
I found this issue in Redis repo
https://github.com/redis/redis/issues/14313 and confirmed that it also
exists in Valkey. The description is pretty lengthy and detailed, and
the TL;DR is that **during failover sentinels will pick whatever slave
instance it discovers to promote, which can cause serious data leakage
if that particular slave server actually contains data of another
customer/tenant**.
I spent a couple weeks on this issue and got it working as expected. To
make it easier for code reading, I broke up the code changes into
smaller patches for review. This is the first part.
(Note that I'm using term master/slave and I'm aware that it's the same
as primary/replica)
## How to reproduce the issue
We need to run two clusters and have two sentinel instances monitoring
each one.
Duplicate the file `sentinel.conf` as `sentinel-a.conf` and
`sentinel-b.conf`. Add `sentinel monitor cluster-a 127.0.0.1 6379 1` to
`sentinel-a.conf`, and `sentinel monitor cluster-b 127.0.0.1 6380 1` to
`sentinel-a.conf`. Then replace all occurrence of "mymaster" to
"cluster-a"/"cluster-b" correspondingly.
At the root directory of the repo:
```bash
# Set up the valkey server clusters
./src/valkey-server --port 6379 # this is cluster A
./src/valkey-server --port 6380 # this is cluster B
./src/valkey-server --port 6381 --replicaof 127.0.0.1 6379 # slave in cluster A
# Set up the sentinel instances
./src/valkey-server sentinel-a.conf --sentinel --port 26379
./src/valkey-server sentinel-b.conf --sentinel --port 26380
# Insert data
./src/valkey-cli -p 6379 set "key-cluster-a" "value-a"
./src/valkey-cli -p 6380 set "key-cluster-b" "value-b"
# Verify that the slave has replicated the data
./src/valkey-cli -p 6381 get "key-cluster-a"
# Manually ask the slave to follow a diffrent master
./src/valkey-cli -p 6381 replicaof 127.0.0.1 6380
```
Now shut down the master server (port 6379) in cluster A and we will see
sentinel A starting failover trying to promote a candidate slave. Since
the only slave 6381now belongs to a diffrent cluster (cluster B), the
failover should fail with an event `-failover-abort-no-good-slave`, but
the sentinel A still thinks slave 6381 is following master A 6379 and
thus promotes 6381 to become the new master in cluster A.
## Changes I made
The challenging part of solving this issue is to set up tests. The
entire sentinel test suite is designed to have only one master-slave
cluster with the master named "mymaster", but my code changes would
require spinning up two clusters so I had to refactor the
`tests/sentinel/tests/includes/init-tests.tcl` file to support so.
I also added a simple test in file `04-replica-selection.tcl` to verify
that the second cluster is now working. This file will be where I
implement the tests on sentinels verifying replication ID during
failover once this PR is merged. Interestingly, this file was intended
to test the slave selection procedure but the test was never implemented
since 2014.
Signed-off-by: Zhijun <dszhijun@gmail.com>
1. Fix double-call to finishSlotMigrationJob in
clusterUpdateSlotImportsOnOwnershipChange(): when n == myself, the
function was called once with 'assigned to myself' message, then fell
through to call it again with 'no longer owned by source'. Added else
branch to make the two error paths mutually exclusive.
2. Fix RESP protocol violation in clusterCommandSyncSlotsFinish():
addReply(c, shared.ok) was sent unconditionally before validating the
job name and state, so on error the client would receive both +OK and
-ERR. Moved the OK reply after all validations pass.
Note that it is an internal command and the only clients sending it are
primary to replica connections and fake AOF clients. Both of those turn
off replies, so the reply doesn't actually get parsed and therefore the
violation has no effect. But good to clean it up
Signed-off-by: chx9 <lovelypiska@outlook.com>
### Bug
When `ACL LOAD` deletes a user, it calls `freeClientOrCloseLater()` on
clients authenticated as that user. If `freeClient()` defers the actual
free (because the client has `flag.protected` set or has pending IO),
the client remains in `server.clients` with `c->user` still pointing to
the old user object.
After `raxFreeWithCallback(old_users, ACLFreeUserVoid)` frees all old
user objects, `c->user` becomes a dangling pointer. Any subsequent code
that dereferences `c->user` on the deferred client triggers a
heap-use-after-free.
### Crash sequence
1. Client A runs `ACL LOAD`
2. Client B is authenticated as a user that was removed from the ACL
file
3. Client B has `flag.protected` set (e.g. has pending IO (IO threads)
4. `ACLLoadFromFile` → `freeClientOrCloseLater(B, 0)` → `freeClient(B)`
→ defers to `freeClientAsync` because B is protected
5. `raxFreeWithCallback(old_users, ...)` frees all old user objects
including B's user
6. B is still in `server.clients` with `c->user` pointing to freed
memory
7. `CLIENT LIST` or any path that calls `catClientInfoString`
dereferences `c->user->name` → **UAF**
### Fix
Set `c->user` to default user before calling `freeClientOrCloseLater()`
in `ACLLoadFromFile`. The existing NULL guard in `catClientInfoString`
(`client->user ? client->user->name : "(superuser)"`) handles this
safely.
### Test
Added `DEBUG PROTECT-CLIENT <client-id>` subcommand that calls
`protectClient()` on the target, forcing `freeClient` to defer to async.
This enables deterministic reproduction without timing races.
The test:
1. Creates a user, connects a client as that user
2. Protects the client via `DEBUG PROTECT-CLIENT`
3. Removes the user from the ACL file and runs `ACL LOAD`
4. Runs `CLIENT LIST` which dereferences `c->user->name`
Without the fix, this crashes under ASAN with:
```
ERROR: AddressSanitizer: heap-use-after-free in catClientInfoString
READ of size 8
freed by: raxRecursiveFree → raxFreeWithCallback → ACLLoadFromFile
```
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Resolution strategy: Hanzo (kv) is on the upstream 'unstable' branch
which has more features than the 9.0.4-lts release. For all 114 conflict
files, take HEAD verbatim to preserve Hanzo's feature set and rebrand.
CVE security fixes from 9.0.4 will be cherry-picked separately:
- CVE-2026-23479 (UAF in unblockClientOnKey)
- CVE-2026-25243 (RESTORE malformed zipmap)
- CVE-2026-23631 (Lua script full sync UAF)
Required for hanzoai/.github/.github/workflows/docker-build.yml@main —
without it the workflow_call dies as startup_failure with no jobs
dispatched. Caller permissions are a CEILING.
Replace bespoke 59-line docker build with the 10-line canonical caller
that uses hanzoai/.github/.github/workflows/docker-build.yml@main.
Native amd64+arm64 ARC runners, semver + branch tags, multi-arch manifest
via the one shared workflow path. Cache-key preserved for scope=kv.
Co-authored-by: Hanzo AI <dev@hanzo.ai>
The HFE-blocking-keyspace-notify test added in #3743 is racy and
intermittently fails with:
```
Expected '{event del key h1} {event hdel key h1}' to be equal to '{event hdel key h1}'
(context: ... cmd {assert_equal [lsort {{event hdel key h1} {event del key h1}}] [lsort [r b_keyspace.events]]} proc ::test)
```
## Why it races
When `HGETDEL` empties the hash, `hgetdelCommand` fires two
notifications back-to-back on the main thread:
1. `notifyKeyspaceEvent(NOTIFY_HASH, "hdel", ...)`
2. `notifyKeyspaceEvent(NOTIFY_GENERIC, "del", ...)` (the key was
removed)
The `block_keyspace_notification` test module's callback always spawns a
background thread that sleeps 1s, appends to the event log, and
optionally unblocks the client. For the second notification,
`RM_BlockClient` returns `NULL` (the client is already blocked by the
first), so the second thread never unblocks anyone — it only appends
`"del"` to the log.
The first thread is the one that unblocks the client, so `HGETDEL`
returns as soon as that thread has logged `"hdel"`. The test client then
immediately reads `b_keyspace.events`, racing the second thread which
has not yet acquired the event-log mutex. On a loaded host or under
valgrind the second thread routinely loses the race, leaving only
`"hdel"` visible.
## Fix
`wait_for_condition` until both events have been logged before asserting
on their contents. This matches the pattern already used by the four
other event-log assertions in this file (e.g. the `Server-created
keyspace notification` and `Event that fires twice` blocks).
```tcl
wait_for_condition 50 100 {
[llength [r b_keyspace.events]] >= 2
} else {
fail "Did not see both hdel and del events: [r b_keyspace.events]"
}
assert_equal [lsort {{event hdel key h1} {event del key h1}}] [lsort [r b_keyspace.events]]
```
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Update the data-size to match the automated perf benchmarking as 96 is
also embedded so moving to 128 to test for non-embedded.
I have also update the wfs to use the config files from the
`valkey-perf-benchmark` so we don't need to track the configs in 2
places, everything will be pulled from source `valkey-perf-benchmark`
and the configs will be standard throughout all runs
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Closes#3663.
## Why
Three callers in `src/modules/lua/script_lua.c` lost error-code handling
during the #2858 move of the Lua scripting engine into a module. On the
wire, `redis.pcall()` (no args), `redis.setresp(4)`, `pcall(redis.log,
1)`, `pcall(redis.log, 10, 'msg')`, and `pcall(redis.set_repl)` now
return the bare message instead of `ERR <message>`. OOM and WRONGTYPE
happened to keep working because their code letters survived elsewhere
by accident.
Reproducer:
```
$ ./src/valkey-cli eval "return redis.pcall()" 0
"Please specify at least one argument for this redis lib call" # before this PR
"ERR Please specify at least one argument for this redis lib call" # with this PR
```
## What changed
In `src/modules/lua/script_lua.c`:
1. `luaPushErrorBuff` — bare-message branch (case 2): prepend `ERR `
unless the message already starts with a code letter (defensive — five
existing callers hardcode `"ERR ..."` strings; double-prefixing them
would also be wrong).
2. `luaProcessReplyError` push_error branch: re-add the leading `-`
before delegating to `luaPushError` so the case-1 code-extraction path
handles `OOM`, `WRONGTYPE`, `READONLY`, etc.
3. `errorCallback` errno==0 branch: same re-add-`-` approach.
41 LoC across the three functions.
## Test
`tests/unit/scripting.tcl` regression at line 1028 covers:
- All five direct-API errors above — asserts `ERR ` prefix
- WRONGTYPE error path — asserts the code survives and is not
double-prefixed
- Runs 4 times under the `foreach is_eval × foreach
script_compatibility_api` matrix
Fails on `unstable`, passes with this PR.
---------
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
When a module subscribes to NOTIFY_HASH keyspace events and blocks the
client in the notification callback, hexpireGenericCommand,
hgetdelCommand,
and hpersistCommand would trigger debugServerAssert in
notifyKeyspaceEvent()
because addReplyArrayLen() sets buffered_reply=1 before the notification
fires. This debug assert was added in #1819.
Affected commands: HEXPIRE, HPEXPIRE, HEXPIREAT, HPEXPIREAT, HGETDEL,
HPERSIST.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
When a replica receives a keyless command from a client that has
negotiated `CAPA REDIRECT` and has not sent `READONLY`, redirect it to
the primary with a `-REDIRECT` response.
Previously, keyless commands like `SCAN, DBSIZE, RANDOMKEY` always
executed locally on replicas, and keyless writes like FLUSHDB, FLUSHALL
returned -READONLY error with no way for clients to discover the
primary.
This reuses the existing `CAPA REDIRECT` capability. The capa signals
that the client can handle -REDIRECT responses — it does not prescribe
when redirects are sent. Clients that send READONLY opt into local
replica execution and are not redirected.
Only commands with CMD_READONLY or CMD_WRITE are redirected, so
connection/admin commands (PING, READONLY, CLIENT) remain unaffected.
EXEC with all-keyless queued commands (c->slot == -1) is treated as
keyless and the transaction is discarded.
---------
Signed-off-by: Yana Molodetsky <yamolodu@amazon.com>
Signed-off-by: Yana Molodetsky <59420437+yanamolo@users.noreply.github.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
The hash-seed config is an sds string and may contain embedded NUL
bytes (sdssplitargs preserves \xNN escapes inside double quotes).
In the old code getHashSeedFromString() used strlen() on it, so two
hash-seed values differing only after a \x00 byte collapsed to the
same SipHash seed, can break it.
hash-seed was added in #2608.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
release.c is compiled into valkey-cli, valkey-benchmark, and the
valkeylib-gtest static library, but only valkey-server declared a
dependency on the release_header custom target that runs mkreleasehdr.sh
to generate src/release.h. On a clean build (no stale release.h on disk)
or under high parallelism, those targets could attempt to compile
release.c before the header was generated and fail with "release.h: No
such file or directory".
Add the missing add_dependencies(... release_header) for all three.
Signed-off-by: Vadym Khoptynets <1099644+poiuj@users.noreply.github.com>
## Summary
This PR addresses 2 race conditions where deferred `freeClient`
(introduced by #3324) clobbers replication state set by `REPLICAOF`
commands.
Found while triaging the recurring `test-ubuntu-tls-io-threads` daily CI
failure in `tests/unit/wait.tcl`.
## Root Cause
Since PR #3324 ("Redesign IO threading communication model"),
`freeClient()` on a primary client with pending IO is deferred via
`freeClientAsync` (gated on `clientHasPendingIO`). When the deferred
free eventually executes, it chains through `replicationCachePrimary()`
-> `replicationHandlePrimaryDisconnection()`, which unconditionally sets
`server.repl_state = REPL_STATE_CONNECT`.
This causes two bugs:
### Bug 1: REPLICAOF NO ONE (SIGSEGV)
`replicationUnsetPrimary()` sets `primary_host = NULL` before calling
`freeClient`. The deferred free runs later, sets `repl_state =
REPL_STATE_CONNECT` while `primary_host` is still NULL.
`replicationCron` then calls `connectWithPrimary()` which passes NULL to
`connTLSConnect()` -> `inet_pton(AF_INET, NULL, ...)` -> SIGSEGV.
### Bug 2: REPLICAOF newhost newport (connection leak)
`replicationSetPrimary()` calls `freeClient(old_primary)` (deferred),
then sets `primary_host` to the new IP and progresses `repl_state` to
`REPL_STATE_CONNECTING` with a new connection handle in
`server.repl_transfer_s`. The deferred free runs later, clobbers
`repl_state` back to `REPL_STATE_CONNECT`. `replicationCron` then calls
`connectWithPrimary()` again, overwriting `server.repl_transfer_s`
without closing the previous connection -- an FD leak.
## Fix
Make `replicationHandlePrimaryDisconnection()` only transition to
`REPL_STATE_CONNECT` when `repl_state` is still `REPL_STATE_CONNECTED`
and `primary_host` is set. This means the disconnection is genuine and
no other state transition has already occurred. If `repl_state` has
already moved on (CONNECT, CONNECTING, NONE, etc.), the deferred free is
stale and the function leaves the state untouched.
Additionally:
- `connTLSConnect()`: Return `C_ERR` if `addr` is NULL (defense in
depth).
- `tests/unit/wait.tcl`: Add 10s timeout to the blocking WAITAOF test,
and add dedicated tests for the repoint scenario.
## Reproduction
Reproduced locally by establishing TLS replication and executing
`REPLICAOF NO ONE`:
- Without fix: server crashes with signal 11, accessing address 0x0
- With fix: server continues operating normally
## Testing
- Full `unit/wait` test suite passes (51/51) with IO threads enabled
- New tests "Repoint replica between primaries does not leak connections
or crash" and "Rapid repoint does not crash or leak" pass
- Crash reproduced locally over TLS (SIGSEGV without fix, graceful
handling with fix)
---------
Signed-off-by: Yaron Sananes <yaron.sananes@gmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
This test will insert keys, and as can be seen from the logs, the insertion
somehow is very slow in this CI, eventually causing a repl-timeout disconnection.
```
50655:M 14 May 2026 00:44:57.812 - DB 0: 5 keys (0 volatile) in 7 slots HT.
50655:M 14 May 2026 00:45:02.884 - DB 0: 6 keys (0 volatile) in 7 slots HT.
50655:M 14 May 2026 00:45:06.014 # Timing out slot migration xxx after not receiving ack for too long
```
Set repl-timeout for these tests to prevent the timeout disconnections.
Also the tests does not actually require inserting distinct keys, we only
need to fill the replication buffer, so there is no need for the different
keyname, this can save the CI some memory.
Closes#3702.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Squashed 'deps/libvalkey/' changes from 45c2ed15c..cb5ff91aa
cb5ff91aa Release 0.5.0
cd3d5d8ff Fix potential heap-buffer-overflow in cluster error reply parsing (#305)
32c7c5d09 Refactor error handling to use valkeySetErrorFromErrno and valkeyClearError (#303)
e4b548b32 Remove support for external dict while still supporting external sds (#302)
61b27c453 Try all addresses from DNS before failing to connect (#300)
8aff94fab RDMA: Fix lost EPOLLIN/POLLIN events
b9285b5fc Optimize read buffer compaction and reduce copying (#294)
0fa809877 Additional overflow protection for MAP/ATTR
b26c56e87 Fix gcc warnings (#287)
git-subtree-dir: deps/libvalkey
git-subtree-split: cb5ff91aa5816807f0a549bd2b36e611253dfeb6
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
Currently, the `CLUSTER SETSLOT MIGRATING/IMPORTING` commands
only check the ownership of the slot. So if the user mistyped node IDs while
writing these commands, it will be pretty confusing to debug, since the
`valkey-cli --cluster check` command will still report the correct status.
This PR adds a simple check to verify if the node ID in the command
points to itself, reducing confusion when doing manual hash slot
migration.
For IMPORTING, myself is the target node, so the "node" must be the
source node (not myself).
For MIGRATING, myself is the source node, so the "node" must be the
target node (not myself).
Signed-off-by: hieu2102 <hieund2102@gmail.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Rewrite the CLUSTERDOWN test to use a 3-node cluster and cover four
distinct degraded-cluster scenarios:
1. Node down with full-coverage enabled -> CLUSTERDOWN on all slots
2. Node down with full-coverage disabled -> MOVED for unreachable
slots, local slots handled normally
3. Slot unassigned with full-coverage enabled -> "Hash slot not served"
for the unassigned slot, CLUSTERDOWN for others
4. Slot unassigned with full-coverage disabled -> "Hash slot not served"
for the unassigned slot, MOVED for remote slots
Signed-off-by: Binbin <binloveplay1314@qq.com>
This should be bzmpopGetKeys, however, it is harmless because
blmpopGetKeys and bzmpopGetKeys are implemented exactly the same.
This typo was introduced in the very beginning.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The weekly workflow has been broken since May 10.
#3358 added a `consolidate-test-failures` job to `daily.yml` that
needs `actions: write` to delete per-job artifacts. `weekly.yml` calls
`daily.yml` as a reusable workflow but only grants `actions: read`
Verified on my fork:
`determine-release-branches` ran, the nested `daily.yml` matrix
expanded, and the child jobs were started. Cancelled after that.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
This bug was introduced in #3366.
Before PR #3366, hash-seed config was applied directly via
hashtableSetHashFunctionSeed(), so clusterscanFingerprint() correctly
used hash_function_seed to derive the fingerprint.
```c
if (server.hash_seed != NULL) {
memset(hashseed, 0, sizeof(hashseed));
getHashSeedFromString(hashseed, sizeof(hashseed), server.hash_seed);
hashtableSetHashFunctionSeed(hashseed);
}
```
PR #3366 introduced a separate configurable_hash_seed for data
hashtables and kept hash_function_seed as a random per-process value.
```c
/* Set the configured hash seed used by data hashtables (keys, sets, zsets,
* hashes) or use the random seed if not configured. */
if (server.hash_seed) {
uint8_t seed[16] = {0};
getHashSeedFromString(seed, sizeof(seed), server.hash_seed);
setConfigurableHashSeed(seed);
} else {
setConfigurableHashSeed(hashtableGetHashFunctionSeed());
}
```
However, clusterscanFingerprint() was not updated accordingly — it
still reads hash_function_seed, which is now random on every node.
This makes fingerprints differ across nodes even when they share the
same hash-seed config, causing cursors to restart on failover.
CLUSTERSCAN was introduced in #2934.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Follow-ups to the IO-threads redesign (#3324 / #3367). Bundles several
independent fixes and refactors:
- tryOffloadFreeObjToIOThreads: only offload the free of the sds buffer
(which the I/O thread allocated). The robj itself is main-thread-owned
and is now freed on the main thread.
- trySendWriteToIOThreads: defer clearing last_header until after a
successful enqueue, so the main thread can't mutate a header the I/O
thread is concurrently reading.
- evictClients: drop the pending_freed / close_asap bookkeeping and rely
on freeClient's return value to bump stat_evictedclients. The old
accounting double-counted protected clients and could under-evict.
- Make MPSC/SPMC/SPSC queue sizes a runtime parameter on *Init(q, size)
instead of compile-time *_QUEUE_SIZE macros in queues.h. The I/O-threads
sizes (16384 / 4096 / 4096) now live as constants in io_threads.c; tests
pass their own sizes.
- updateIOThreads uses io_shared_outbox.queue_size instead of the macro.
- Add function-level doc comments to queues.h.
- Rename IOThreadFreeArgv/IOThreadPoll → ioThreadFreeArgv/ioThreadPoll
for consistency.
- Remove unused job_handler typedef; fix static_assert message (7 → 8).
clang-format pass; Makefile tab→spaces nit.
- Replace the getrusage(RUSAGE_THREAD) sys/user CPU sampling (and its
#ifdef RUSAGE_THREAD fallback) with server.stat_active_time. One
portable signal, one threshold (IO_IGNITION_MAIN_THREAD_ACTIVE_PERCENT =
30) replaces the dual sys>30% / sys>5%+user>50% rules.
- Renames STATS_METRIC_MAIN_THREAD_CPU_{SYS,USER} →
STATS_METRIC_MAIN_THREAD_ACTIVE_TIME.
---------
Signed-off-by: akash kumar <akumdev@amazon.com>
Signed-off-by: Akash Kumar <45854686+akashkgit@users.noreply.github.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
This after causes the test to take 15 seconds, but it might only
require just over 10 seconds since the MIGRATE_SOCKET_CACHE_TTL
is 10 seconds.
```
/* Cleanup expired MIGRATE cached sockets. */
run_with_period(1000) {
migrateCloseTimedoutSockets();
}
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
Required for hanzoai/.github/.github/workflows/docker-build.yml@main —
without it the workflow_call dies as startup_failure with no jobs
dispatched. Caller permissions are a CEILING.
Replicas with replication offset 0 are flagged as "loading" in CLUSTER
SHARDS and are hidden in CLUSTER SLOTS. These commands assume that the
replicas have not yet synched from their primary, but the replication
offset is zero also when the primary hasn't had any commands written to
the replication stream yet.
The replication offset becomes non-zero when the primary sends a PING
on the replication stream, which it does every 10 seconds by default.
This eventually marks the replicas "online" in CLUSTER SHARDS / CLUSTER
SLOTS.
This change sends a PING on the replication stream when the first
replica connects, to trigger a non-zero replication offset and make the
replicas show up as "online" immediately.
The test framework's "start_cluster" waits for the all nodes to be
marked "online" before running test cases. Production deployment tools
may have similar checks. With this change, starting a cluster gets up to
10 seconds faster.
Some tests with large clusters need these extra 10 seconds for the
cluster to stabilize, so test framework's wait_for_cluster_propagation
is allowed 10 extra seconds (60 instead of 50).
**Additional test framework change:** Catch assertions and print the
duration of setting up the cluster in start_cluster.
This change catches asserts and timeouts in wait_for_condition during
start_cluster's setup phase, prints the error and skips the test cases
within the cluster_cluster body, instead of crashing the whole test
framework with a Tcl exception and stack trace. This is implemented
as a test fixture with a setup and teardown code, which is a new
construct added to the test framework.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
When a blocked client is woken up on a key, unblockClientOnKey()
re-executes its pending command via processCommandAndResetClient(). That
function returns C_ERR when the client was freed as a side effect of the
re-execution (it detects this by server.current_client becoming NULL
inside freeClient()).
The pre-fix code ignored the return value and unconditionally accessed
c->flag.blocked / c->flag.module afterwards -- a use-after-free on the
freed client.
The fix mirrors the existing dead-client handling in
processPendingCommandAndInputBuffer() (src/networking.c): on C_ERR,
unwind the execution unit, restore server.current_client, and return
without touching c.
Note on testing: reaching the C_ERR branch requires the reprocessed
command to synchronously free the current client (for example a
replica/primary-link teardown or a module path that calls freeClient()).
None of the blocking commands routed through unblockClientOnKey
(BLPOP/BLMOVE/BRPOPLPUSH/BLMPOP/ XREADGROUP and friends) take that path
from a plain RESP client, so a Tcl-level integration reproducer is not
feasible without adding a test-only debug hook; this patch does not add
such a hook. A targeted gtest reproducer was also considered but
rejected as net-negative: it would require ~200 lines of fake-client
scaffolding to exercise a 7-line defensive guard and would become a
"change detector" as warned against in src/unit/README.md. The existing
tests/unit/type/list.tcl suite (288 tests) exercises the happy path of
this function and continues to pass unchanged. Running the suite under
AddressSanitizer (make SANITIZER=address) provides the strongest signal
against regressions of this class of bug.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
Continuation of #3315 (accidentally closed)
Part of #2670
## Summary
Automatically detect test failures from daily CI runs and create/update
GitHub issues.
## What it does
- After each daily CI run, detects test failures from all test
environments
- Creates a new GitHub issue if the failure is not already reported
- Comments on existing issues if the failure is already reported
- Local usage: Developers can generate a JSON report of test failures
locally by passing --failures-output:
example:
```./runtest --single unit/auth --failures-output results.json --verbose```
Without the flag, no file is created.
## Changes
- `tests/test_helper.tcl` — add `--failures-output` flag to write valkey/moduleapi failures to a specified JSON file, filter TIMEOUT/Sanitizer/Valgrind/Can't start/check for memory leaks
- `tests/instances.tcl` — add failure tracking and `--failures-output` support for sentinel/cluster tests
- `.github/workflows/daily.yml` — pass `--failures-output` to all test commands, one artifact upload per job, consolidation job to merge all artifacts
- `.github/workflows/test-failure-detector.yml` — new workflow triggered on Daily completion to create/update GitHub issues
- `.github/actions/upload-test-failures/action.yml` — reusable composite action for uploading test failure artifacts
## Testing
Ran multiple daily workflow dispatches with dummy tests and verified:
- Failure JSON files created correctly for valkey, moduleapi, sentinel, cluster
- Artifacts uploaded and consolidated into single report
- Issues created and commented on for repeated failures:
- - (valkey)https://github.com/hanxizh9910/valkey/issues/158
- - (moduleapi)https://github.com/hanxizh9910/valkey/issues/76
- - (cluster)https://github.com/hanxizh9910/valkey/issues/157
- - (sentinel)https://github.com/hanxizh9910/valkey/issues/156
Note: Previous test issues have been closed. Here's what it looks like when failures are detected (the sentinel and cluster dummy test failures are intentional):
<img width="1559" height="515" alt="Multiple test failure issues created automatically" src="https://github.com/user-attachments/assets/ce4e1ffa-83f2-44dd-a6e2-13b07f0507a5" />
- Example of running daily: https://github.com/hanxizh9910/valkey/actions/runs/23165826266
Result: https://github.com/hanxizh9910/valkey/issues:
<img width="1447" height="349" alt="Screenshot 2026-03-17 at 11 45 36 AM" src="https://github.com/user-attachments/assets/f8b18fb8-5541-4421-b30e-f14e16e82ce7" />
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Introduced in test here #3578
The `deferred_reply` module regression test can emit two top-level RESP
replies for a single client command: the normal `SET` reply followed by
the module callback reply.
The req/res log validator assumes each logged argv is followed by one
response.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
CI caught ip and name SDS allocations being leaked in
fetchClusterConfiguration. The ip SDS was copied again via sdsnew()
before being passed to createClusterNode(), leaking the original. The
name SDS was leaked when the node already existed in the dict.
Free ip and name on all exit paths in fetchClusterConfiguration. Remove
stale guard in freeClusterNode, no longer needed since #1392
CI Error -
```
Direct leak of 33 byte(s) in 3 object(s) allocated from:
#0 0x7f4c3a0fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x5564620c124a in ztrymalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:172
#2 0x5564620c124a in zmalloc_usable /home/runner/work/valkey/valkey/src/zmalloc.c:268
#3 0x5564620dfbe6 in _sdsnewlen.constprop.0 /home/runner/work/valkey/valkey/src/sds.c:102
#4 0x556462050996 in sdsnewlen /home/runner/work/valkey/valkey/src/sds.c:169
#5 0x556462050996 in sdsnew /home/runner/work/valkey/valkey/src/sds.c:185
#6 0x556462050996 in fetchClusterConfiguration /home/runner/work/valkey/valkey/src/valkey-benchmark.c:1477
```
Issue was reproduceable locally using `leaks --atExit`
Signed-off-by: nmvk <r@nmvk.com>
Instead of scanning one slot at a time `CLUSTERSCAN` now scans the
entire contiguous range of slots owned by the current node.
Implementation details
* Re-sharding safe as the hash slot is updated based on the local cursor
position.
* Fingerprint remains stable across the entire contiguous slot range
instead of being reset per slot.
* Parsing/validation of parameters for the SCAN commands is refactored
and moved to a separate function.
```
> CLUSTERSCAN 0
"0-{06S}-0" # start at slot 0
> CLUSTERSCAN 0-{06S}-0
"aBcDeF-{06S}-48" # scanning slot 0...
> CLUSTERSCAN aBcDeF-{06S}-48
"aBcDeF-{1Y7}-16" # slot 0 done, continues to slot 6 (same node hence FP is unchanged)
> CLUSTERSCAN aBcDeF-{1Y7}-16
"aBcDeF-{0or}-32" # slot 6 done, continues to slot 100 (same node hence FP is unchanged)
...
> CLUSTERSCAN aBcDeF-{...}-64
"0-{8YG}-0" # Current continuous slot boundary reached hence cross-node transition
```
Follow-up of #2934
---------
Signed-off-by: nmvk <r@nmvk.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fixes server crash when RDMA benchmark clients disconnect (part of
#3345).
This PR focuses on the server-side crash fix. The benchmark
client-side fix will be submitted in a separate PR.
## Server Crash on Client Disconnect
When interrupting valkey-benchmark with Ctrl+C, the server would crash
with a segmentation fault in handleReadResult, accessing address 0x8
(NULL pointer dereference).
### Root Cause
The same RDMA connection could be added to pending_list multiple times,
leading to use-after-free:
1. When a client disconnects, rdmaHandleDisconnect() adds the connection
to pending_list and sets conn->state = CONN_STATE_CLOSED
2. Before rdmaProcessPendingData() processes it, event-driven callbacks
(e.g., connRdmaSetWriteHandler) could add the same connection again to
pending_list
3. In rdmaProcessPendingData(), the first iteration processes and may
free the connection, but the second iteration accesses the already freed
connection → SIGSEGV at address 0x8 (NULL + offset)
The problem was exacerbated because:
- valkey-benchmark creates multiple concurrent connections
- On Ctrl+C, these connections disconnect nearly simultaneously
- Event callbacks can re-add connections before processing completes
**Stack trace from crash:**
handleReadResult+0x100
readQueryFromClient+0x63
rdmaProcessPendingData (0x1b1eb9)
beforeSleep+0x82
### Solution
Four targeted fixes in `src/rdma.c`:
1. `rdmaHandleDisconnect (line 535)`: Check if pending_list_node is NULL
before adding to pending_list to prevent duplicates
```C
/* we can't close connection now, let's mark this connection as closed
state */
if (rdma_conn->pending_list_node == NULL) {
listAddNodeTail(pending_list, conn);
rdma_conn->pending_list_node = listLast(pending_list);
}
```
2. `connRdmaSetWriteHandler (line 956)`: Add same check before adding to
pending_list to prevent duplicates during write handler updates
```C
/* does this connection has pending write data? */
if (func) {
if (rdma_conn->pending_list_node == NULL) {
listAddNodeTail(pending_list, conn);
rdma_conn->pending_list_node = listLast(pending_list);
}
} else if (rdma_conn->pending_list_node) {
listDelNode(pending_list, rdma_conn->pending_list_node);
rdma_conn->pending_list_node = NULL;
}
```
3. `rdmaProcessPendingData (line 1786)`: Use iterator node 'ln' instead
of rdma_conn->pending_list_node when deleting, as the latter may have
been overwritten if the connection was re-added
```C
listDelNode(pending_list, ln); // Use ln, not
rdma_conn->pending_list_node
rdma_conn->pending_list_node = NULL;
```
4. `rdmaProcessPendingData (line 1782-1787)`: Reorder operations to call
handlers before deleting from list, ensuring connection structure
remains valid during handler execution
```C
if (conn->state == CONN_STATE_ERROR || conn->state == CONN_STATE_CLOSED)
{
/* Invoke handlers first, then remove from list */
if (callHandler(conn, conn->read_handler)) {
callHandler(conn, conn->write_handler);
}
listDelNode(pending_list, ln);
rdma_conn->pending_list_node = NULL;
++processed;
continue;
}
```
The fix uses **pending_list_node** as both a list pointer and a
"**already in list**" flag:
- NULL = not in list, safe to add
- non-NULL = already in list, skip adding
This ensures each connection appears in pending_list at most once,
preventing use-after-free crashes.
Signed-off-by: Ada-Church-Closure <nunotabashinobu066@gmail.com>
Incrementally release memory to the OS using `madvice(MADV_DONTNEED)`
when rehashing. This reduces the latency of the `free()` call at the end
of the rehashing.
### Problem
Upon rehashing completion, `rehashingCompleted()` frees the old table
via `zfree()`. For large tables (e.g., tens of millions of keys), this
operation can take tens of milliseconds, causing noticeable latency
spikes in the server.
### Solution
After each bucket migration, if 16 pages worth of buckets have been
processed, we immediately release those pages to the OS via
`madvise(MADV_DONTNEED)`. This incremental approach ensures that by the
time `rehashingCompleted()` is called, most physical pages have already
been returned to the OS, making the final cleanup fast and predictable.
---------
Signed-off-by: chzhoo <czawyx@163.com>
Ensure deferred-length reply placeholders are created and resolved in
the same reply list that subsequent nested replies use when the deferred
reply buffer is active. This prevents malformed responses when module
callbacks build postponed-length arrays while a client is already
deferring output.
Add a regression test module and unit test that reproduce the issue
through keyspace notifications and verify the nested array reply is
serialized correctly. Register the new module with the module test
build.
Here is a small illustration of what happens with and without the fix:
## With the fix — placeholder goes to `c->deferred_reply`:
```
c->buf: (empty)
c->reply: (empty)
c->deferred_reply: [+OK\r\n] [*2\r\n] [+first\r\n] [*2\r\n] [+a\r\n] [+b\r\n]
^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SET reply reply callback (outer array, inner array via POSTPONED_LEN)
→ commitDeferredReplyBuffer joins into c->reply
→ Wire: +OK\r\n *2\r\n +first\r\n *2\r\n +a\r\n +b\r\n ✓
OK [ "first", ["a", "b"] ]
```
## Without the fix — placeholder goes to `c->reply`:
```
c->buf: (empty)
c->reply: [*2\r\n] ← placeholder filled by setDeferredArrayLen (WRONG LIST!)
c->deferred_reply: [+OK\r\n] [*2\r\n] [+first\r\n] [+a\r\n] [+b\r\n]
^^^^^^^^ outer array "first" "a" "b"
→ commitDeferredReplyBuffer appends deferred_reply AFTER reply
→ Wire: *2\r\n +OK\r\n *2\r\n +first\r\n +a\r\n +b\r\n ✗
[ OK, ["first", "a"] ] "b" ← orphaned
```
* networking
* tests/modules
* tests/unit/moduleapi
**Generated by CodeLite**
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Add createFileEvent(): for non-RDMA, it is just aeCreateFileEvent.
For RDMA, when registering AE_WRITABLE, register the event and call the
handler once (same as the old direct writeHandler kick).
Do not speculatively call readHandler after registering AE_READABLE
(my previous approach): that interacts badly with libvalkey’s
valkey-io/libvalkey#301 wakeup path and can stall the benchmark after
the first request.
Problem fixed:
`valkey-benchmark --rdma` could hang or stall (e.g. GET-heavy
workloads). Part of that was lost EPOLLIN when libvalkey drained the CQ
and processed inbound data on the write path: the outer poll never saw a
“new” edge. That belongs in libvalkey and is addressed by
valkey-io/libvalkey#301 (eventfd re-arm / nested epoll integration).
Separately, RDMA is not driven by kernel POLLOUT on the benchmark’s fd
the way TCP is, so AE_WRITABLE may never fire unless we explicitly run
the write path once after registration. The code previously did that
with ad-hoc writeHandler calls (issueFirstRequestForClients,
resetClient).
Benchmark RDMA runs should use a libvalkey tree that includes
valkey-io/libvalkey#301 (or equivalent fix).
---------
Signed-off-by: Ada-Church-Closure <nunotabashinobu066@gmail.com>
Add a release notes entry for **Valkey 9.0.4** covering the three
security fixes being ported to the `9.0` branch:
- **CVE-2026-23479** — Use-After-Free in unblock client flow
- **CVE-2026-25243** — Invalid Memory Access in RESTORE command
- **CVE-2026-23631** — Use-after-free when full sync occurs during a
yielding Lua/function execution
Only modifies `00-RELEASENOTES`. The actual code fixes are in separate
PRs targeting `9.0`.
---------
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
When a blocked client is woken up on a key, unblockClientOnKey()
re-executes its pending command via processCommandAndResetClient(). That
function returns C_ERR when the client was freed as a side effect of the
re-execution (it detects this by server.current_client becoming NULL
inside freeClient()).
The pre-fix code ignored the return value and unconditionally accessed
c->flag.blocked / c->flag.module afterwards -- a use-after-free on the
freed client.
The fix mirrors the existing dead-client handling in
processPendingCommandAndInputBuffer() (src/networking.c): on C_ERR,
unwind the execution unit, restore server.current_client, and return
without touching c.
Note on testing: reaching the C_ERR branch requires the reprocessed
command to synchronously free the current client (for example a
replica/primary-link teardown or a module path that calls freeClient()).
None of the blocking commands routed through unblockClientOnKey
(BLPOP/BLMOVE/BRPOPLPUSH/BLMPOP/ XREADGROUP and friends) take that path
from a plain RESP client, so a Tcl-level integration reproducer is not
feasible without adding a test-only debug hook; this patch does not add
such a hook. A targeted gtest reproducer was also considered but
rejected as net-negative: it would require ~200 lines of fake-client
scaffolding to exercise a 7-line defensive guard and would become a
"change detector" as warned against in src/unit/README.md. The existing
tests/unit/type/list.tcl suite (288 tests) exercises the happy path of
this function and continues to pass unchanged. Running the suite under
AddressSanitizer (make SANITIZER=address) provides the strongest signal
against regressions of this class of bug.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
Root cause: zipmapValidateIntegrity() and zipmapNext() use different
methods to calculate pointer advancement for length-encoded fields.
Validation reads the actual encoded size via
zipmapGetEncodedLengthSize() (which returns 5 for the 0xFE prefix), but
zipmapRawKeyLength() (used by zipmapNext during hash conversion)
recalculates via zipmapEncodeLength() which returns 1 for decoded
lengths < 254. A crafted zipmap with an overlong 5-byte encoding for a
small length passes validation but causes a 4-byte pointer mismatch in
zipmapNext(), leading to heap buffer over-reads during the
zipmap-to-listpack conversion.
Fix: add sanity checks in zipmapValidateIntegrity() to reject entries
where the decoded length < ZIPMAP_BIGLEN (254) but the encoding uses
more than 1 byte. This is applied to both field-name and value lengths.
Test: added a regression test in tests/unit/dump.tcl that crafts a
RESTORE payload with a 2-entry zipmap where the first field uses an
overlong 5-byte length encoding for value 3. Post-patch, this is cleanly
rejected by zipmapValidateIntegrity(). Pre-patch, the misaligned
zipmapNext() reads garbage (confirmed via server log: "Hash zipmap with
dup elements, or big length (0)") which also produces an error, so the
test serves as a defense-in-depth regression anchor rather than a strict
pass/fail differentiator. The actual heap over-read is detectable with
AddressSanitizer builds.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
Changes applied (3 files):
1. src/zipmap.c: Add sanity checks in zipmapValidateIntegrity() to
reject entries where the decoded length < ZIPMAP_BIGLEN (254) but the
encoding uses more than 1 byte. This prevents a pointer arithmetic
mismatch between validation and zipmapNext() that leads to heap buffer
over-reads.
2. src/rdb.c (hash zipmap conversion): Reorder the
hashtableAdd()/lpSafeToAdd() checks so lpSafeToAdd() is evaluated before
hashtableAdd() takes ownership of the field SDS. Add missing lpFree(lp)
in the error path to fix a memory leak when the conversion fails.
3. src/rdb.c (stream consumer PEL): Remove erroneous
streamFreeNACK(nack) in the "Duplicated consumer PEL entry" error path.
The nack is a shared reference from the global PEL (obtained via
raxFind), so freeing it here causes a double-free when the stream object
is later destroyed.
Test: added a regression test in tests/unit/dump.tcl that crafts a
RESTORE payload with a 2-entry zipmap where the first field uses an
overlong 5-byte length encoding for value 3. Post-patch, this is cleanly
rejected by zipmapValidateIntegrity(). Pre-patch, the misaligned
zipmapNext() reads garbage (confirmed via server log: "Hash zipmap with
dup elements, or big length (0)") which also produces an error, so the
test serves as a defense-in-depth regression anchor rather than a strict
pass/fail differentiator. The actual heap over-read is detectable with
AddressSanitizer builds.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
During a full sync, the functions/scripting engine is freed right before
loading the RDB from the primary. If a Lua script is still running and
yielding via the long-command mechanism at that moment, the freed engine
can be accessed when the script resumes, causing a use-after-free.
Add a guard at the top of replicaReceiveRDBFromPrimaryToMemory() to
check isInsideYieldingLongCommand() and return early, deferring the sync
processing until the script completes.
No validating test was added because the vulnerability is a race
condition between a yielding Lua script and a replication event handler,
which cannot be reliably triggered in a deterministic Tcl test.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
During a full sync, the functions/scripting engine is freed right before
loading the RDB from the primary. If a Lua script is still running and
yielding via the long-command mechanism at that moment, the freed engine
can be accessed when the script resumes, causing a use-after-free.
Add a guard at the top of replicaReceiveRDBFromPrimaryToMemory() to
check isInsideYieldingLongCommand() and return early, deferring the sync
processing until the script completes.
No validating test was added because the vulnerability is a race
condition between a yielding Lua script and a replication event handler,
which cannot be reliably triggered in a deterministic Tcl test.
Signed-off-by: ikolomi <ikolomin@amazon.com>
Co-authored-by: ikolomi <ikolomin@amazon.com>
## Fix SIGSEGV in VM_GetLRU, VM_SetLRU, VM_GetLFU, VM_SetLFU on NULL key
### Description
`VM_GetLRU`, `VM_SetLRU`, `VM_GetLFU`, and `VM_SetLFU` crash with
SIGSEGV when passed a NULL `ValkeyModuleKey` pointer. This happens
because all four functions dereference `key->value` without first
checking if `key` itself is NULL.
When a module opens a nonexistent key in `VALKEYMODULE_READ` mode,
`VM_OpenKey` returns NULL. If a module passes that NULL pointer into any
of these functions, the server crashes.
### Reproduction
```
valkey-server --loadmodule tests/modules/misc.so
valkey-cli test.getlru nonexistent_key
# Server crashes: SIGSEGV (signal 11)
```
### Fix
**`src/module.c`** — Add a `!key` guard before dereferencing
`key->value` in all four functions:
```c
// Before:
if (!key->value) return VALKEYMODULE_ERR;
// After:
if (!key || !key->value) return VALKEYMODULE_ERR;
```
**`tests/modules/misc.c`** — Add early return after
`open_key_or_reply()` in `test_getlru`, `test_setlru`, `test_getlfu`,
and `test_setlfu`. The helper already sends the error reply to the
client when the key is not found, so the command handler just needs to
stop processing:
```c
ValkeyModuleKey *key = open_key_or_reply(ctx, argv[1], VALKEYMODULE_READ|VALKEYMODULE_OPEN_KEY_NOTOUCH);
if (!key) return VALKEYMODULE_OK;
```
### After fix
```
valkey-cli test.getlru nonexistent_key
(error) key not found
# Server stays up
```
Signed-off-by: Yaron Sananes <yaron.sananes@gmail.com>
It's important to enabled ASAN on run-extra-tests label so we can
catch some of the bugs in the PRs before they are merged into unstable.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Big endian support on Valkey is "best effort" and not guaranteed, but we
haven't been doing any regular testing at all afaik. This PR adds a job
to the daily workflow to run UTs on an emulated big endian platform.
Integration tests failed excessively because of how slow emulation is.
I fixed several problems with tests and improved UT coverage of key
points where endian byte order matters - and fwiw I didn't find any
bugs. I think the main coverage gap remaining after this is RDB
serialization (maybe little endian <-> big endian round trips?)
There are couple lines of endian-specific code for #3166 and this change
can test it.
Signed-off-by: Rain Valentine <rsg000@gmail.com>
The function lpEncodeBacklen() uses `<= 127` for the 1-byte case but `<
16383`, `< 2097151`, and `< 268435455` for the subsequent cases. This
means the exact values 16383, 2097151, and 268435455 (i.e. 2^14-1,
2^21-1, 2^28-1) unnecessarily use one extra byte than needed:
- `l < 16383` → `16383` (2^14-1) uses 3 bytes instead of 2
- `l < 2097151` → `2097151` (2^21-1) uses 4 bytes instead of 3
- `l < 268435455` → `268435455` (2^28-1) uses 5 bytes instead of 4
The decoding side (`lpDecodeBacklen`) is unaffected since it parses
continuation bits continuously without discrete range checks.
This is a correctness issue and has no impact on data integrity since
encoding and decoding use the same function boundaries, but it wastes up
to 1 byte per affected entry.
Signed-off-by: fanpei91 <fanpei91@gmail.com>
When XTRIM marks the last entry in a listpack node as deleted, lpNext()
returns NULL after the lp-count field (EOF). The delta calculation (p -
lp) on a NULL pointer is undefined behavior and produces a garbage
pointer, corrupting the listpack. A subsequent XREAD hitting the
corrupted node triggers the lpValidateNext assertion failure and crashes
the server.
Guard the delta calculation with a NULL check so the while(p) loop
terminates naturally when the last entry is reached.
Fixes#3569
Signed-off-by: Saurabh Kher <saurabh@amazon.com>
Co-authored-by: Saurabh Kher <saurabh@amazon.com>
### Summary
This PR fixes a NULL pointer dereference (SIGSEGV) in
`connectSlotExportJob()`
(`src/cluster_migrateslots.c`) that can crash a Valkey cluster node,
causing a
denial-of-service condition.
### Root Cause
When `CLUSTER MIGRATESLOTS` is issued, a migration job is created with
state
`SLOT_EXPORT_CONNECTING`. On the next `clusterCron()` tick,
`proceedWithSlotMigration()` calls `connectSlotExportJob()`, which looks
up the
target node via `clusterLookupNode()`.
`clusterLookupNode()` can legitimately return `NULL` — for example, if
the
target node is removed from the cluster (e.g. via `CLUSTER FORGET`)
between the
time the migration job is created and the time the cron fires. This is a
realistic race condition in any cluster topology change scenario.
The return value was **never checked**, so the subsequent call to
`getNodeDefaultReplicationPort(n)` immediately dereferences the NULL
pointer,
crashing the process:
```c
// Before fix — vulnerable
clusterNode *n = clusterLookupNode(job->target_node_name, CLUSTER_NAMELEN);
int port = getNodeDefaultReplicationPort(n); // SIGSEGV if n == NULL
serverLog(..., n->ip, port); // second dereference
Signed-off-by: chenshi5012 <chenshi5012@163.com>
Fixes#1905
## Summary
The direct use of `je_calloc` in `src/allocator_defrag.c` causes
compilation failures on systems (e.g., Arch Linux with GCC 14.2.1) where
`calloc` is marked as deprecated and `-Werror=deprecated` is enabled.
## Changes
Replace the two `je_calloc` calls in `allocatorDefragInit()` with
`zcalloc_num`, which is the proper Valkey allocation wrapper that
provides the same semantics (num × size with zero-fill) without directly
invoking the deprecated `calloc` symbol.
## Testing
- Build compiles cleanly
- Integration tests pass (unit/memefficiency, defrag, unit/other — 51
passed, 0 failed)
Signed-off-by: jaduffy <jaduffy@amazon.com>
In `getKeysUsingKeySpecs`, when extracting keys based on the
`KSPEC_FK_KEYNUM `spec (like in the `EVAL` command), the server read the
number of keys from the arguments and calculated the expected end index.
However, it called `getKeysPrepareResult` to allocate memory for the
result array before validating whether last was within the bounds of the
actual arguments provided.
If a client sent a command with a huge declared number of keys (e.g.,
`COMMAND GETKEYS EVAL "return 1" 2147483647 key1`), the server would
allocate a massive amount of memory. Since `vm.overcommit_memory` is
recommended, this allocation would NOT normally have triggered OOM (we
never wrote to it so there is no physical memory allocated), but if you
disable overcommit, this could trigger an OOM.
You can reproduce it with:
```
$ prlimit --as=1073741824 src/valkey-server --save ""
...
384270:M 30 Apr 2026 04:27:24.456 * Ready to accept connections tcp
...
<in valkey-cli>
127.0.0.1:6379> command getkeys eval "return 1" 2147483647 key1
...
<in server log>
384270:M 30 Apr 2026 04:29:26.950 # Out Of Memory allocating 17179869176 bytes!
```
## Solution
* Moved the bounds check `if (last >= argc || last < first || first >=
argc)` to execute before the call to `getKeysPrepareResult`, preventing
the large allocation on invalid input.
* To further catch issues like this, protected against integer overflow
during the calculation of last by using a long long temporary variable.
If it exceeds INT_MAX or falls below INT_MIN, the spec is marked invalid
immediately.
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
clusterManagerNodePrimaryRandom() called srand(time(NULL)) on every
invocation, then immediately rand() % primary_count. When called in a
tight loop for uncovered slots, all calls within the same wall-clock
second produce the identical seed, causing every uncovered slot to be
assigned to the same primary node.
Remove the srand() call since the PRNG is already seeded at startup
(srand(time(NULL) ^ getpid()) at line 9838). This allows rand() to
advance its state across calls, distributing uncovered slots randomly
across available primaries.
---------
Signed-off-by: Abhishek Mathur <matshek@amazon.com>
Co-authored-by: Abhishek Mathur <matshek@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
When read() returns 0 (EOF/connection closed) in syncRead(), errno is
not set by POSIX, so it retains a stale value (typically 0). This causes
callers using connGetLastError() to log strerror(0) which is the
misleading string "Success".
Set errno = ECONNRESET on EOF in syncRead(), matching the existing
pattern used for the timeout case (errno = ETIMEDOUT).
Also set conn->last_errno = errno in connSocketSyncWrite,
connSocketSyncRead, and connSocketSyncReadLine wrappers, matching the
pattern used by their async counterparts connSocketWrite and
connSocketRead.
After this fix, replica logs will show:
"I/O error reading bulk count from PRIMARY: Connection reset by peer"
instead of the misleading:
"I/O error reading bulk count from PRIMARY: Success"
---------
Signed-off-by: Abhishek Mathur <matshek@amazon.com>
Signed-off-by: djk1027 <djk9510271@gmail.com>
Co-authored-by: Abhishek Mathur <matshek@amazon.com>
Co-authored-by: Daejun Kim <djk9510271@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Free BYPOLYGON points before returning from invalid COUNT parsing paths
in GEOSEARCH/GEOSEARCHSTORE.
Closes#3567
---------
Signed-off-by: Su Ko <rhtn1128@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Address this compile warning:
```c
CC util.o
util.c:638:1: warning: ‘no_sanitize’ attribute directive ignored [-Wattributes]
__attribute__((no_sanitize_address, no_sanitize("thread"), used)) static int (*string2ll_resolver(void))(const char *, size_t, long long *) {
^~~~~~~~~~~~~
```
Addresses portability concerns around these attributes.
---------
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
I noticed this LTO compile warning in the eval code. Looks like it's
getting confused about an sds length, even though checked above. Just
added an assert to clarify.
The warning:
```c
LINK valkey-server
eval.c: In function ‘evalExtractShebangFlags’:
eval.c:263:27: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
*out_engine = zcalloc(engine_name_len + 1);
^
zmalloc.c:324:7: note: in a call to allocation function ‘valkey_calloc’ declared here
void *zcalloc(size_t size) {
^
```
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
This change was introduced in #3382. This test is already very slow on
its own. Under valgrind it gets slow enough that the per-node restart
step lets primaries be marked FAIL and triggers failovers, after which
"Verify slaves consistency" no longer holds since it assumes the original
topology.
It was never run under valgrind before and exercises nothing valgrind
meaningfully covers, so just tag it valgrind:skip.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The argument `count /= 2` modifies `count` as a side effect, and the
following `count /= 2` divides it again unnecessarily.
Since `count` is not used after this point, fix it by using `count / 2`
without the side effect and remove the redundant second assignment.
Signed-off-by: djk1027 <djk9510271@gmail.com>
Migrated the remaining cluster tests to tests/unit/cluster/ to use the same
framework for all cluster tests. Cleaned up the obsolete cluster test framework
files and updated the CI workflows to use the new unified test runner.
Changes:
Moved and mapped 6 test files:
- 03-failover-loop.tcl → Merged into existing failover.tcl
- 04-resharding.tcl → resharding.tcl
- 12-replica-migration-2.tcl + 12.1-replica-migration-3.tcl →
replica-migration-slow.tcl
- 07-replica-migration.tcl → Merged into existing replica-migration.tcl
- 28-cluster-shards.tcl → Merged into existing cluster-shards.tcl
Other changes:
- Converted old framework APIs (e.g., K, RI) to new framework APIs (e.g., R, srv)
- Added process_is_alive check in cluster_util.tcl to fix an exception in
failover tests caused by executing ps on dead processes
- Heavy tests (resharding, replica-migration-slow) marked with slow tag and
wrapped in run_solo to prevent resource contention in sanitizer environments
- replica-migration-slow marked with valgrind:skip tag since it is very slow
- Removed the entire tests/cluster/ directory including run.tcl, cluster.tcl,
includes/, and helpers/
- Kept runtest-cluster as a wrapper script (exec ./runtest --cluster "$@")
- Removed ./runtest-cluster calls from .github/workflows/daily.yml as cluster
tests are now included in ./runtest
Closes#2297.
Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Remove eval script cache entries that belong to a scripting engine when
that engine is unregistered. This prevents the eval cache from retaining
dangling engine pointers and keeps the tracked script memory in sync
after engine shutdown.
The scripting engine unregister path now invokes a new eval cleanup
helper, which scans the cached scripts, drops matching entries from the
LRU list and dictionary, and adjusts cache memory accounting accordingly.
* scripting engine
* eval cache
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
If not cleared, the job may no longer be valid by the time the client
goes to cleanup. This dangling reference could cause a crash if you set
slot-migration-log-max-len to 0 and are very unlucky.
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
The default value of lua-enable-insecure-api cannot be safely changed
from no to yes due to two issues:
1. In createEngineContext(), lua_enable_insecure_api was hardcoded to 0
before initializing Lua states, so deprecated APIs (newproxy, setfenv,
getfenv) were never registered in the global table regardless of the
actual config value. Once the global table is locked, the config
change has no effect.
2. lua_insecure_api_current was initialized to 0 (struct zero-init) and
never synced with the final config value. If the default was changed
to yes(1), a subsequent CONFIG SET no would see both values as 0 and
skip the evalReset() call in updateLuaEnableInsecureApi().
Fix by reading the real config via isLuaInsecureAPIEnabled() in
createEngineContext() before Lua state initialization, and syncing
lua_insecure_api_current after all config sources (default, config file,
command-line args) are applied.
Signed-off-by: Binbin <binloveplay1314@qq.com>
In here we should go to error to free the resources:
```
error:
if (listen_cmid) rdma_destroy_id(listen_cmid);
if (listen_channel) rdma_destroy_event_channel(listen_channel);
ret = ANET_ERR;
end:
freeaddrinfo(servinfo);
return ret;
}
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
The bug was in dismissHashtable(), which computes the size passed to
zmadvise_dontneed() for the top-level hashtable
tables.
ht->tables[i] points to a contiguous array of bucket objects, but the
code used sizeof(bucket *) instead of
sizeof(bucket) when calculating the length. That means it treated the
allocation like an array of pointers rather than an
array of buckets.
As a result, the advised range was much smaller than the actual table
allocation. On 64-bit builds, bucket is 64 bytes
while bucket * is 8 bytes, so only about one eighth of the table was
covered. This does not usually break correctness,
but it defeats the purpose of the function: after a fork, we want to
tell the kernel that the hashtable pages are no
longer needed so we reduce copy-on-write overhead. With the wrong size,
most of the table memory was never included in
that hint.
The fix is to use sizeof(bucket) so the full top-level bucket array is
passed to zmadvise_dontneed().
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
### Summary
The daily CI sanitizer jobs with clang are failing during the build
step.
When the static Lua module is built with `-flto`, the `.o` files contain
LLVM bitcode that gets archived into `libvalkeylua.a`. The system linker
cannot read this bitcode, causing build failures:
`/usr/bin/ld:
/home/runner/work/valkey/valkey/src/modules/lua/libvalkeylua.a: member
/home/runner/work/valkey/valkey/src/modules/lua/libvalkeylua.a(debug_lua.o)
in archive is not an object`
The previous fix (#3546) pinned clang to version 17, but this was
insufficient, the issue is not just a version mismatch but that the
system linker fundamentally cannot read LTO bitcode from `.a` archives.
Example failure:
https://github.com/valkey-io/valkey/actions/runs/24865821147/job/72801509768
### Fix
Strip LTO flags from OPTIMIZATION in the Lua module Makefile using
`override`
Tested: https://github.com/hanxizh9910/valkey/actions/runs/24913834442
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
This PR bootstraps Valkey's provenance guard integration.
The provenance guard is a content-based similarity detection system that helps maintain proper code provenance by comparing incoming PR changes against fingerprint databases built from Redis commits and PRs. The matching logic now lives in the external `valkey-io/verify-provenance` action repository; this PR wires Valkey to that action and seeds the required database branch.
Key features:
* Content-based detection: Uses normalized diff fingerprints and fuzzy matching to detect similar changes, including cases where files have moved or been refactored.
* Externalized action logic: The check and refresh implementation is maintained in `valkey-io/verify-provenance` and is pinned by exact commit SHA from Valkey workflows.
* Provenance Guard workflow: Runs on PR activity to check incoming changes against the provenance databases and report potential matches.
* Daily Refresh workflow: Runs daily to refresh PR fingerprints and commits updated data back to `verify-provenance-db`.
* Dedicated DB branch: Stores provenance databases on the orphan `verify-provenance-db` branch, separate from Valkey source code.
* Privacy-first storage: Stores compressed non-reversible fingerprints, not source code.
The initial `verify-provenance-db` branch has been bootstrapped with fingerprints of Redis commits and PRs.
---------
Signed-off-by: Ping Xie <pingxie@outlook.com>
This follows up on the commandresult API work and fixes cleanup around
unsubscribe and module unload.
The main issue was that command-result event listeners could leave stale
state behind. On unload, we removed the listeners themselves but didn’t
fully update the fast-path listener counters. Separately, unsubscribing
with a NULL callback could behave badly if the listener wasn’t present
anymore. In practice, that meant later commands could still walk into
command-result event handling after the module was supposed to be
cleaned up.
Failed in Daily as well yesterday:
https://github.com/valkey-io/valkey/actions/runs/24753491944/job/72421581610#step:10:852
Related Failures:
https://github.com/valkey-io/valkey/pull/2936#issuecomment-4290490199
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Replace bespoke 59-line docker build with the 10-line canonical caller
that uses hanzoai/.github/.github/workflows/docker-build.yml@main.
Native amd64+arm64 ARC runners, semver + branch tags, multi-arch manifest
via the one shared workflow path. Cache-key preserved for scope=kv.
Co-authored-by: Hanzo AI <dev@hanzo.ai>
Replace bespoke 59-line docker build with the 10-line canonical caller
that uses hanzoai/.github/.github/workflows/docker-build.yml@main.
Native amd64+arm64 ARC runners, semver + branch tags, multi-arch manifest
via the one shared workflow path. Cache-key preserved for scope=kv.
### Analysis
The daily CI sanitizer jobs with clang are failing during the build
step.
The `ubuntu-latest` runner now has clang 18, but the LLVM gold plugin
is still version 17. When the static Lua module is built with `-flto`,
the `.o` files contain LLVM 18 bitcode that the gold plugin (v17) cannot
read:
`bfd plugin: LLVM gold plugin has failed to create LTO module: Unknown
attribute kind (91) (Producer: 'LLVM18.1.3' Reader: 'LLVM 17.0.6')
`
Example failure:
https://github.com/valkey-io/valkey/actions/runs/24753491944/job/72421581512
### Fix
Pin the sanitizer jobs to `clang-17` so the compiler and gold plugin
versions match.
Tested(successfully built):
https://github.com/hanxizh9910/valkey/actions/runs/24859845008
### Note
If `clang-17` is removed from the `ubuntu-latest` image in the future,
we may need to either add an explicit install step
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
The SPMC queue from #3324 needs each `spmcCell` to be cache-line
aligned, but plain `zmalloc()` does not guarantee that in all build
configurations.
This change introduces `zmalloc_cache_aligned()` and uses it for the
SPMC queue buffer allocation in `spmcInit()`.
Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Match HGETDEL with the existing batch-delete pattern used by HDEL.
HDEL already pauses hashtable auto-shrink while deleting multiple fields
so shrink evaluation is deferred until the batch completes. HGETDEL was
missing the same optimization even though it also deletes fields in a loop.
Pause auto-shrink for hashtable-encoded hashes before the HGETDEL delete
loop and resume it once afterwards. This preserves observable behavior
and reduces redundant shrink work for multi-field deletes.
Same as #3144.
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
## Summary
Fix a file descriptor leak in `connSocketBlockingConnect()` when
`aeWait()` times out.
## Bug
When `anetTcpNonBlockConnect()` succeeds but `aeWait()` times out (e.g.,
MIGRATE to an unreachable host), the fd is leaked because it was never
assigned to `conn->fd`. The caller's `connClose()` checks `conn->fd !=
-1` and skips cleanup.
## Fix
Assign `conn->fd = fd` immediately after `anetTcpNonBlockConnect()`
succeeds, before `aeWait()`. This way the caller's normal `connClose()`
cleanup path handles the fd on any error, which is consistent with how
the rest of the connection lifecycle works.
TLS connections also benefit since `connTLSBlockingConnect` delegates to
this function for the TCP layer.
## Reproducer
```
valkey-cli SET key hello
# Repeat against unreachable host:
for i in $(seq 1 30); do valkey-cli MIGRATE 192.0.2.1 6379 key 0 500; done
# Check: /proc/<pid>/fd shows 30 leaked socket fds
```
*This issue was generated by AI but verified, with love, by a human.*
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
There is a double free issue in the code. The error handling path called
both decrRefCount(o) and streamFreeNACK(nack), but the nack was obtained
from cgroup->pel via raxFind and is still referenced there. decrRefCount(o)
frees it through freeStream -> streamFreeCG -> raxFreeWithCallback(cg->pel, zfree),
so the explicit streamFreeNACK(nack) causes a double free.
Remove the redundant streamFreeNACK(nack) call and add a regression
test with a crafted corrupt payload that triggers the duplicate consumer
PEL entry path.
This was introduced in 6ec241d934.
Signed-off-by: Binbin <binloveplay1314@qq.com>
## Problem
`Fix cluster` in `tests/unit/cluster/many-slot-migration.tcl` has been
timing out daily on valgrind jobs since April 3, 2026. The test runs 10
cluster nodes under valgrind, migrating 40,000 keys across 1,000 slots —
too much work for valgrind-instrumented builds.
The slowdown is caused by #3366 (dict→hashtable wrapper). Under `-O0`
(valgrind builds), the `static inline` wrappers become real function
calls that valgrind instruments, adding ~75% overhead to hot paths like
`dictSize`. This compounds across 10 valgrind processes over a 20-minute
migration test. No impact on production builds (`-O2` inlines everything).
## Fix
Scale the test workload down under valgrind: 10,000 keys / 250 slots
instead of 40,000 / 1,000. Normal runs are unchanged. Still exercises
the same cluster repair path.
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Co-authored-by: sarthakaggarwal97 <sarthakaggarwal97@users.noreply.github.com>
https://github.com/valkey-io/valkey/pull/3324 introduced `BATCH_SIZE` as
a const int local variable and used it as an array bound. Clang 17
rejects this with:
```
io_threads.c:305:22: error: variable length array folded to constant array as an extension [-Werror,-Wgnu-folding-constant]
305 | void *batch_jobs[BATCH_SIZE];
| ^~~~~~~~~~
1 error generated.
make[1]: *** [io_threads.o] Error 1
make: *** [all] Error 2
```
Old Clang versions do not emit this warning, maybe that is why the CI
passed. Fix by promoting `BATCH_SIZE` to a file-scope `#define`.
Signed-off-by: Yang Zhao <zymy701@gmail.com>
## Add Command Result Event Notifications for Modules
### Summary
1. Adds new server events `ValkeyModuleEvent_CommandResultSuccess` and
`ValkeyModuleEvent_CommandResultFailure` for that can notify subscribed
modules after command execution. This enables modules to implement audit
logging, error monitoring, performance tracking, and observability
without modifying core server code.
2. Adds new server event `ValkeyModuleEvent_CommandResultACLDenied` for
commands rejected by ACL. Together with PR #2237 this covers auditing of
authentication and authorisation.
### Motivation
There is currently no module API to observe command outcomes after
execution or to capture ACL denied commands. Modules that need audit
logging or error monitoring have no mechanism to be notified when
commands succeed or fail, what arguments were used, how long they took,
or how many keys were modified. This feature fills that gap using the
existing `ValkeyModule_SubscribeToServerEvent()` infrastructure.
### API
#### Events
| Event | Description |
|---|---|
| `ValkeyModuleEvent_CommandResultSuccess` | Fired after a command
completes successfully |
| `ValkeyModuleEvent_CommandResultFailure` | Fired after a command
returns an error |
| `ValkeyModuleEvent_CommandACLDenied` | Fired after a command is
rejected by ACL |
These are separate events (not sub-events), so modules can for example
only subscribe to failures without incurring any callback overhead for
successful commands.
#### Event Data: `ValkeyModuleCommandResultInfo`
The `data` pointer passed to the callback can be cast to
`ValkeyModuleCommandResultInfo`:
```c
typedef struct ValkeyModuleCommandResultInfo {
uint64_t version; /* Version of this structure for ABI compat. */
const char *command_name; /* Full command name (e.g., "SET", "CLIENT|LIST"). */
long long duration_us; /* Execution duration in microseconds. */
long long dirty; /* Number of keys modified. */
uint64_t client_id; /* Client ID that executed the command. */
int is_module_client; /* 1 if command was from RM_Call, 0 otherwise. */
int argc; /* Number of command arguments. */
ValkeyModuleString **argv; /* Command arguments array (zero-copy, read-only). */
int acl_deny_reason; /* ACL_DENIED_CMD/KEY/CHANNEL/AUTH; 0 for non-ACL events */
const char *acl_object; /* Denied resource name (key/channel); NULL for CMD/AUTH */
} ValkeyModuleCommandResultInfoV1;
```
The struct is versioned (`VALKEYMODULE_COMMANDRESULTINFO_VERSION`) for
forward-compatible API evolution.
### Usage Example
```c
/* Callback receives events for whichever event(s) you subscribed to */
void OnCommandResult(ValkeyModuleCtx *ctx, ValkeyModuleEvent eid,
uint64_t subevent, void *data) {
VALKEYMODULE_NOT_USED(ctx);
VALKEYMODULE_NOT_USED(subevent);
ValkeyModuleCommandResultInfo *info = (ValkeyModuleCommandResultInfo *)data;
if (info->version != VALKEYMODULE_COMMANDRESULTINFO_VERSION) return;
int failed = (eid.id == VALKEYMODULE_EVENT_COMMAND_RESULT_FAILURE);
/* Access fields directly */
printf("command=%s status=%s duration=%lldus dirty=%lld client=%llu\n",
info->command_name,
failed ? "FAIL" : "OK",
info->duration_us,
info->dirty,
info->client_id);
/* Access argv (read-only, zero-copy) */
for (int i = 0; i < info->argc; i++) {
size_t len;
const char *arg = ValkeyModule_StringPtrLen(info->argv[i], &len);
printf(" argv[%d] = %.*s\n", i, (int)len, arg);
}
}
/* Subscribe in ValkeyModule_OnLoad or at runtime */
/* Option A: command failures only (recommended for audit logging) */
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultFailure, OnCommandResult);
/* Option B: command successes only */
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultSuccess, OnCommandResult);
/* Option C: both command outcomes*/
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultSuccess, OnCommandResult);
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultFailure, OnCommandResult);
/* Subscribe to ACL Denied */
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultACLDenied, onCommandResult);
/* Unsubscribe pass NULL callback */
ValkeyModule_SubscribeToServerEvent(ctx,
ValkeyModuleEvent_CommandResultFailure, NULL);
```
### Design Decisions
- **Separate events instead of sub-events**: Modules subscribing only to
failures have zero overhead for successful commands (~2ns listener-list
check vs ~30ns callback invocation per command). This is critical since
success events fire on the hot path of every command.
- **Stack-allocated info struct**: The `ValkeyModuleCommandResultInfoV1`
is built on the stack ΓÇö no heap allocation per event.
- **Zero-copy argv**: Arguments are passed directly from the client's
argv array. Any integer-encoded arguments (from `tryObjectEncoding()`
during command execution) are decoded to string-encoded objects before
being passed to the callback, ensuring compatibility with
`ValkeyModule_StringPtrLen()`.
- **Early exit**: If no modules are subscribed to any server events, the
event firing function returns immediately before building the info
struct.
- **Uses existing server event infrastructure**: Follows the
`ValkeyModule_SubscribeToServerEvent()` pattern used by all other server
events, rather than introducing a new callback mechanism.
### Files Changed
| File | Change |
|---|---|
| `src/valkeymodule.h` | Event IDs, event constants,
`ValkeyModuleCommandResultInfoV1` struct |
| `src/module.c` | `moduleFireCommandResultEvent()`, event
documentation, event version entries |
| `src/module.h` | Function declaration |
| `src/server.c` | Call `moduleFireCommandResultEvent()` from `call()`
after command execution |
| `src/server.c` | Call to `moduleFireCommandACLDeniedEvent` in
`processCommand` after ACL rejection |
| `tests/modules/commandresult.c` | Test module exercising the full API
|
| `tests/unit/moduleapi/commandresult.tcl` | Integration tests |
---------
Signed-off-by: martinrvisser <mvisser@hotmail.com>
Signed-off-by: martinrvisser <martinrvisser@users.noreply.github.com>
Co-authored-by: Ricardo Dias <rjd15372@gmail.com>
Add a build option to compile the Lua scripting engine as a static
module and wire the server to load it directly at startup when enabled.
The module load path now resolves on-load and on-unload entry points
from the main binary, and the module lifecycle keeps those callbacks so
unload works without a shared library handle.
The Lua module build was updated to support both static and shared
variants, with the static path exporting visible wrapper symbols and
linking the server with the module archive. While touching the Lua code,
a few internal symbols were renamed for consistency and the monotonic
time helper was clarified.
Note that this PR addresses the LUA module, but it can be applied to
other "core" modules (like: Bloom, Json, Search and others). With this
change, it will be easier to ship Valkey bundle with modules.
Areas touched:
* CMake
* Makefile
* Lua scripting module
* Core module loading
**Generated by CodeLite**
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
This improves COB memory tracking when using copy avoidance for bulk
string replies. This fix addresses underestimation of client memory
usage that occurred when reply buffers stored pointers to shared `robj`
instead of copying data.
IO threads calculate actual reply sizes by calling `sdslen()` on strings
before writing, for that we need atomic `tracked_for_cob` flag in
payload headers to prevent race conditions and double accounting.
See #2396
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
`hpersistCommand` calls `addReplyArrayLen` before `lookupKeyWrite` +
`checkType`. When HPERSIST targets a non-hash key, the server writes a
RESP array header followed by a WRONGTYPE error — a malformed response
that permanently desynchronizes the client connection.
This moves `lookupKeyWrite` + `checkType` before `addReplyArrayLen`,
matching the pattern used by every other HFE command (e.g.
`hgetdelCommand`, `hexpireGenericCommand`).
Added a test for HPERSIST on a wrong-type key.
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
## Problem
The test `Test module aof save on server start from empty` in
`tests/unit/moduleapi/hooks.tcl` sporadically crashes with `I/O error
reading reply`.
**Frequency:** 2 out of 15 days (March 26 on
`centosstream9-tls-module-no-tls`, April 8 on `fedorarawhide-jemalloc`).
**Example failing run:**
https://github.com/valkey-io/valkey/actions/runs/24110987718/job/70345236353
## Root Cause
The crash is a **use-after-unload** in the auth test module's blocking
authentication thread, NOT a timing issue in the AOF test.
The crash log from April 8 shows:
```
71112:M 00:42:59.710 * Module testacl unloaded
71112:M 00:42:59.711 # crashed by signal: 11, si_code: 1
71112:M 00:42:59.711 # Crashed running the instruction at: 0x7f9dc717384b
```
The sequence:
1. `blocking_auth_cb` spawns a background thread
(`AuthBlock_ThreadMain`) that sleeps 500ms
2. Thread wakes, calls `ValkeyModule_UnblockClient()` → main thread
processes unblock, decrements `module->blocked_clients`
3. Auth command completes, test calls `r module unload testacl`
4. `moduleUnloadInternal` checks `blocked_clients == 0` if true,
proceeds with `dlclose()`
5. **But the background thread is still executing cleanup code**
(freeing strings, returning from function)
6. Thread returns into unmapped memory → **SIGSEGV**
The `invalidFunctionWasCalled` in the stack trace is the crash handler's
safety stub, and the crashing address `0x7f9dc717384b` is in the
unmapped auth.so address space.
## Fix
Track the background thread ID and `pthread_join()` it in
`ValkeyModule_OnUnload` before the module is dlclose'd. This ensures the
thread has fully exited before the code is unmapped.
The key insight is that `ValkeyModule_UnblockClient()` signals "auth is
done" but not "thread is done" — the thread still has cleanup code to
execute after that call. `pthread_join()` is the correct synchronization
point because it only returns after the thread has fully exited.
No mutex is needed since both `blocking_auth_cb` (which creates the
thread) and `OnUnload` (which joins it) run on the main event loop
thread.
Changes to `tests/modules/auth.c`:
- Add global `blocking_auth_tid` and `blocking_auth_tid_valid` flag
- Set `blocking_auth_tid_valid = 1` after successful `pthread_create`
- In `OnUnload`, `pthread_join` the thread if one was created
## Testing
Ran `unit/moduleapi/hooks` 100 loops on rpm-distros and ubuntu runners —
**all passed**:
- **Workflow run:**
https://github.com/roshkhatri/valkey/actions/runs/24164276124
- **Config:** `--loops 100 --single unit/moduleapi/hooks` on
`almalinux8`, `almalinux9`, `fedoralatest`, `fedorarawhide`,
`centosstream9`, `ubuntu-jemalloc`, `ubuntu-arm`
- **Result:** 7/7 jobs ✅, zero failures across 700 total test iterations
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Instead of "scanning" random bucket chains using a random cursor for
each scan call, start at a random cursor and then continue sampling
buckets in scan order. The scan stops when we have sampled all elements,
so the cursor never wraps around to sample the same buckets again. This
ensures that we don't get any duplicate samples.
The functions hashtableRandomEntry and hashtableFairRandomEntry keeps
the old behavior using another sampling function preserving their
behavior. The fairness tests fail if we use the modified
hashtableSampleEntries.
This restores the behavior of dictGetSomeKeys (which is now an alias of
hashtableSampleEntries) and deflakes the test case:
Gossip count scales with higher percentage of
`cluster-message-gossip-perc`
in tests/unit/cluster/packet.tcl
Fixes#3454
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
The test introduced in #2227 is fragile because it unconditionally asserted
that "best ranked replica" would be logged for every shard's replica. This
assumption was incorrect for two reasons:
1. myselfIsBestRankedReplica() requires failover_failed_primary_rank == 0.
When two primaries fail simultaneously, only the shard with the lowest
shard_id gets failed_primary_rank == 0. The other shard's replicas can
only trigger "Myself become the best ranked replica" after the first
shard completes failover and the result propagates via gossip.
2. clusterAllReplicasThinkPrimaryIsFail() requires all sibling replicas
to have their MY_PRIMARY_FAIL flag propagated via gossip. If the
election delay is shorter than the gossip propagation time, the
replica may start the election before receiving the flag.
It's also too strict and checks too many things. Like we can check that there
is no delay before starting the failover, but we shouldn't check that the rank
0 replica actually wins, because maybe it doesn't. Also we're not sure the other
replica does a psync afterwards, it can also do a full sync.
Restructure the test into three focused scenarios with retry logic:
- Test 0 (3 primaries + 1 replica): The sole replica is deterministically
the best ranked replica. All conditions are trivially satisfied.
- Test 1 (3 primaries + 2 replicas): Single primary failure with two
competing replicas. failed_primary_rank is deterministically 0, but
MY_PRIMARY_FAIL gossip timing may prevent triggering in some runs.
Uses internal retry with cluster recovery to ensure at least one
successful trigger.
- Test 2 (5 primaries + 7 replicas): Two primaries failure. Verifies no
failover timeout and uses retry to cover the "best ranked replica"
path for at least one shard.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Cherry-pick of 3e45a495c from unstable.
Skipped dual-channel-replication.tcl (lazyfree test doesn't exist on
9.0).
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Adds cluster bus byte metrics to `CLUSTER INFO`, split by admin,
pub/sub, and module traffic. The change tracks sent and received bytes
on the cluster bus, exposes them as
`cluster_stats_*_bytes_{sent,received}`.
Example:
```
> src/valkey-cli CLUSTER INFO
cluster_state:fail
...
cluster_stats_bytes_sent:46088
cluster_stats_bytes_received:46080
cluster_stats_pubsub_bytes_sent:0
cluster_stats_pubsub_bytes_received:0
cluster_stats_module_bytes_sent:0
cluster_stats_module_bytes_received:0
total_cluster_links_buffer_limit_exceeded:0
```
Fixes: https://github.com/valkey-io/valkey/issues/1929
---------
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Minor cleanup, the function will return -1 on error.
It worked anyway before because it is implicitly converted:
-1 => SIZE_MAX => -1 again when it's implicitly cast back to
ssize_t.
Signed-off-by: Binbin <binloveplay1314@qq.com>
This allows setting the host and port using environment variables, which
makes using valkey-cli way easier in some development environments. No
need to recall the right host/port, just let your `.env` do the work.
---
Built locally and tested with,
```fish
VALKEY_PORT=36379 ./src/valkey-cli
```
```console
127.0.0.1:36379>
```
And using both,
```fish
VALKEYCLI_HOST=(hostname).local VALKEYCLI_PORT=36379 ./src/valkey-cli
```
```console
<HOSTNAME>.local:36379>
```
Signed-off-by: Dietrich Daroch <Dietrich@Daroch.me>
#### Summary
This PR redesigns the IO threading communication model, replacing the
inefficient client-list polling approach with a high-performance,
lock-free queue architecture. This change improves throughput by
**8–17%** across various workloads and lays the groundwork for
offloading command execution to IO threads in following PRs.
### Performance Comparison: Unstable vs New IO Queues
| Type | Operation | Unstable Branch (M TPS) | New IO Queues (M TPS)|
Difference (%) |
| :--- | :--- | :--- | :--- | :--- |
| **CME**<sup>1</sup> | SET | 1.02 | 1.19 | **+16.67%** |
| **CME** | GET | 1.30 | 1.47 | **+13.08%** |
| **CMD**<sup>2</sup> | SET | 1.15 | 1.35 | **+17.39%** |
| **CMD** | GET | 1.52 | 1.64 | **+7.89%** |
<sup>1</sup> Amazon terminology for cluster mode
<sup>2</sup> Amazon termonology for standalone mode, i.e. config
`cluster-enabled no`
- Test Configuration: 8 IO threads • 400 clients • 512-byte values • 3M
keys
#### Motivation
The previous IO model had several limitations that created performance
bottlenecks:
* **Inefficient Polling:** The main thread lacks a direct notification
mechanism for completed work. Instead, it must constantly iterate
through a list of all pending clients to check their state, wasting
significant CPU cycles.
* **Manual Load Balancing:** Jobs are assigned to specific threads
upfront. This requires the main thread to predict which thread to use,
often leaving some threads idle while others are overloaded.
* **Static Scaling:** Thread activation relies on a fixed heuristic
(e.g., 1 thread per 2 events). This approach fails to adapt to varying
workloads, such as TLS connections or differing read/write sizes.
### The Solution
To address these inefficiencies, this PR replaces the single SPSC queue
used currently with three specialized queues to handle communication and
load balancing more effectively.
#### 1. Main > IO: Shared Queue (Single Producer Multi Consumer)
Single queue from the main-thread to IO threads.
* **Automatic Load Balancing:** All threads pull from the same source.
Busy threads take less work, and idle threads take more, so we don't
need to manually select a thread.
* **Adaptive Scaling:** We now use the queue depth to decide when to add
or remove threads. If the queue is full, we scale up; if it's empty, we
scale down.
* *Ignition:* To get things started before the queue fills up, we
monitor the main thread's CPU. If usage goes over 30%, we wake up the
first IO thread.
* **Implementation:** To prevent contention among consumers, each item
in the ring buffer is padded to reside in its own cache line. Sequence
numbers are utilized to indicate whether a cell is empty or populated,
allowing threads to safely claim work.
#### 2. IO > Main: The Response Channel (MPSC Queue)
We replaced the old polling loop with a response queue.
* ** Faster Completion:** IO threads push completed jobs into this
queue. The main thread detects new data simply by checking if the queue
is not empty, removing the need to scan pending clients.
* **Contention Management:** To avoid lock contention, each thread
reserves a slot by atomically incrementing the tail index. In the rare
event that the queue is full, pending jobs are buffered in a local
temporary list until space becomes available.
#### 3. MAIN > IO (Thread-Specific): Private Inbox (SPSC Queue)
We kept the existing Single-Producer Single-Consumer (SPSC) queues for
tasks that must happen on a specific thread (like freeing memory
allocated by that thread). IO threads always check their private inbox
before looking at the shared queue.
### Changes Required
* **Async client release**
The main thread no longer busy-waits for IO threads to finish with a
client. Since the client must be popped from the multi-producer queue
before it can be released, clients with pending IO are now marked for
asynchronous closure.
* **eviction clients logic**
Updated evictClients() to account for memory pending release (clients
marked close_asap). freeClient() now returns a status code (1 for freed,
0 for async-close) to ensure the eviction loop does not over-evict by
ignoring memory that is about to be reclaimed.
* **events-per-io-thread config**
Replaced the `events-per-io-thread` configuration with
`io-threads-always-active`. as we no longer track events, since this
config is use only for tests no backward compatibility issue arises.
* **packed job instead of handlers**
Jobs are now represented as tagged pointers (using lower 3 bits for job
type) instead of separate `{handler, data}` structs. This reduces memory
overhead and allows jobs to be passed through the queues as single
pointers.
* **head caching in spsc queue**
The SPSC queue now caches the `head` index on the producer side
(`head_cache`) to avoid frequent atomic loads. The producer only
refreshes from the atomic `head` when the cache indicates the queue
might be full, reducing cross-thread cache-line bouncing.
* **deferred commit in SPSC queue**.
`spscEnqueue()` supports batching via a `commit` flag. Multiple jobs can
be enqueued with `commit=false`, then flushed with a single
`spscCommit()` call, reducing atomic operations and cache-line bouncing.
* **rollback on fullness check failure**
When `spmcEnqueue()` fails due to a full queue, the client state is
rolled back (e.g., `io_write_state` reset to `CLIENT_IDLE`). This
rollback approach removes the need to call an expensive `isFull` check
before every enqueue, we just attempt the enqueue and revert if it
fails.
* **epoll offloading via SPSC at high thread counts**.
When `active_io_threads_num > 9`, poll jobs are sent to per-thread SPSC
queues (round-robin). Since threads check their private queue first,
this ensures poll jobs are processed promptly without waiting behind
jobs in the shared SPMC queue.
* **avoid offload write before read comes back**
Added a check `if (c->io_read_state == CLIENT_PENDING_IO) return C_OK`
in `trySendWriteToIOThreads()`. In the previous per-thread SPSC
implementation, we could send consecutive read and write jobs for the
same client knowing a single thread would handle them in order. With the
shared SPMC queue, different threads may pick up the jobs, so we must
wait for the read to complete before sending a write to avoid 2 threads
handling the same client.
* **removing pending_read_list_node from client and
clients_pending_io_read/write lists from server**
Removed `pending_read_list_node` from the `client` struct and
`clients_pending_io_read`/`clients_pending_io_write` lists from
`valkeyServer`. as the new mpsc eliminates the need for these tracking
structures.
* **added inst metrics for pending io jobs**
Added `instantaneous_io_pending_jobs` metric via `STATS_METRIC_IO_WAIT`
to track average queue depth over time.
* **added stat for current active threads number**
Added `active_io_threads_num` to the INFO stats output for better
visibility.
* **added internal inst metric for main-thread cpu (non apple
compliant)**
Added `STATS_METRIC_MAIN_THREAD_CPU_SYS` to track main thread CPU usage
via `getrusage(RUSAGE_THREAD)`. This powers the "ignition" policy, when
CPU exceeds 30%, the first IO thread is activated. `RUSAGE_THREAD` is
Linux-specific, so macOS falls back to event-count heuristics.
* **added stat for pending read and writes for io**
Added `io_threaded_reads_pending` and `io_threaded_writes_pending` stats
to track how many read/write jobs are currently in-flight to IO threads.
* **added volatile for crashed**
Changed `server.crashed` from `int` to `volatile int` to ensure the
crash flag is visible across threads immediately, allowing IO threads to
detect a crash and stop sending responses back to the main thread to
avoid deadlock on crash.
---------
Signed-off-by: Uri Yagelnik <uriy@amazon.com>
Signed-off-by: akash kumar <akumdev@amazon.com>
Co-authored-by: Uri Yagelnik <uriy@amazon.com>
Co-authored-by: Dan Touitou <dan.touitou@gmail.com>
This PR introduces `ValkeyModule_CallArgv`, a new low-level module API
for calling server commands. It is designed as a complement to the
existing `ValkeyModule_Call` covering two scenarios where
`ValkeyModule_Call` is suboptimal:
1. **The module already holds the arguments as an argv array.**
`ValkeyModule_Call` always allocates a new argv array, creates a new
string object for the command name, and calls
`incrRefCount`/`decrRefCount` on every argument. `ValkeyModule_CallArgv`
borrows the caller's argv array directly — no allocation, no refcount
manipulation. Ownership of the array stays with the caller.
2. **The module wants to forward the reply to its own client without
inspecting it.** `ValkeyModule_Call` always parses the raw RESP bytes
into an intermediate `ValkeyModuleCallReply` tree and then re-serializes
it back to RESP to send to the caller. `ValkeyModule_CallArgv` exposes
the raw RESP bytes directly through a callback, allowing a pass-through
module command to copy bytes from the inner command's output buffer
straight into the outer client's output buffer.
---
## New API
### `ValkeyModule_CallArgv`
```c
int ValkeyModule_CallArgv(ValkeyModuleCtx *ctx,
ValkeyModuleString **argv,
int argc,
int flags,
ValkeyModuleReplyHandlers *reply_handlers,
void *reply_ctx);
```
Calls a server command using an existing argv array. The caller retains
ownership of `argv` and its elements — they are never freed or
ref-counted by the function.
Returns `VALKEYMODULE_OK` on success or `VALKEYMODULE_ERR` on error,
with `errno` set to the same values as `ValkeyModule_Call`. The function
does **not** return a `ValkeyModuleCallReply`; the reply is delivered
through `reply_handlers` instead.
`flags` is a bitwise OR of one or more `VALKEYMODULE_CALL_ARGV_*`
constants, which correspond directly to the format-string specifiers of
`ValkeyModule_Call`:
| Flag | Equivalent format char | Meaning |
|---|---|---|
| `VALKEYMODULE_CALL_ARGV_REPLICATE` | `!` | Propagate to replicas and
AOF |
| `VALKEYMODULE_CALL_ARGV_NO_AOF` | `A` | Skip AOF propagation |
| `VALKEYMODULE_CALL_ARGV_NO_REPLICAS` | `R` | Skip replica propagation
|
| `VALKEYMODULE_CALL_ARGV_RESP_3` | `3` | Force RESP3 for the inner call
|
| `VALKEYMODULE_CALL_ARGV_RESP_AUTO` | `0` | Match the calling client's
protocol (RESP2 or RESP3) |
| `VALKEYMODULE_CALL_ARGV_RUN_AS_USER` | `C` | Apply ACL checks for the
context user |
| `VALKEYMODULE_CALL_ARGV_SCRIPT_MODE` | `S` | Mark as script execution
|
| `VALKEYMODULE_CALL_ARGV_NO_WRITES` | `W` | Disallow write commands |
| `VALKEYMODULE_CALL_ARGV_ERRORS_AS_REPLIES` | `E` | Return error
replies instead of raising errno |
| `VALKEYMODULE_CALL_ARGV_RESPECT_DENY_OOM` | `M` | Honour deny-oom
policy |
| `VALKEYMODULE_CALL_ARGV_DRY_RUN` | `D` | Dry-run mode (implies
`ERRORS_AS_REPLIES`) |
| `VALKEYMODULE_CALL_ARGV_ALLOW_BLOCK` | `K` | Allow the inner command
to block |
| `VALKEYMODULE_CALL_ARGV_REPLY_EXACT` | `X` | Disable reply type
coercion |
---
### `ValkeyModuleReplyHandlers`
```c
typedef struct ValkeyModuleReplyHandlers {
/* Scalar types */
void (*null)(void *ctx);
void (*bulkString)(void *ctx, const char *str, size_t len);
void (*simpleString)(void *ctx, const char *str, size_t len);
void (*error)(void *ctx, const char *msg, size_t len);
void (*integer)(void *ctx, long long val);
void (*doubleVal)(void *ctx, double val);
void (*boolVal)(void *ctx, int val);
void (*bigNumber)(void *ctx, const char *str, size_t len);
void (*verbatimString)(void *ctx, const char *str, size_t len, const char *fmt);
/* Collection types — paired start/end callbacks */
void (*arrayStart)(void *ctx, size_t len);
void (*arrayEnd)(void *ctx);
void (*mapStart)(void *ctx, size_t len);
void (*mapEnd)(void *ctx);
void (*setStart)(void *ctx, size_t len);
void (*setEnd)(void *ctx);
void (*attributeStart)(void *ctx, size_t len);
void (*attributeEnd)(void *ctx);
/* Invoked on parse error */
void (*replyParsingError)(void *ctx);
/* Raw-bytes intercept — called before per-type callbacks */
int (*onRespAvailable)(void *ctx, ValkeyModuleCtx *module_ctx, const char *proto, size_t proto_len);
/* Blocking command support */
void (*onBlocked)(void *ctx, ValkeyModuleCtx *module_ctx, ValkeyModuleCallArgvBlockedHandle *handle);
void *context; /* Passed as first argument to every callback */
} ValkeyModuleReplyHandlers;
```
All fields are optional; set unused ones to `NULL`.
**`onRespAvailable`** is called first, before any per-type callback,
with the complete raw RESP reply in the `proto`/`proto_len` arguments.
Return `1` to continue into the per-type callbacks; return `0` to stop
after the raw bytes. Pass `NULL` to skip and go straight to per-type
dispatching.
**`onBlocked`** is only called when `VALKEYMODULE_CALL_ARGV_ALLOW_BLOCK`
is set and the inner command blocks. The `handle` can be passed to
`ValkeyModule_CallArgvAbort` to cancel the call. The handle is valid
until either `onRespAvailable` is invoked or
`ValkeyModule_CallArgvAbort` is called, whichever comes first.
---
### `ValkeyModule_CallArgvAbort`
```c
int ValkeyModule_CallArgvAbort(ValkeyModuleCallArgvBlockedHandle *handle);
```
Cancels a blocking call whose `onBlocked` callback has already fired.
Returns `VALKEYMODULE_OK` if the abort succeeded (neither
`onRespAvailable` nor any other reply callback will be invoked), or
`VALKEYMODULE_ERR` if the call has already completed.
---
### `ValkeyModule_ReplyRaw`
```c
int ValkeyModule_ReplyRaw(ValkeyModuleCtx *ctx, const char *proto, size_t proto_len);
```
Appends raw RESP bytes directly to the calling client's output buffer,
with no parsing or re-serialization. Intended for use inside an
`onRespAvailable` callback to implement zero-copy reply forwarding.
---
## Usage Example: `ValkeyModule_Call` vs `ValkeyModule_CallArgv`
The following two implementations of a generic pass-through proxy
command are functionally equivalent, but use different code paths
internally.
### With `ValkeyModule_Call` (existing API)
```c
int ProxyCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
if (argc < 2) return ValkeyModule_WrongArity(ctx);
/* VM_Call builds a new argv array internally, creates a string object for
* the command name, and calls incrRefCount/decrRefCount on every argument.
* The reply is parsed into a CallReply tree, then re-serialized to RESP. */
ValkeyModuleCallReply *reply =
ValkeyModule_Call(ctx, ValkeyModule_StringPtrLen(argv[1], NULL), "v",
argv + 2, (size_t)(argc - 2));
return ValkeyModule_ReplyWithCallReply(ctx, reply);
}
```
### With `ValkeyModule_CallArgv` and `onRespAvailable` (new API —
pass-through)
```c
static int forwardRawReply(void *ctx, ValkeyModuleCtx *mctx,
const char *proto, size_t proto_len) {
(void)ctx;
/* Write the raw RESP bytes directly to the outer client — no parse, no
* re-serialize, no intermediate allocation. */
ValkeyModule_ReplyRaw(mctx, proto, proto_len);
return 0; /* stop — do not invoke per-type callbacks */
}
static ValkeyModuleReplyHandlers passthrough_handlers = {
.onRespAvailable = forwardRawReply,
};
int ProxyCommand(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
if (argc < 2) return ValkeyModule_WrongArity(ctx);
/* VM_CallArgv borrows argv directly (no copy, no refcount changes).
* The reply is written straight into the outer client's output buffer
* through the onRespAvailable callback above. */
if (ValkeyModule_CallArgv(ctx, argv + 1, argc - 1,
VALKEYMODULE_CALL_ARGV_RESP_AUTO,
&passthrough_handlers,
NULL) == VALKEYMODULE_ERR) {
return ValkeyModule_ReplyWithError(ctx, "ERR command failed");
}
return VALKEYMODULE_OK;
}
```
Use `VALKEYMODULE_CALL_ARGV_RESP_AUTO` to ensure the inner command
replies in the same protocol version as the outer client (RESP2 or
RESP3), so the raw bytes can be forwarded without modification.
---
## Internal changes
### `argv_borrowed` client flag
A new `argv_borrowed` bit is added to the client flags struct. When
`VM_CallArgv` runs the inner command on a temporary client, it sets this
flag to signal that the temp client does not own the argv array.
`freeClientArgv` and `freeClientOriginalArgv` respect the flag and skip
freeing. `resetClient` clears it unconditionally after each command.
Several string commands (`SET`, `MSET`, `APPEND`, …) that previously
encoded or reused argv objects in-place are guarded so they do not
mutate argv elements they do not own.
A debug-only assertion (enabled when `enable-debug-assert yes` is set in
the config) verifies at runtime that no command modifies the argv array
or decrements any element's ref-count when `argv_borrowed` is set.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Signed-off-by: Ricardo Dias <rjd15372@gmail.com>
This commit introduces a new configuration option `cluster-config-save-behavior`
that controls how the cluster handles nodes.conf file save failures.
The option supports two modes:
- `sync` (default): Synchronously save the config file. If the save fails,
the process exits. This maintains backward compatibility with the old
behavior (before 9.1).
- `best-effort`: Synchronously save the config file. If the save fails,
only log a warning and continue running. This allows the node to survive
disk failures (e.g., disk full, read-only filesystem) without exitting,
giving administrators time to address the issue.
Note that this modifies the behavior of #1032, whereas #1032 was "best-effort",
we have now introduced a configuration option that defaults to "sync."
See #1032 discussion for more details.
Background:
When a disk becomes read-only or full, any cluster metadata change would
trigger a nodes.conf save attempt. With the old behavior, the node would
immediately exit via clusterSaveConfigOrDie(), potentially causing multiple
nodes on the same machine to crash simultaneously, leading to cluster
unavailability.
The new `best-effort` mode addresses this by allowing nodes to continue
operating even when disk writes fail. This is particularly useful in cloud
environments where disk failures are more common due to scale.
Note: Startup-time config saves (in clusterInit and verifyClusterConfigWithData)
still use clusterSaveConfigOrDie() since disk issues at startup should cause
immediate failure.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Previously, `redactClientCommandArgument()` forced a full copy of
`c->argv` into `original_argv` immediately, then replaced the sensitive
slot with `shared.redacted`. This meant any call to redact an argument
triggered an eager argv backup even when no rewriting was needed.
The new approach stores the indices of arguments to be redacted in a
small dynamic array (`redact_args`) on the client struct. Redaction is
applied lazily when the argv is actually consumed (e.g. by the command
log), avoiding the unnecessary argv copy on the hot path.
This change is required for the implementation of
`ValkeyModule_CallArgv` module API function (#3122) , because CallArgv
owns the `argv` array, which can't be modified by the redact function.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Two changes to tests/unit/cluster/faster-failover.tcl:
1. FAIL detection timeout: `wait_for_condition 1000 10` → `1000 50`
(10s → 50s)
2. psync_max_retries: 1200 → 2400 normal (120s → 240s),
6000 → 12000 valgrind (600s → 1200s)
The test `The best replica can initiate an election immediately in an
automatic failover` in `tests/unit/cluster/faster-failover.tcl` has been
flaky since it was introduced on March 27, 2026 by #2227.
**Frequency:** 8 out of 15 days (Mar 27 – Apr 8), across valgrind,
sanitizer, and slow CI runners.
**Common errors:**
- `log message of "Successful partial resynchronization with primary"
not found` (timeout waiting for psync)
- `expected pattern found in srv -N log file: *best ranked replica*`
(timeout waiting for FAIL propagation)
The test spins up a 12-node cluster (5 primaries + 7 replicas), pauses
nodes, and waits for FAIL detection to propagate across all nodes before
failover + partial resync.
A previous fix attempt #3424 increased the psync timeout from 50s to
120s (600s valgrind), which reduced frequency but did not eliminate it.
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
In rdbLoadObject, when sdstrynewlen is OK and hashtableAdd is OK,
but lpSafeToAdd is FAIL, field will be double-freed.
But lpSafeToAdd will only fail when len + add > LISTPACK_MAX_SAFETY_SIZE(1GB),
so it can rarely happened and trivial.
Signed-off-by: charsyam <charsyam@naver.com>
rewriteConfigFormatMemory() and rewriteConfigBytesOption() used long long
parameters to represent memory bytes, but configs like maxmemory are stored
as unsigned long long and allow values up to ULLONG_MAX.
When the value exceeds LLONG_MAX (e.g. 9223372036854775808), it overflows to
negative when passed as long long, causing config rewrite to produce incorrect
output like "maxmemory -8589934592gb".
Change both functions to use unsigned long long, matching the actual semantics
of memory byte values. This is consistent with how numericConfigGet() already
handles MEMORY_CONFIG using ull2string().
There are some MEMORY_CONFIG are signed and can be negative:
1. maxmemory-clients is a PERCENT_CONFIG
2. slot-migration-max-failover-repl-bytes is a SIGNED_MEMORY_CONFIG (after #3443)
None of them were affected:
```
static void numericConfigRewrite(standardConfig *config, const char *name, struct rewriteConfigState *state) {
...
if (config->data.numeric.flags & PERCENT_CONFIG && value < 0) {
rewriteConfigPercentOption(state, name, -value, config->data.numeric.default_value);
} else if (config->data.numeric.flags & SIGNED_MEMORY_CONFIG && value < 0) {
rewriteConfigNumericalOption(state, name, value, config->data.numeric.default_value);
} else if (config->data.numeric.flags & MEMORY_CONFIG) {
rewriteConfigBytesOption(state, name, value, config->data.numeric.default_value);
...
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
Increase time to wait for bgsave to start. This wait has been seen
failing in these test cases.
The test case loops with 'no', 'slow', 'fast', 'all' and 'timeout'
replicas, in tests/integration/replication.tcl
Example:
*** [err]: diskless fast replicas drop during rdb pipe in
tests/integration/replication.tcl
rdb child didn't terminate
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In valkey.conf, slot-migration-max-failover-repl-bytes allows setting
to -1 to disable the limit.
```
Setting this to -1 will disable this limit
```
But slot-migration-max-failover-repl-bytes is defined as MEMORY_CONFIG
and memtoull() rejects negative inputs, making it impossible to set the
value to -1 via config file or CONFIG SET.
```
>>> 'slot-migration-max-failover-repl-bytes "-1"'
argument must be a memory value
```
Introduce SIGNED_MEMORY_CONFIG flag for memory configs that also accept
plain negative number. When memtoull() fails and this flag is set, fall back to string2ll()
for parsing. Use ll2string() for CONFIG GET and rewriteConfigNumericalOption() for
CONFIG REWRITE when the value is negative.
Add a serverAssert in initConfigValues() to enforce that PERCENT_CONFIG
and SIGNED_MEMORY_CONFIG are never combined on the same config, since
both use negative values with different semantics.
This means we have had this issue since it was introduced in #1949.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
The faster-failover.tcl test starts 12 cluster nodes 5 times with TLS,
which is CPU intensive due to the volume of TLS handshakes. Running
alongside other tests on CI runners causes random I/O timeouts in
unrelated tests.
Tagging it as `tls:skip`
Signed-off-by: nmvk <r@nmvk.com>
Some test cases write thoughsands of commands in a pipeline and
afterwards read the replies. This can lead to TCP ACK being dropped and
the connection broken. CLIENT REPLY OFF prevents this.
"Main db not affected when fail to diskless load" in
cluster/diskless-load-swapdb has been observed to be flaky.
The others are just defensive but they follow the same pattern.
Similar fixes in the past: #3430, #2483.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fix two things:
1. Incorrect RDB object size reported when writing hash fields with
expiration, because of wrong position of parentheses. Affects the
serialized size reported in DEBUG OBJECT.
2. Memory leak when loading a zipmap (only used in really old RDB
versions).
Signed-off-by: charsyam <charsyam@naver.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.com>
This PR increases the embedded string threshold to 128 bytes (2 cache
lines) in order to improve memory overhead. I also fixed the tests that
pertained to embedded values.
Closes#3025
---------
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
valkey-cli and valkey-benchmark link only a small subset of .o files and
do not include cluster_migrateslots.o. At -O3 this works because the
compiler either inlines or discards the unused static function —
the symbol reference never reaches the linker. At -O0 (no inlining, no
dead-code elimination), the compiler emits the full body of
getClientType into every .o that includes server.h, producing an
unresolved
reference to _isImportSlotMigrationJob at link time.
fix is to avoid including server.h as part of external application
compilation dependency
fixes: https://github.com/valkey-io/valkey/issues/3415
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Replace 'const int seqBufferMaxLength' with a #define to avoid a
variable-length array warning (-Wgnu-folding-constant) in C.
Also add -Werror to the linenoise Makefile so future warnings are caught
at build time.
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Found while implementing `clusterNodeGetSlotRangeEnd` for `CLUSTERSCAN`
range bounded scanning, which uses the simlilar approach.
When tested on s390x via Docker and QEMU. Slot boundary came as 5440
instead of 5461.
```
127.0.0.1:6001> clusterscan 0-{06S}-0
1) "0-{4HD}-0"
2) 1) "{06S}key1"
127.0.0.1:6001> keyslot 0-{4HD}-0
(error) ERR unknown command 'keyslot', with args beginning with: '0-{4HD}-0'
127.0.0.1:6001> cluster keyslot 0-{4HD}-0
(integer) 5440
127.0.0.1:6001> cluster nodes
08e28d7e8dcfc731ac537d0518bfa32577da6ec7 127.0.0.1:6002@16002 master - 0 1774415944347 2 connected 5461-10922
53f6d4e61eee13ea98441eec05b3c4c95c6c83a4 127.0.0.1:6003@16003 master - 0 1774415943314 3 connected 10923-16383
46664ac624f001cc0708e8ddcfcc9e45e8f444a0 127.0.0.1:6001@16001 myself,master - 0 0 1 connected 0-5460
```
Updating here as it is same approach and does not follow `memrev64ifbe`
followed by `memcpy`
Signed-off-by: nmvk <r@nmvk.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Replace the dict.c implementation with a header-only wrapper (dict.h)
around the hashtable API. The dict types, iterators and API functions
are now typedefs, macros and inline functions that delegate to
hashtable. This unifies the hashtable implementations in the project and
removes duplicated logic.
Changes to dict:
- Remove dict.c; dict.h is now the entire implementation
- dict, dictType and dictIterator are direct aliases for the hashtable
counterparts.
- dictEntry is a struct allocated by dict wrapper functions to hold key
and value. It doesn't have a next pointer anymore.
- Fix key duplication for dictTypes that had keyDup callback by
calling sdsdup() at call sites in functions.c
- Remove unused functions, macros, includes and casts
- Move some dict defrag logic to defrag.c
- Remove obsolete dict unit tests (covered by test_hashtable.cpp)
Changes to hashtable:
- Change hashtable keyCompare convention to match dict: non-zero means
keys are equal, so existing dict compare functions can be reused
- Add const to hashtableMemUsage parameter
Changes to server implementation:
- Deduplicate common dict/hashtable callbacks in server.c
- Change configured hash-seed to only apply to data hashtables. In
particular, it must not modify the hash seed for dicts already
initialized during startup for reading configs and similar.
Changes to libvalkey:
- Let libvalkey use its own dict implementation.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fixing multiple flaky tests.
slave buffer are counted correctly in tests/unit/maxmemory.tcl
Memory efficiency with values in range * in tests/unit/memefficiency.tcl
These tests send large numbers of pipelined commands using deferring
clients without reading replies, causing the server's client output
buffer to grow. On slow CI runners, this leads to TCP backpressure and
I/O errors that crash the test runner. Fix: Use CLIENT REPLY OFF to
suppress reply generation, matching the pattern from commit 972f94953c.
---
Sub-replica reports zero repl offset and rank, and fails to win election
in tests/unit/cluster/replica-migration.tcl
New non-empty replica reports zero repl offset and rank, and fails to
win election in tests/unit/cluster/replica-migration.tcl
In the replica-migration tests, a MOVED errors results in an Tcl
exception. After failover, wait_for_condition blocks issue GET commands
to cluster nodes that may not have fully updated their slot routing. An
unhandled MOVED exception crashes the test runner. Fix: Wrap the
condition in catch so MOVED errors are retried. Also wrap debug prints
in the else clause. Fixes the following tests:
---
Replica can update the config epoch when trigger the failover -
automatic in tests/unit/cluster/failover2.tcl
Increase wait timeout for failover expiry. The test waits 10 seconds for
"Failover attempt expired", but the default cluster-node-timeout in
start_cluster is 3000ms, making auth_timeout 6 seconds plus ~3
seconds for failure detection — barely fitting in 10 seconds and failing
on slow CI runners. Fix: Increase wait from 1000×10ms to 1200×50ms
(60 seconds).
---
dual-channel-replication lazyfree test
The test looks up the replica's main-channel connection id after writing
50MB of data. On slow CI runners, the replica connection may have been
disconnected by the output buffer soft limit (64MB/60s) before the
lookup, causing get_client_id_by_last_cmd to return empty. Two changes:
1. Move the connection id lookup before the write loop, while the sync
is known to be in progress.
2. Reduce writes from 50 x 1MB to 10 x 1MB. The test only needs enough
data to exceed the lazyfree threshold (64 blocks ~= 1MB). 10MB is
sufficient and avoids approaching the output buffer limit.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
## Summary
This PR handles a network race condition that would cause a replica to read stale cluster
packet and then incorrectly promote itself to an empty primary within an existing shard.
## Issue
"Migrated replica reports zero repl offset and rank, and fails to win election - sigstop"
the test case in `replica-migration.tcl` is a known example where the network race condition
can occur.
Here's the timeline:
- T1: Slot migration — R7 and R3 both try to replicate from R0. Only R3 succeeds, triggering
a BGSAVE on R0, while R7 blocks in `receiveSynchronousResponse()`.
- T2: While R7 is blocked, R0 is SIGSTOP'd by the test. R4 wins the election and becomes the
new primary. R4 sends PINGs to R7 - These PINGs land in R7's kernel TCP receive buffer but
are not read by R7 yet.
- T3 (5s after T1, due to receiveSynchronousResponse): R7 wakes up:
- It reads from inbound links and finds out the remote nodes have already closed their end
(they detected R7 as FAIL), getting "I/O error: connection closed" on each. R7 calls `accept()`
for the new connections that were established during the block. These connections carry data
that was sent seconds ago while R7 was dead.
- R7's outbound links are still valid, so it sends PING to R4.
- T4: R7 receives and processes fresh PONG packet from R4 on outbound link, reconfiguring itself
to follow R4.
- T5: R7 reads stale PING packet of R4 via inbound link. This packet was generated R4 was following
R0, so R7 incorrectly believes R4 is still following R0 now. And Reconfiguring itself as a replica
of R0 from R4.
- T6: R7 finds R0 is FAIL, so it starts an election and wins, and becomes an empty-primary.
## Analyze
So in T4, R4 is the new primary, and R7 is reconfiguring itself as a replica. So in R7's view,
R4 is the primary, and myself (R7) is a replica.
And in T5, there is a stale packet from sender (R4), the stale packet is saying: sender (R4) is
a replica and R0 is the primary.
We originally had a logic for stale packet, meaning we would try to ignore stale packet that
would cause exceptions. So in T5:
- sender_claims_to_be_primary is false since R4 is saying it is a replica.
- sender_last_reported_as_primary is true since in R7's view, R4 is a primary.
- sender_claimed_primary is R0, and sender (R4) and sender_claimed_primary (R0) is in the same shard.
- nodeEpoch(sender_claimed_primary) is R0's epoch. R0 is an old and dead (not yet) primary.
- sender_claimed_config_epoch is R0's epoch since R4 is a replica, and R0 is R4's primary.
- nodeEpoch(sender_claimed_primary) == sender_claimed_config_epoch, so the logic fail and we process
a stale packet. So in this point, the packet is not a stale packet in R4 and R0's view.
- But it is a stale packet in myself (R7) view. In R7's local view, R4 is the new primary and it
should have a bigger epoch, that is nodeEpoch(sender) should > sender_claimed_config_epoch.
## Fix
The PR fixes the issue by enhancing the existing guardrail logic against stale packet. Previously
that logic only detects `nodeEpoch(sender_claimed_primary) > sender_claimed_config_epoch` as stale
packet, now it also checks `nodeEpoch(sender) > sender_claimed_config_epoch` to make sure we have
up-to-date primary-replica chain.
Signed-off-by: Zhijun <dszhijun@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
The test case "The best replica can initiate an election immediately
test" has been failing in CI jobs.
Increase the timeout to account for slow runners.
Old waiting time: 50 seconds. New waiting time: 120 seconds, with
valgrind: 600 seconds.
Intoduced in #2227.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In Daily test runs with the `--accurate` flag, the corrupt-dump-fuzzer
test runs for 10 minutes (600 seconds) with "sanitize_dump: no" and then
another 10 minutes with "sanitize_dump: yes". This causes the runner to
time out the whole test job to be aborted with a sigterm from the
runner.
Example:
13697:signal-handler (1774917673) Received SIGTERM scheduling
shutdown...
This change reduces this hard-coded fuzzer run from 10 to 1 minute. We
have many tests jobs so the fuzzer gets plenty of time to run anyway.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Previously, watchForKey() checked for duplicate watched keys by iterating
through the client's entire watched_keys list with O(N) complexity, where
N is the total number of keys watched by the client. So the time complexity
for the WATCH command could be quite poor and become a slow command.
This commit introduces a per-db hashtable (watched_keys_by_db) in the
client's multiState structure to enable O(1) duplicate key detection.
The hashtable is lazily allocated only when the client starts watching
keys, minimizing memory overhead for clients that don't use WATCH.
The per-db hashtable stores watchedKey* directly as the hashtable entry
since it already contains the key, so no custom destructors are needed.
Memory management remains centralized in the watched_keys list.
This optimization is especially beneficial when a client watches many
keys across different databases, as the check no longer scales with
the total watched key count.
This might be a minor scenario, but there's no harm in optimizing it.
There is a test in multi.tcl, before this patch, it took 15s, and after
this patch, it only took 50ms.
```
set elements {}
for {set i 0} {$i < 50000} {incr i} {
lappend elements key-$i
}
r watch {*}$elements
r watch {*}$elements
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
To avoid freeing the cluster link when EAGAIN occurs,
so that we can try again and keeping the send messages.
Signed-off-by: Binbin <binloveplay1314@qq.com>
The RXE project should keep the same version with the CI machine,
showing uname in RDMA CI job to find out the reason of kmod installing
failure.
Signed-off-by: zhenwei pi <zhenwei.pi@linux.dev>
These failures seem to be attributed to a race condition in the Aborted
test case.
`rdb-key-save-delay` 10000 was being set after `$master exec` triggered
the full resync. Since repl-diskless-sync-delay 0 was set, the master
would immediately start streaming the RDB to the replica once it
reconnected. On the ARM runner, when it's fast enough, the entire RDB
generation and transfer could complete in ~78ms, before the delay was
ever applied. This meant the replica would complete the swap and have
1010 keys instead of the expected 200 and there would be no
async_loading window to observe or abort which led to the failures.
We saw this in the daily test failure logs
```
92948:S * RDB memory usage when created 110.85 Mb
92948:S * Done loading RDB, keys loaded: 1010, keys expired: 0
```
The fix moves `rdb-key-save-delay 10000` to before `$master exec` to
guarantee the delay is in effect on the master before the RDB generation
begins.
Closes#3394, closes#3395.
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
In #2023 (#2209, etc.), we are exploring ways to make failover faster,
that is, to minimize the delay.
When a node is marked as FAIL and before the failover starts, there is a
delay of 500-1000ms. The original purpose of this delay:
1. Allow FAIL to propagate to at least a majority of the primaries. This
makes sure they will vote when a replica sends failover auth request.
2. Allow replicas to exchange their offsets, so they will have a correct
view of their own rank.
We want to minimize this delay while ensuring safety. It is useful for
example in these cases:
1. If there is only one replica, then we don't need any delay, or
2. If there are more replicas, with a fast network, the replicas can
exchange the offsets very quickly and start the failover within a few
milliseconds instead of 500-1000ms.
In this PR, when we can be sure that the replica is the best ranked
replica, we let
it initiate a failover immediately and completely remove the delay.
### How to ensure safety?
1. To make sure this replica has the best rank, it only skips the delay
if it is sure that it have the best rank and that all replicas in the
same shard agree that the primary is failing. A new flag
`CLUSTER_NODE_MY_PRIMARY_FAIL` is introduced to indicate that each
replica has marked its primary as FAIL. If all replicas say that the
primary is failing, we also know that the offset is not updated,
because the offset is not incrementing when the primary is failing.
We can skip the delay only if we have received a message from all
replicas and they all have set this flag.
2. To make sure the primaries will vote even if they didn't receive the
FAIL yet, we use the `CLUSTERMSG_FLAG0_FORCEACK` to make sure they
will vote. This is equivalent to broadcasting a FAIL message to all
primaries before we broadcast the failover auth request (but
cheaper).
The race between FAIL (broadcast by A) and AUTH REQUEST (broadcast by R)
is illustrated in the following sequence diagram:
```
A R B C
| | | |
| FAIL | | |
|----->| AUTH R.| |
| |------->| |
| FAIL | | |
|-------------->| |
| | AUTH R.| |
| |-------------->|
| FAIL | | |
|--------------------->|
```
### Details
This is the how the failover is initiated, with new steps marked with
**(new)**:
1. A majority of primaries have marked another primary as PFAIL.
2. Some nodes counts failure reports and marks the failing primary as
FAIL. The node
that detects FAIL broadcasts it to all nodes in the cluster.
3. When a replica receives FAIL (or detects FAIL itself by counting
PFAIL reports) it schedules a failover:
a. It sets a timeout (500ms + random 0-500ms).
b. It broadcasts pong to the other replicas in the same shard.
c. **(new)** The pong (actually the clusterMsg header) has a new flag
`CLUSTER_NODE_MY_PRIMARY_FAIL`. When the replicas broadcast pong
to each other here, this flag is set.
4. **(new)** When the following conditions are met, skip the remaining
delay and start the failover using AUTH REQUEST with the FORCE ACK
flag set, that is if
a. a PONG is received from every other replica in the same shard
(broadcast within the shard) and
b. all replicas have marked that its primary is FAIL in their last
message (the new `CLUSTER_NODE_MY_PRIMARY_FAIL` flag is set) and
c. this is the best replica (rank = 0) and
d. my replication offset != 0.
5. When the delay has passed and no other replica has initiated
failover, then initiate failover.
Notes:
* With 3(c), we don't need to wait for FAIL to propagate to all voting
primaries. At this point, a FAIL has already been broadcast by some
node, but there is a race so our auth request may arrive to some node
before the FAIL. Using the FORCE ACK flag ensures the primaries will
vote for us. (It is equivalent to broacasting another FAIL just before
broadcasting auth request.)
* 4(b) ensures that we have received the replication offset from all
other replicas and that it's up to date. If a replica says that it's
primary is failing, it also means that the replication from the
primary to that replica has stopped.
* 4(c) is to avoid a special bad case. It can happen that not all
replicas know about each other. In this case, two replicas can think
they are both the best replica and start the failover at the same
time. This can already happen without this PR. When it happens, it
usually means that a new replica has just joined and it has no data
(offset = 0) and if it wins the election, there is a problem of
dataloss (discussed and partially mitigated for the replica migration
case in #885). To avoid this case, skip this fast failover path if the
replica has offset = 0.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
## Problem
The `EntryTest.entryUpdate` unit test fails on macOS with (mentioned in
#3200):
Expected: (entryMemUsage(e10)) < (current_embedded_allocation_size * 3 /
4)
actual: 48 vs 48
## Root Cause
`entryMemUsage` for embedded entries reflects the actual zmalloc
allocation size, which depends on the platform allocator's bucket sizes.
Valkey's bundled jemalloc is configured with `LG_QUANTUM=3` (8-byte
granularity), giving size classes: 8, 16, 24, 32, 40, 48, 56, 64, ...
However, macOS libc uses 16-byte aligned buckets: 16, 32, 48, 64, 80,
...
The test's value10 (21 chars) produces an entryReqSize of 40 bytes.
Jemalloc has a 40-byte size class, so entryMemUsage returns 40. macOS
rounds up to 48, which equals 3/4 of e9's 64-byte allocation, causing
the strict less-than assertion to fail.
## Fix
Shrink value10 from 21 to 13 characters, reducing entryReqSize from 40
to 32 bytes. Both allocators have a 32-byte bucket, and 32 < 48 holds on
both platforms.
## Test
All tests pass on macOS.
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
The `valkey-cli --cluster del-node` command fails when attempting to
delete unreachable or failed nodes, reporting `No such node ID` even
though the node exists in the cluster topology.
The root cause is the command only loads information about reachable
nodes, causing the lookup to fail. This PR added a new function for
loading all nodes information to solve this.
### Implementation
1. Loading all nodes from gossip:
- Added `clusterManagerLoadAllInfoFromNode()` that loads both reachable
and unreachable nodes from cluster gossip
- Extracts common logic into `clusterManagerLoadInfoCommon()` with an
`include_unreachable` flag
- Keeps the original `clusterManagerLoadInfoFromNode()` unchanged to
avoid affecting existing callers
2. Added success message to be consistent with other cluster commands:
`[OK] Node <id> removed from the cluster.`
3. Added test coverage for `del-node` which previously had none.
4. Load slot information from gossip for unreachable nodes in
`clusterManagerNodeLoadInfo()`
5. Skip unreachable primaries in `clusterManagerNodeWithLeastReplicas()`
### Testing
```
./runtest --single unit/cluster/cli
[ok]: del-node: Cannot delete node with slots (9 ms)
[ok]: del-node: Delete reachable node without slots (23 ms)
[ok]: del-node: Delete unreachable node without slots (1333 ms)
[ok]: del-node: Cannot delete unreachable primary with slots (3368 ms)
```
```
valkey-cli --cluster del-node 127.0.0.1:7000 eb837ea7c48908e5304eafd8b1b3ced57147c448
Could not connect to Valkey at 127.0.0.1:7002: Connection refused
>>> Removing node eb837ea7c48908e5304eafd8b1b3ced57147c448 from cluster 127.0.0.1:7000
>>> Sending CLUSTER FORGET messages to the cluster...
>>> WARNING: Could not connect to node 127.0.0.1:7002, unable to send CLUSTER RESET.
[OK] Node eb837ea7c48908e5304eafd8b1b3ced57147c448 removed from the cluster.
```
### Behavior change
Before
```
$ valkey-cli --cluster del-node <entry-node-ip>:<entry-node-port> <failed-node-id>
Could not connect to Valkey at <target-node-ip>:<target-node-port>: Connection refused
>>> Removing node <id> from cluster <entry-node-ip>:<entry-node-port>
[ERR] No such node ID <id>
```
After
```
$ valkey-cli --cluster del-node <entry-node-ip>:<entry-node-port> <failed-node-id>
Could not connect to Valkey at <target-node-ip>:<target-node-port>: Connection refused
>>> Removing node <id> from cluster <entry-node-ip>:<entry-node-port>
>>> Sending CLUSTER FORGET messages to the cluster...
>>> WARNING: Could not connect to node <target-node-ip>:<target-node-port>, unable to send CLUSTER RESET.
[OK] Node <id> removed from the cluster.
```
### Related Issue
Fixes https://github.com/valkey-io/valkey/issues/3208
---------
Signed-off-by: Yang Zhao <zymy701@gmail.com>
Upload the entire results directory instead of only metrics JSON files.
This includes server logs which are useful for debugging benchmark
failures.
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Pin package manager dependencies in CI workflows to improve the Pinned-Dependencies
score in OpenSSF Scorecard.
Changes:
- benchmark-on-label.yml, benchmark-release.yml: add `--require-hashes`
to `pip install` adding on valkey-perf-benchmark repo:
https://github.com/valkey-io/valkey-perf-benchmark/pull/44
- ci.yml: pin `yamlfmt` to `v0.21.0` instead of `@latest`
- reply-schemas-linter.yml: use npm ci with `package-lock.json` instead
of unpinned npm install, package files in `utils/reply-schema-linter/`
Signed-off-by: Roshaan Khatri <rvkhatri@amazon.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Previously, our workflow used a global concurrency group, which
effectively limited execution to one running job and one pending job.
Any additional requests were automatically canceled, preventing a true
queue from forming.
We are now shifting to a model where we remove the concurrency
restriction and allow jobs to queue directly on the self-hosted runner.
This enables multiple workflow runs to be accepted and queued instead of
being dropped.
While GitHub can accept workflow triggers at a high rate (e.g., hundreds
per minute), the actual execution is still constrained by runner
capacity, in our case, a single runner processing one job at a time.
However, queued jobs are subject to GitHub’s 24-hour timeout policy.
This means any job that waits in the queue for more than 24 hours before
starting will be automatically canceled (timedout).
In practical terms, this approach improves reliability by eliminating
premature cancellations, but the effective queue size is still bounded
by how many jobs the runner can process within a 24-hour window. we
could increase the number of runners to run these in parallel.
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
When a MATCH pattern maps to a specific slot, `CLUSTERSCAN` can skip
directly to that slot
instead of walking through all slots one by one.
- On `cursor 0`, starts directly at the matching slot
- If cursor is behind the matching slot, jumps forward
- If cursor is ahead of the matching slot, we conclude the scan as we
cannot match keys
- If both SLOT and MATCH are provided but target different slots,
returns 0 immediately
Signed-off-by: nmvk <r@nmvk.com>
The daily workflow was directly invoking the `valkey-unit-gtests` executable.
The intended invocation is to use `gtest-parallel` to ensure that the tests are executed in isolation.
Signed-off-by: harrylin98 <harrylin980107@gmail.com>
I'm developing a module to provide luajit as lua execution engine.
See #1229 for details.
Unfortunately said module doesn't support debugging yet. So in order to
test it with valkey tests scripting debug tests need to be skipped. This
patch makes an anonymous test skippable by name.
Signed-off-by: secwall <secwall@yandex-team.ru>
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.
This was introduced in #1405.
Signed-off-by: Binbin <binloveplay1314@qq.com>
`weekly.yml` calls `daily.yml` with `use_git_ref` set to each release
branch (for example 7.2). But the checkout logic in `daily.yml` only
used `inputs.use_git_ref` when `github.event_name` was
`workflow_dispatch` or `workflow_call`. otherwise it fell back to
`github.ref`.
For reusable workflows, GitHub keeps the caller workflow’s github
context. That means when `weekly.yml` is triggered by schedule, the
called `daily.yml` still sees `github.event_name == 'schedule'` and
`github.ref` for the caller branch (unstable). As a result, jobs labeled
as release-branch runs could still check out unstable.
Added a guard for Gtest Unit Tests. It will skip the job if gtest is not
available / supported.
Run with CI Issue:
https://github.com/valkey-io/valkey/actions/runs/22815380713
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Carries on from where #3161 left off. The test-sanitizer-address-large-memory
jobs were being OOM-killed on GitHub-hosted runners (15.6GB RAM) due to
ASAN's 2-3x memory overhead.
Changes:
- Skip 4GB quicklist compression test under ASAN (requires ~16-24GB with
dual buffers + ASAN overhead)
- Reduce integration test sizes from 5GB to 4.1GB (preserves >4GB 32-bit
boundary coverage)
- Reduce XADD iterations from 10 to 3
- Add memory monitoring to track minimum free memory during CI runs
Signed-off-by: Rain Valentine <rsg000@gmail.com>
The "New Master down consecutively" test was sometimes failing under
Valgrind by timing out. The new overrides match those used for the
cluster in the first part of the file - see #2672
Under Valgrind's 10-20x slowdown, a single failover requiring ~15
seconds of server time can exceed the test's 100-second wall-clock wait.
Error text:
```
*** [err]: New Master down consecutively in tests/unit/cluster/slave-selection.tcl
No failover detected when master 12 fails
```
Daily run failure:
https://github.com/valkey-io/valkey/actions/runs/22421982161/job/64921545936#logs
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Rain Valentine <rainval@amazon.com>
When a replica finishes a **disk-based** full sync and both `appendonly
yes` and `aof-use-rdb-preamble yes` are enabled, this PR reuses the
received `dump.rdb` as the new AOF base file, instead of triggering
`BGREWRITEAOF`.
This avoids generating an almost identical base snapshot and removes
redundant CPU/IO work.
Before:
1. Replica receives RDB via disk-based full sync
2. Replica loads RDB into memory
3. Replica may delete synced RDB (`rdb-del-sync-files`)
4. `BGREWRITEAOF` runs and generates a new base snapshot
5. New snapshot becomes AOF base
After:
1. Replica receives RDB via disk-based full sync
2. Replica loads RDB into memory
3. Synced RDB is renamed to the new AOF base file
4. A new incremental AOF is created
5. No `BGREWRITEAOF` is needed
Tests:
- [x] Disk-based sync + preamble on: RDB reused as AOF base
- [x] New writes after sync: incr AOF works correctly
- [x] Replica restart: reused AOF loads correctly
- [x] Disk-based sync + preamble off: fallback to `BGREWRITEAOF`
- [x] Diskless sync + preamble on: fallback to `BGREWRITEAOF`
- [x] Diskless sync with stale local `dump.rdb`: stale file is not
reused
---------
Signed-off-by: Ray Cao <zisong.cw@alibaba-inc.com>
`availability_zone` was added to the HELLO command in #1487, and
it was missing the reply schema and the test was wrongly marked
with `logreqres:skip`.
`availability-zone` was added to CLUSTER SHARDS and CLUSTER SLOTS
commands in #3156, and it was missing the reply schema.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Fixes the failing run:
```
[err]: Force the use of IO threads and assert active IO thread usage in tests/unit/io-threads.tcl
Expected '0.000000' to be more than '0' (context: type source line 49 file /Users/runner/work/valkey/valkey/tests/unit/io-threads.tcl cmd {assert_morethan $used_active_time 0} proc ::test)
```
The change deflakes the test by generating enough I/O-thread work.
The test previously created 16 deferred clients but only sent 15 total
`INCR` commands. On fast runners, that burst could be short enough that
`used_active_time_io_thread_*` was still reported as `0.000000`, causing
the test to fail even though I/O threads had run.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
- Supports `VALKEYMODULE_CONFIG_UNSIGNED` and `UNSIGNED_CONFIG` for a
wider range of configurations.
- Supported the API for `ValkeyModule_RegisterUnsignedNumericConfig`
- `string2ull` method supports the length parameter, making it more
secure
---------
Signed-off-by: artikell <739609084@qq.com>
Signed-off-by: skyfirelee <739609084@qq.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
There is now a port of fast_float in C. So instead of having an optional
fast_float dependency, we can just use ffc instead, unconditionally.
https://github.com/kolemannix/ffc.h
It is a high quality port. The performance should be the same or
improved.
Note : I am the maintainer and main author of fast_float.
---------
Signed-off-by: Daniel Lemire <daniel@lemire.me>
With current Makefile, `LDFLAGS` are not used for modules.
This results in security options not applied.
```
$ annocheck /usr/lib64/valkey/modules/rdma.so
annocheck: Version 12.99.
Hardened: rdma.so: FAIL: bind-now test because not linked with -Wl,-z,now
Hardened: Rerun annocheck with --verbose to see more information on the tests.
Hardened: rdma.so: Overall: FAIL.
```
With this patch
```
$ annocheck /usr/lib64/valkey/modules/rdma.so
annocheck: Version 12.99.
Hardened: rdma.so: PASS.
```
Signed-off-by: Remi Collet <remi@remirepo.net>
If the command is in the form of "SET key value EX/PX/EXAT ttl",
then we don't need to rewrite the entire command vector.
In addition to saving the rewriting of the command vector, we also
save one command table lookup in this case ince we don't need to
lookup SET command again.
We already have many relevant test coverages in expire.tcl. This
PR does not change anything around the propagate.
Somehow like 8c03ef7d06.
Signed-off-by: Binbin <binloveplay1314@qq.com>
In flushall we will call killRDBChild to kill the RDB child, but
we need to wait for checkChildrenDone and waitpid to kick in, so
that we can call resetChildState to reset the state. The previous
wait_for_condition 10 100 (1s) might not be enough, changed it to
wait_for_condition 50 100 (5s).
```
[err]: Test FLUSHALL aborts bgsave in tests/integration/rdb.tcl
bgsave not aborted
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
`zdiffAlgorithm2()` can break out early once the destination cardinality
reaches zero. In that path, a temporary SDS created by `zuiSdsFromValue()`
was left dirty and never released, because that cleanup normally happens on
the next iterator step.
This patch explicitly discards the dirty value before the early `break`.
```
==11724== 11 bytes in 1 blocks are definitely lost in loss record 36 of 1,442
==11724== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==11724== by 0x2FA974: ztrymalloc_usable_internal (zmalloc.c:156)
==11724== by 0x2FAAEA: zmalloc_usable (zmalloc.c:200)
==11724== by 0x282D25: _sdsnewlen (sds.c:102)
==11724== by 0x2830B2: sdsnewlen (sds.c:169)
==11724== by 0x2DB537: zuiSdsFromValue (t_zset.c:2290)
==11724== by 0x2DBE00: zdiffAlgorithm2 (t_zset.c:2502)
==11724== by 0x2DC085: zdiff (t_zset.c:2568)
==11724== by 0x2DD01F: zunionInterDiffGenericCommand (t_zset.c:2817)
==11724== by 0x2DD574: zdiffCommand (t_zset.c:2898)
==11724== by 0x29F5AE: call (server.c:3883)
==11724== by 0x2A1598: processCommand (server.c:4569)
```
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthakaggarwal97@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Implemented a way to propagate az through gossip and obtain az
information
through the CLUSTER SHARDS and CLUSTER SLOTS commands. It will only be
displayed if the node is configured with it.
Closes#3110
---------
Signed-off-by: Su Ko <rhtn1128@gmail.com>
It's a combination of MSET and EXPIRE, which allows us to set an
expiration
time simultaneously with the MSET operation.
Syntax for the new MSETEX command:
```
/* MSETEX numkeys key value [key value ...] [NX | XX]
* [EX seconds | PX milliseconds |
* EXAT seconds-timestamp | PXAT milliseconds-timestamp | KEEPTTL] */
```
The MSETEX command supports a set of options (like SET command):
- EX seconds -- Set the specified expire time, in seconds (a positive
integer).
- PX milliseconds -- Set the specified expire time, in milliseconds (a
positive
integer).
- EXAT timestamp-seconds -- Set the specified Unix time at which the
keys will
expire, in seconds (a positive integer).
- PXAT timestamp-milliseconds -- Set the specified Unix time at which
the keys
will expire, in milliseconds (a positive integer).
- KEEPTTL -- Retain the time to live associated with the keys.
- NX -- Only set the keys if all keys do not already exist.
- XX -- Only set the keys if all keys already exists.
Return Value (like MSETNX command):
- Integer reply: 0 if no key was set due to NX or XX options.
- Integer reply: 1 if all the keys were set.
Closes#2592
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Observation around `dictGetSomeKeys(N)` performance is that it is around
10x faster than randomly picking `N` elements over N*3 attempts.
Further, this avoids chances of not picking N elements due to duplicate
element selection with individual random pick. Overall, it should help
picking gossip node(s) for cluster bus message faster.
---------
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
This PR introduces a new config `cluster-message-gossip-perc` which
allows an operator to modify the amount of gossip node information to be
sent per ping/pong/meet message. It can be modified dynamically (Related
to https://github.com/valkey-io/valkey/issues/2291). The default value
is 10% i.e. 10% of peer node information would be gossiped along with
each ping/pong/meet packet. Users can tune this configuration, setting
the value higher allows faster information dissemination whereas setting
it lower would lead to direct PING messages if no information was
received about a node with the `server.cluster_node_timeout/2` period.
Note: the behavior for partially failed gossip nodes still remains
intact where all the `pfail` nodes are part of the message for faster
propagation of information and faster transition of `PFAIL` to `FAIL`.
---------
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
Now we will be able to add a `run-cluster-benchmark` label to run a
benchmark with cluster-mode enabled valkey-server
It will use the config
https://github.com/valkey-io/valkey/blob/unstable/.github/benchmark_configs/benchmark-config-arm.json
modified for for cluster mode with a single clustermode enabled instance of
valkey.
It uses the same single instance for the benchmark as for run-benchmark.
If both labels are used, they are sequential in the same concurrency group `group:
ec2-al-2023-pr-benchmarking-arm64`.
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Addresses issue #2350
As noted in the issue, much of the expire code uses raw long long for
timestamps, which provides no semantic meaning about the unit or purpose
of the value. Valkey already defines mstime_t (milliseconds) and
ustime_t (microseconds) typedefs — this PR replaces bare long long
declarations with the appropriate typedef wherever the value represents
an expiration timestamp or time duration.
This PR only fixes a small subset in the codebase, but it is an
incremental step toward fully replacing the bare long long references.
---------
Signed-off-by: curious-george-rk <r.ebu@gmail.com>
Co-authored-by: curious-george-rk <r.ebu@gmail.com>
Before this change, if you built `valkey-server` (e.g. `make
valkey-server`) the LUA module was not built. With this change, the LUA
module is now a direct a dependency of `valkey-server`
- (unless `BUILD_LUA=no` is passed)
Added some colors to the Lua module & Lua lib `Makefile`s to they blend
nicely in the build output.
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Implemented CLUSTERSCAN command for topology-aware scanning
Unlike `SCAN` which is local to a single node, `CLUSTERSCAN` provides a
mechanism that helps clients iterate across slot boundaries and handles
`MOVED` redirections.
**Key details**
* Global cluster iteration via `fingerprint-{hashtag}-cursor`
* Scan one slot at a time
* Start the CLUSTERSCAN with 0
* SLOT argument for parallel scanning of multiple slots
* Re-use scanGenericCommand for the response
**Cursor format:** `fingerprint-{hashtag}-localcursor`
- Fingerprint is a hash of the node's DB seed that identifies the
current memory layout. On mismatch, scan restarts from cursor 0
rather than returning an error.
- Fingerprint 0 indicates a cross slot cursor (e.g., initial cursor
or slot transition) where validation is skipped.
- Hashtag encodes the target slot
- Local cursor tracks position within the slot
**Usage:**
```
CLUSTERSCAN <cursor> [MATCH pattern] [COUNT count] [TYPE type] [SLOT number]
```
```
CLUSTERSCAN 0 # Start scanning from slot 0
CLUSTERSCAN <cursor> # Continue from cursor
CLUSTERSCAN 0 SLOT 1000 # Start scanning specific slot
CLUSTERSCAN <cursor> MATCH user:* COUNT 100
```
---------
Signed-off-by: nmvk <r@nmvk.com>
Signed-off-by: Raghav <r@nmvk.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Currently after changing cluster-replica-no-failover, we will
call clusterUpdateMyselfFlags to trigger CLUSTER_TODO_SAVE_CONFIG.
But in gossip when the sender's NOFAILOVER changes, we won't
trigger the save, this cause nodes.conf to not save the latest
nofailover flag.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Replace integer mono_ticksPerMicrosecond with fixed-point arithmetic in
x86 TSC monotonic clock for better accuracy (and performance because of
multiplication + shift instead of division).
Calibration now uses double precision to compute multiplier.
**Comparison with TSC @ 2400.5 ticks/us**
**Old Method:**
Calibration: 2400 (truncated from 2400.5)
Converting 1 second of ticks: 2,400,500,000 / 2400 = 1,000,208 us
Error: +208 us per second
**New Method:**
Calibration: sample_ticks_per_us = 2400.5 (double stores exactly in this
case)
Multiplier: 2^24 / 2400.5 = 6989.942 -> stored as 6989 in uint64_t
Converting 1 second: (2,400,500,000 * 6989) >> 24 = 999,992 us
Error: -8 us per second
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Since there is some mismatch between the already installed `ar` tool on
a macOS runner
and Clang 22, installed by brew; lets use the brew installed `llvm-ar`.
Expected to fix the issue in CI job `build-macos-latest`.
---------
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
In #3260 we try to deflake it by using a assert_range, but the upper
limit of the range is low so the test is still flaky. Increase the upper
limit to 200ms more than the expected latency, e.g. from 550 to 700 when
the expected latency is 500.
```
*** [err]: LATENCY GRAPH can output the event graph in tests/unit/latency-monitor.tcl
Expected '625' to be between to '450' and '550' (context: type source line 143 file /Users/runner/work/valkey/valkey/tests/unit/latency-monitor.tcl cmd {assert_range $high 450 550} proc ::test)
```
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Before we added LUA as a module we had a logic to NOT register the
luaHook when the value of `busy-reply-threshold` config is set to 0.
Now we ALWAYS register the hook and in order to keep aligned with
old behavior we will let the execution of the script continue from
the interrupt hook when `busy-reply-threshold` config is set == 0.
And in value.conf, we are saying the config can be negative, but in
fact the config is minimum at 0, fix the valkey.conf as well.
```
# The default is 5 seconds. It is possible to set it to 0 or a negative value
# to disable this mechanism (uninterrupted execution)
```
It was introduced in #2858.
Signed-off-by: Alon Arenberg <alonare@amazon.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
This PR fixes a Codecov workflow misconfiguration introduced when
upgrading codecov/codecov-action from v4 to v5 (in #3185).
In v5, the action expects files (plural), but the workflow still used
file.
The coverage shown is 0 right now:
https://app.codecov.io/gh/valkey-io/valkey
Documentation from -
https://github.com/codecov/codecov-action/tree/v5?tab=readme-ov-file#arguments
```
The following arguments have been changed
file (this has been deprecated in favor of files)
plugin (this has been deprecated in favor of plugins)
```
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
as part of #2508 we introduce a defrag optimization to avoid matching
the replaced defrag entry.
Even though defrag replacements does not impact the skip list order,
when scores are equal, we MUST compare elements lexicographically to
maintain correct skip list ordering.
Otherwise we might miss locating the entry.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
`SSL_get0_peer_certificate()` was introduced in OpenSSL 3.0. The recent
commit 136852efc (Support TLS authentication using SAN URI) used it in
`tlsGetPeerUser()` without a version guard, breaking builds against
`OpenSSL 1.1.x.`
Use `SSL_get_peer_certificate()` on OpenSSL < 3.0 with the corresponding
`X509_free()` since the older API increments the reference count.
Fixes build failure: implicit declaration of function
`SSL_get0_peer_certificate [-Werror=implicit-function-declaration]`
Also fixes the version mismatch for almalinux 9 daily tests.
Closes#3304.
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
The CodeQL workflow is currently throwing a deprecation warning
regarding use of v3.
> CodeQL Action v3 will be deprecated in December 2026. Please update
all occurrences of the CodeQL Action in your workflow files to v4.
This PR introduces the following changes:
* References to CodeQL v3 have been updated to the SHA of the latest
CodeQL release, [v4.32.5].
Signed-off-by: Kurt McKee <contactme@kurtmckee.org>
Closes https://github.com/valkey-io/valkey/issues/3077
### Overview
URI in SAN is used to represent client identities in modern mTLS
deployments where CN may be empty or deprecated. See the
https://github.com/valkey-io/valkey/issues/3077#issuecomment-3775615304
for more details.
When `tls-auth-clients-user URI` is configured, during the TLS
handshake, the server iterates through the URIs in the client
certificate and authenticates the client as the first enabled user whose
name matches one of those URIs.
### Implementation
- Introduced a new value `URI` for `tls-auth-clients-user`
- Added new function `getCertSanUri` that:
- Extracts URI entries from the certificate's SAN extension
- Checks each URI against existing Valkey users
- Returns the first URI that matches an enabled user
- Renamed `getCertFieldByName` → `getCertSubjectFieldByName` for clarity
- Modified `tlsGetPeerUsername` to support both CN and URI
authentication modes
### Example behavior
Common setup
```
# client certificate X509v3 Subject Alternative Name
URI:urn:valkey:user:first, URI:urn:valkey:user:second
# valkey.conf
tls-auth-clients-user URI
hide-user-data-from-log no
```
Use case 1: multiple enabled users
```
user urn:valkey:user:first on >clientpass allcommands allkeys
user urn:valkey:user:second on >clientpass allcommands allkeys
39762:M 26 Jan 2026 22:06:25.122 - TLS: Auto-authenticated client as urn:valkey:user:first
```
Use case 2: first URI disabled, second enabled
```
user urn:valkey:user:first off >clientpass allcommands allkeys
user urn:valkey:user:second on >clientpass allcommands allkeys
39792:M 26 Jan 2026 22:07:08.006 - TLS: Auto-authenticated client as urn:valkey:user:second
```
Use case 3: all matching users disabled or no matching user
```
user urn:valkey:user:first off >clientpass allcommands allkeys
user urn:valkey:user:second off >clientpass allcommands allkeys
39812:M 26 Jan 2026 22:07:34.174 * TLS: No matching user found in certificate SAN URI fields
127.0.0.1:6379> acl whoami
"default"
127.0.0.1:6379> acl log
1) 1) "count"
2) (integer) 1
3) "reason"
4) "tls-cert"
5) "context"
6) "toplevel"
7) "object"
8) ""
9) "username"
10) "urn:valkey:user:second"
11) "age-seconds"
12) "17.381"
13) "client-info"
14) "id=3 addr=127.0.0.1:57236 laddr=127.0.0.1:6379 fd=8 name= age=0 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=0 multi-mem=0 rbs=16384 rbp=16384 obl=0 oll=0 omem=0 tot-mem=17024 events=r cmd=NULL user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=0 tot-net-out=0 tot-cmds=0"
15) "entry-id"
16) (integer) 0
17) "timestamp-created"
18) (integer) 1771963041866
19) "timestamp-last-updated"
20) (integer) 1771963041866
127.0.0.1:6379>
```
---------
Signed-off-by: Yang Zhao <zymy701@gmail.com>
The test "Blocking keyspace notification with pipelining hset after
hget" was recently failing intermittently with two different errors:
1. `Expected [expr {114 * 10 < 1114}]` - timing assertion failed under Valgrind
2. `Timeout waiting for blocked clients` - race condition on normal runs
The test used wall-clock timing to verify that hget (non-blocking)
completed faster than hset (blocking). This is unreliable because:
- Valgrind slows execution 10-50x, making timing ratios meaningless
- Fast systems may complete both operations in <10ms, causing ratio failures
This fix replaces timing assertions with blocked client count checks,
which directly verify the blocking mechanism rather than inferring it
from timing. The test now confirms hget's response is available before
hset blocks, then waits for the blocked client count to transition
through the expected states.
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Simple function renaming. The zsl prefix denotes the function is
specific to the zset skiplist, but zslFreeLexRange and zslParseLexRange
are not specific to zset encoding. `zset` is a more accurate prefix.
I am working on #3166 but this is worthwhile on its own.
Signed-off-by: Rain Valentine <rainval@amazon.com>
Honors `workflow_call` inputs rather than checking out the
`GITHUB_HEAD_REF` always.
```
Determining the checkout info
/usr/bin/git branch --list --remote origin/8.0
origin/8.0
/usr/bin/git sparse-checkout disable
/usr/bin/git config --local --unset-all extensions.worktreeConfig
Checking out the ref
/usr/bin/git checkout --progress --force -B 8.0 refs/remotes/origin/8.0
Switched to a new branch '8.0'
branch '8.0' set up to track 'origin/8.0'.
```
Now the workflow checks out the right branch.
Link:
https://github.com/sarthakaggarwal97/valkey/actions/runs/22599943936/job/65479450708#step:3:83
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
After introducing dual channel replication in #60, we added two new info
fields `replicas_repl_buffer_size` and `replicas_repl_buffer_peak` to the
info replication section.
However, we did not display the actual memory usage of the replication
buffer in info memory section. Furthermore, since the replicas replication
buffer was not previously included in memory overhead section, this caused
the `used_memory_dataset` and `used_memory_overhead` fields in INFO MEMORY
to display incorrect values, and also caused the `overhead.total` field in
`MEMORY STATS` to display an incorrect value. Previously, on replica nodes,
the replicas replication buffer was included in dataset memory.
Changes:
- `mem_total_replication_buffers` should include all replication buffers, so
now it will also contain `mem_replicas_repl_buffer`.
- Added a new `mem_replicas_repl_buffer` field in INFO MEMORY section. During
the dual channel replication, it show total memory consumed by the replicas
replication buffer on replica side.
- Added a `new replicas.repl.buffer` field in MEMORY STATS. It is the same
metric as `mem_replicas_repl_buffer`.
Signed-off-by: Binbin <binloveplay1314@qq.com>
In `evalMode()`, `argv2` is allocated with `zmalloc()` but freed with `free()`.
On builds using jemalloc/tcmalloc, this can cause `free(): invalid pointer`
Closes#3273
Signed-off-by: Su Ko <rhtn1128@gmail.com>
Suppress valgrind for BIO jobs valgrind errors:
https://github.com/valkey-io/valkey/actions/runs/21969557125/job/63467641572#step:6:8648
## Changes
- Add `allocBioJob()` with `__attribute__((noinline))` as a centralized
allocation function for all BIO jobs. The `noinline` attribute ensures
it appears as a distinct frame in valgrind stack traces.
- Replace all direct `zmalloc` calls in `bioCreate*Job` functions
with`allocBioJob()`.
- Add a valgrind suppression in `src/valgrind.sup` matching definite
leaks with `allocBioJob` in the call stack.
- Remove `current_job` introduced in #3256
---------
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
Fix#3222
- Replace assert_equal with wait_for_condition for replica DBSIZE checks
- Fix a duplicate assert_equal line that checked $not_owning_repl twice
instead of checking both $not_owning_prim and $not_owning_repl.
## Analysis
wait_for_countkeysinslot did work, the keys were there. The issue is
that COUNTKEYSINSLOT and DBSIZE read from different sources.
COUNTKEYSINSLOT reads the actual hashtable size:
```
// countKeysInSlotForDb (cluster.c, line 831)
kvstoreHashtableSize(db->keys, hashslot)
→ hashtableSize(ht) // actual keys in the slot
```
DBSIZE reads a cached counter:
```
// dbsizeCommand (db.c)
kvstoreSize(c->db->keys) // This can be found in line 358, kvstore.c)
→ kvs->key_count // cached total
```
When a slot is marked as importing, keys are excluded from key_count:
```
// cumulativeKeyCountAdd (kvstore.c, line 157)
if (kvstoreIsImporting(kvs, didx)) {
kvs->importing_key_count += delta; // hidden from DBSIZE
return; // key_count NOT updated
}
kvs->key_count += delta; // visible to DBSIZE
```
key_count will be updated when:
```
finishSlotMigrationJob (cluster_migrateslots.c, line 2302)
-> call setSlotImportingStateInAllDbs(slots, 0) (cluster_migrateslots.c, line 2340)
-> call setSlotImportingStateInDb() (cluster_migrateslots.c, line 249)
-> call kvstoreSetIsImporting() (cluster_migrates.c, line 238)
-> call cmulativeKeyCountAdd() (kvstore.c, line 955)
-> kvs->key_count += delta;
```
When slots are marked as importing, the keys exist in the hashtables but
are excluded from metrics like key_count. As documented in
kvstoreSetIsImporting (kvstore.c):
"Importing hashtables are not included in hashtable metrics and are
excluded from scanning and random key lookup."
After the replica restarts and resyncs, there is a brief window where
the migrated slots are still marked as importing. During this window,
COUNTKEYSINSLOT sees the keys (reads actual hashtable) but DBSIZE
returns 0 (reads key_count which excludes importing keys). Once the
importing flag is cleared, key_count is updated and DBSIZE returns the
correct value.
wait_for_condition handles this by retrying until DBSIZE reflects the
updated key_count.
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Publish OpenSSF Scorecard results, which means users and downstream
consumers can easily discover the project’s security best-practice
signals via Scorecard API.
Publishing Scorecard results:
- Improves transparency for users and integrators
- Provides early visibility into missing or improvable security
practices
Fixes#3162
---------
Signed-off-by: Gagan H R <hrgagan4@gmail.com>
This PR decouples the mandatory RDMA library dependency from
`valkey-cli` and `valkey-benchmark` by implementing a **runtime dynamic
loading (lazy loading)** mechanism.
When `BUILD_RDMA=module` is used, the CLI tools are no longer
hard-linked to `librdmacm` and `libibverbs`. Instead, these libraries
are only loaded via `dlopen` when the `--rdma` flag is explicitly
invoked by the user.
- **Dynamic Symbol Loader**: Implemented a symbol loader in libvalkey
using dlopen() and dlsym(), here:
https://github.com/valkey-io/libvalkey/pull/284. That was merged and
libvalkey was updated in valkey to include this change.
- **Build System Propagation**:
- **Makefile**: When building as a module, pass `USE_DLOPEN_RDMA` flag
to libvalkey's makefile.
- **CMake**: When building as a module, pass `ENABLE_DLOPEN_RDMA` flag
to libvalkey's CMake build.
Fixes#3070
Signed-off-by: Ada-Church-Closure <nunotabashinobu066@gmail.com>
The test was using `assert_morethan_equal` with fixed minimum thresholds
(500ms for high, 300ms for low), which caused flaky failures when the
actual latency values were slightly below these thresholds due to timing
variations.
Example failure:
```
Expected '499' to be more than or equal to '500' (context: type eval line 12 cmd {assert_morethan_equal $high 500} proc ::test)
```
Changes:
- Replaced assert_morethan_equal $high 500 with assert_range $high 450
550
- Replaced assert_morethan_equal $low 300 with assert_range $low 250 350
The new ranges (450-550ms for high, 250-350ms for low) align with the
expected latency values from the debug sleep commands (0.5s and 0.3s
respectively) while providing tolerance for timing variations,
consistent with the approach used elsewhere in the test file.
Signed-off-by: Yana Molodetsky <yamolodu@amazon.com>
## Problem
The peek-then-pop pattern introduced in
https://github.com/valkey-io/valkey/pull/3178 solved the valgrind
false-positive leak report
(https://github.com/valkey-io/valkey/actions/runs/21969557125/job/63467641572#step:6:8648)
for in-flight BIO jobs, but `peek` is a problematic API on a mutexqueue:
multiple readers can peek the same item, and one reader can pop what
another peeked.
## Fix
Instead, store the in-flight job pointer in
`bio_worker_data.current_job` before processing and clear it after
freeing. Since `bio_workers[]` is a static array, valgrind can always
trace from global memory to the job allocation, even after the worker
thread is cancelled.
Removed `mutexQueuePeek` from mutexqueue.{c,h} and its test.
## Test
Manually run Daily workflow: All four valgrind jobs are green:
`test-valgrind-test` passed, `test-valgrind-misc` passed,
`test-valgrind-no-malloc-usable-size-test` passed,
`test-valgrind-no-malloc-usable-size-misc` passed.
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
The memory test was commented out in #2858 and should have been
reenabled. On further investigation I found that the server hangs during
shutdown inside the `bioDrainWorker(BIO_LAZY_FREE)` call. This causes
deadlock because the lock was acquired for shutdown but lazy free jobs
require the GIL too:
- main thread: `serverCron()` acquires GIL via `afterSleep()` then calls
`finishShutdown()`, which eventually calls our script module unload code
that calls `bioDrainWorker()`.
- bio threads: Pending lazy free jobs such as `lazyFreeEvalScripts()`
call `scriptingEngineCallFreeFunction()` which requires the GIL.
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
During hashtable shrinking, all keys are inserted into ht1.
This PR adds a mechanism to the hashtable: when there are severe hash
collisions with the new ht1 during the shrink rehashing process, the
hashtable will stop shrinking by swapping ht0 and ht1 to avoid excessive
performance degradation of the new hashtable during this period.
In extreme cases, for example, if ht0 is very large and is reduced to
only one entry, and the new ht1 is very small after the resize, the ht0
rehash process is very slow since we have a lot of empty buckets. If a
large number of elements are added into ht1 at this time, it will lead
to severe hash collisions in ht1.
During the add operation during hashtable shrinking, we check the fill
percentage of ht1. If it exceeds MAX_FILL_PERCENT_HARD, we swap ht0 and
ht1, and abort the shrinking process, and then set rehash_idx back to 0
and restart the rehash.
This PR also added a new debug hashtable-can-abort-shrink subcommand to
control this behavior.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
## Summary
Fixes#2620
Skip loading expired hash fields when a non-preamble RDB is being loaded
on a primary server. Propagate `HDEL` to replicas when expired fields
are skipped.
## Changes
- Updated `rdbLoadObject` signature to accept `rdbflags` and `now`
parameters
- Added logic to skip expired hash fields during RDB load on primary
- Propagate `HDEL` to replicas when `RDBFLAGS_FEED_REPL` is set
- Updated all callers of `rdbLoadObject`
- Added unit test
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Remove -encoding binary from fconfigure as it is no longer supported in
Tcl 9 (Fedora Rawhide and Daily / test-fedoralatest-jemalloc
(pull_request)). -translation binary alone is sufficient.
```
https://github.com/valkey-io/valkey/actions/runs/22324297258/job/64590589777?pr=3225
===== Start of server log (pid 17617) =====
### Starting server for test
17617:M 23 Feb 2026 21:21:47.846 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
17617:M 23 Feb 2026 21:21:47.846 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
17617:M 23 Feb 2026 21:21:47.846 * Valkey version=9.0.3, bits=64, commit=00000000, modified=0, pid=17617, just started
17617:M 23 Feb 2026 21:21:47.846 * Configuration loaded
17617:M 23 Feb 2026 21:21:47.847 * monotonic clock: POSIX clock_gettime
.+^+.
.+#########+.
.+########+########+. Valkey 9.0.3 (00000000/0) 64 bit
.+########+' '+########+.
.########+' .+. '+########. Running in cluster mode
|####+' .+#######+. '+####| Port: 25121
|###| .+###############+. |###| PID: 17617
|###| |#####*'' ''*#####| |###|
|###| |####' .-. '####| |###|
|###| |###( (@@@) )###| |###| https://valkey.io/
|###| |####. '-' .####| |###|
|###| |#####*. .*#####| |###|
|###| '+#####| |#####+' |###|
|####+. +##| |#+' .+####|
'#######+ |##| .+########'
'+###| |##| .+########+'
'| |####+########+'
+#########+'
'+v+'
17617:M 23 Feb 2026 21:21:47.849 * No cluster configuration found, I'm 5501916ccdf76dee6b652cab10402cab3a8f9152
17617:M 23 Feb 2026 21:21:47.852 * Server initialized
17617:M 23 Feb 2026 21:21:47.852 * Ready to accept connections tcp
17617:M 23 Feb 2026 21:21:47.852 * Ready to accept connections unix
17617:M 23 Feb 2026 21:21:47.963 - Accepted 127.0.0.1:37791
17617:M 23 Feb 2026 21:21:47.963 - Client closed connection id=2 addr=127.0.0.1:37791 laddr=127.0.0.1:25121 fd=12 name= age=0 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=20474 argv-mem=0 multi-mem=0 rbs=16384 rbp=16384 obl=0 oll=0 omem=0 tot-mem=37504 events=r cmd=ping user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=7 tot-net-out=7 tot-cmds=1
17617:M 23 Feb 2026 21:21:47.969 - Accepted 127.0.0.1:39251
17617:M 23 Feb 2026 21:21:47.970 * configEpoch set to 1 via CLUSTER SET-CONFIG-EPOCH
17617:M 23 Feb 2026 21:21:47.975 # Missing implement of connection type tls
17617:M 23 Feb 2026 21:21:47.976 # DEBUG LOG: ========== I am primary 0 ==========
17617:M 23 Feb 2026 21:21:49.872 * Cluster state changed: ok
### Starting test Packet with missing gossip messages don't cause invalid read in tests/unit/cluster/packet.tcl
17617:M 23 Feb 2026 21:21:49.879 - Accepting cluster node connection from 127.0.0.1:33701
17617:M 23 Feb 2026 21:21:49.880 - Client closed connection id=3 addr=127.0.0.1:39251 laddr=127.0.0.1:25121 fd=12 name= age=2 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=20474 argv-mem=0 multi-mem=0 rbs=1024 rbp=426 obl=0 oll=0 omem=0 tot-mem=22144 events=r cmd=cluster|info user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=1363 tot-net-out=17324 tot-cmds=47
17617:signal-handler (1771881709) Received SIGTERM scheduling shutdown...
17617:M 23 Feb 2026 21:21:49.973 * User requested shutdown...
17617:M 23 Feb 2026 21:21:49.973 * Removing the pid file.
17617:M 23 Feb 2026 21:21:49.973 * Saving the cluster configuration file before exiting.
17617:M 23 Feb 2026 21:21:49.978 * Removing the unix socket file.
17617:M 23 Feb 2026 21:21:49.978 # Valkey is now ready to exit, bye bye...
===== End of server log (pid 17617) =====
===== Start of server stderr log (pid 17617) =====
===== End of server stderr log (pid 17617) =====
[exception]: Executing test client: unknown encoding "binary": No longer supported.
please use either "-translation binary" or "-encoding iso8859-1".
unknown encoding "binary": No longer supported.
please use either "-translation binary" or "-encoding iso8859-1"
while executing
"fconfigure $sock -translation binary -encoding binary -buffering none -blocking 1"
("uplevel" body line 18)
invoked from within
"uplevel 1 $code"
(procedure "test" line 62)
invoked from within
"test "Packet with missing gossip messages don't cause invalid read" {
set base_port [srv 0 port]
set cluster_port [expr {$base_port + ..."
("uplevel" body line 2)
invoked from within
"uplevel 1 $code"
(procedure "cluster_setup" line 41)
invoked from within
"cluster_setup 1 0 1 continuous_slot_allocation default_replica_allocation {
test "Packet with missing gossip messages don't cause invalid read" {
..."
("uplevel" body line 1)
invoked from within
"uplevel 1 $code "
(procedure "start_server" line 2)
invoked from within
"start_server {overrides {cluster-enabled yes cluster-ping-interval 100 cluster-node-timeout 3000 cluster-databases 16 cluster-slot-stats-enabled yes} ..."
("uplevel" body line 1)
invoked from within
"uplevel 1 $code"
(procedure "start_multiple_servers" line 5)
invoked from within
"start_multiple_servers $node_count $options $code"
(procedure "start_cluster" line 17)
invoked from within
"start_cluster 1 0 {tags {external:skip cluster tls:skip}} {
test "Packet with missing gossip messages don't cause invalid read" {
set base..."
(file "tests/unit/cluster/packet.tcl" line 84)
invoked from within
"source $path"
(procedure "execute_test_file" line 4)
invoked from within
"execute_test_file $data"
(procedure "test_client_main" line 10)
invoked from within
"test_client_main $::test_server_port "
Killing still running Valkey server 10700
Killing still running Valkey server 11659
Killing still running Valkey server 11705
Killing still running Valkey server 12735
Killing still running Valkey server 12775
Killing still running Valkey server 12808
Killing still running Valkey server 12858
Killing still running Valkey server 12901
Killing still running Valkey server 12940
Killing still running Valkey server 12972
Killing still running Valkey server 13004
Killing still running Valkey server 13036
Killing still running Valkey server 13073
Killing still running Valkey server 14952
Killing still running Valkey server 16631
Killing still running Valkey server 16670
Killing still running Valkey server 16686
Killing still running Valkey server 16702
Killing still running Valkey server 16718
Killing still running Valkey server 16734
Killing still running Valkey server 16753
Killing still running Valkey server 16771
Killing still running Valkey server 17094
Killing still running Valkey server 17112
Killing still running Valkey server 17133
Killing still running Valkey server 17262
Killing still running Valkey server 17278
Killing still running Valkey server 17316
Killing still running Valkey server 17349
Killing still running Valkey server 17379
Killing still running Valkey server 17410
Killing still running Valkey server 17430
Killing still running Valkey server 17443
Killing still running Valkey server 17459
Killing still running Valkey server 17478
Killing still running Valkey server 17491
Killing still running Valkey server 17507
Killing still running Valkey server 17526
Killing still running Valkey server 17545
Killing still running Valkey server 17561
Killing still running Valkey server 17577
Killing still running Valkey server 17716
Killing still running Valkey server 17778
Killing still running Valkey server 17825
Killing still running Valkey server 17848
Killing still running Valkey server 17896
Killing still running Valkey server 17931
Killing still running Valkey server 17953
Killing still running Valkey server 17973
Killing still running Valkey server 17991
Killing still running Valkey server 18010
Killing still running Valkey server 18064
Killing still running Valkey server 18082
Killing still running Valkey server 18134
Killing still running Valkey server 18118
Killing still running Valkey server 18150
Killing still running Valkey server 18166
Killing still running Valkey server 18188
Killing still running Valkey server 18209
```
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
The `test_quicklistCompressAndDecompressQuicklistListpackNode` unit test
was failing with ASan in CI with OOM errors. The test was allocating and
compressing up to 1GB of data (32 iterations × 32MB), which exceeded
available memory in CI environments. The test is skipped for ASan
builds.
This PR addressed the unit test failures in
`test-sanitizer-address-large-memory` for both GCC and CLANG
Resolves#3221
---------
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
The test somehow is slow and due to the short cluster-node-timeout, an
automatic
failover may fail to trigger due to cluster-replica-validity-factor:
```
*** [err]: Automatic failover vote is not limited by two times the node timeout - mixed failover in tests/unit/cluster/manual-failover.tcl
The third failover does not happen
xxx # Cluster state changed: fail
xxx # Cluster is currently down: At least one hash slot is not served by any available node. Please check the 'cluster-require-full-coverage' configuration.
```
Closes#3203.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Updates to latest versions for each of the github actions used.
Pinning prevents an attack where the upstream action dependency is
compromised and the "v4" tag for example gets edited to point to a
malicious version. We already do this for most checkout actions in our
workflows.
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
If the client executing `ACL LOAD` is a user that is removed from ACL
file, it frees itself while still executing the command. Same happens
for user while removing it's channel permissions.
This causes `c->conn` pointer to become `null` and later
`serverAssert(c->conn)` fails for that client on write in
`prepareClientToWrite()`.
Solution is to flag current client to be closed after command completes
and reply is written, later this client is freed async.
Resolves#3181
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
After #3103 time sensitive `test-ubuntu-reclaim-cache` started to fail
because now startup always includes 30ms of calibration of HW clock,
that's why we get this output:
```
Run echo "test SAVE doesn't increase cache"
test SAVE doesn't increase cache
2460491776
Could not connect to Valkey at 127.0.0.1:8080: Connection refused
```
Added waits for server to start, locally run, it helps
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
When a cluster reset is performed on a replica node, a new shard ID is generated
because the node is about to become an empty primary node, see #2283.
However, the log added in #2510 caused some confusions. In clusterSetNodeAsPrimary
we will print:
```
serverLog(LL_NOTICE, "Reconfiguring node %.40s (%s) as primary for shard %.40s", n->name, humanNodename(n), n->shard_id);
```
In clusterReset, we first call clusterSetNodeAsPrimary and then generate a new
shard ID, which causes us to print an error shard ID log first.
There is an exmaple, when a replica node performs a cluster reset, we will print:
```
xxx * Cluster reset (user request from 'xxx').
xxx * Reconfiguring node af76a3e0ffcd77bd14fa47ce4d07ab2bdc78702f (xxx) as primary for shard ea528667634af8beed83adac2b9af8360769a1b4
```
But the node shard id is actually:
```
xxx> cluster myshardid
"52ede26d1554dd203161ba09011af14574b2cc84"
```
Now after a new shard ID is generated we will print a log, and we also move the
call to clusterSetNodeAsPrimary after the new shard id, so that we can have the
right one. After this PR:
```
xxx * Cluster reset (user request from 'xxx').
xxx * Moving myself to a new shard bd31870ce73f5977084e6a46e337a4a1ad38fc66.
xxx * Reconfiguring node 1d54b904efd30cd9d7d1abbfd63c8fafbb62e1c8 (xxx) as primary for shard bd31870ce73f5977084e6a46e337a4a1ad38fc66
```
This is part of #2989, but i guess we won't merge the extension fix in a short
time, so i am gonna extracting it separately as a log fix (or improvement).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Remove -encoding binary from fconfigure as it is no longer supported in
Tcl 9 (Fedora Rawhide and Daily / test-fedoralatest-jemalloc
(pull_request)). -translation binary alone is sufficient.
```
https://github.com/valkey-io/valkey/actions/runs/22324297258/job/64590589777?pr=3225
===== Start of server log (pid 17617) =====
### Starting server for test
17617:M 23 Feb 2026 21:21:47.846 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
17617:M 23 Feb 2026 21:21:47.846 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
17617:M 23 Feb 2026 21:21:47.846 * Valkey version=9.0.3, bits=64, commit=00000000, modified=0, pid=17617, just started
17617:M 23 Feb 2026 21:21:47.846 * Configuration loaded
17617:M 23 Feb 2026 21:21:47.847 * monotonic clock: POSIX clock_gettime
.+^+.
.+#########+.
.+########+########+. Valkey 9.0.3 (00000000/0) 64 bit
.+########+' '+########+.
.########+' .+. '+########. Running in cluster mode
|####+' .+#######+. '+####| Port: 25121
|###| .+###############+. |###| PID: 17617
|###| |#####*'' ''*#####| |###|
|###| |####' .-. '####| |###|
|###| |###( (@@@) )###| |###| https://valkey.io/
|###| |####. '-' .####| |###|
|###| |#####*. .*#####| |###|
|###| '+#####| |#####+' |###|
|####+. +##| |#+' .+####|
'#######+ |##| .+########'
'+###| |##| .+########+'
'| |####+########+'
+#########+'
'+v+'
17617:M 23 Feb 2026 21:21:47.849 * No cluster configuration found, I'm 5501916ccdf76dee6b652cab10402cab3a8f9152
17617:M 23 Feb 2026 21:21:47.852 * Server initialized
17617:M 23 Feb 2026 21:21:47.852 * Ready to accept connections tcp
17617:M 23 Feb 2026 21:21:47.852 * Ready to accept connections unix
17617:M 23 Feb 2026 21:21:47.963 - Accepted 127.0.0.1:37791
17617:M 23 Feb 2026 21:21:47.963 - Client closed connection id=2 addr=127.0.0.1:37791 laddr=127.0.0.1:25121 fd=12 name= age=0 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=20474 argv-mem=0 multi-mem=0 rbs=16384 rbp=16384 obl=0 oll=0 omem=0 tot-mem=37504 events=r cmd=ping user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=7 tot-net-out=7 tot-cmds=1
17617:M 23 Feb 2026 21:21:47.969 - Accepted 127.0.0.1:39251
17617:M 23 Feb 2026 21:21:47.970 * configEpoch set to 1 via CLUSTER SET-CONFIG-EPOCH
17617:M 23 Feb 2026 21:21:47.975 # Missing implement of connection type tls
17617:M 23 Feb 2026 21:21:47.976 # DEBUG LOG: ========== I am primary 0 ==========
17617:M 23 Feb 2026 21:21:49.872 * Cluster state changed: ok
### Starting test Packet with missing gossip messages don't cause invalid read in tests/unit/cluster/packet.tcl
17617:M 23 Feb 2026 21:21:49.879 - Accepting cluster node connection from 127.0.0.1:33701
17617:M 23 Feb 2026 21:21:49.880 - Client closed connection id=3 addr=127.0.0.1:39251 laddr=127.0.0.1:25121 fd=12 name= age=2 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=20474 argv-mem=0 multi-mem=0 rbs=1024 rbp=426 obl=0 oll=0 omem=0 tot-mem=22144 events=r cmd=cluster|info user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=1363 tot-net-out=17324 tot-cmds=47
17617:signal-handler (1771881709) Received SIGTERM scheduling shutdown...
17617:M 23 Feb 2026 21:21:49.973 * User requested shutdown...
17617:M 23 Feb 2026 21:21:49.973 * Removing the pid file.
17617:M 23 Feb 2026 21:21:49.973 * Saving the cluster configuration file before exiting.
17617:M 23 Feb 2026 21:21:49.978 * Removing the unix socket file.
17617:M 23 Feb 2026 21:21:49.978 # Valkey is now ready to exit, bye bye...
===== End of server log (pid 17617) =====
===== Start of server stderr log (pid 17617) =====
===== End of server stderr log (pid 17617) =====
[exception]: Executing test client: unknown encoding "binary": No longer supported.
please use either "-translation binary" or "-encoding iso8859-1".
unknown encoding "binary": No longer supported.
please use either "-translation binary" or "-encoding iso8859-1"
while executing
"fconfigure $sock -translation binary -encoding binary -buffering none -blocking 1"
("uplevel" body line 18)
invoked from within
"uplevel 1 $code"
(procedure "test" line 62)
invoked from within
"test "Packet with missing gossip messages don't cause invalid read" {
set base_port [srv 0 port]
set cluster_port [expr {$base_port + ..."
("uplevel" body line 2)
invoked from within
"uplevel 1 $code"
(procedure "cluster_setup" line 41)
invoked from within
"cluster_setup 1 0 1 continuous_slot_allocation default_replica_allocation {
test "Packet with missing gossip messages don't cause invalid read" {
..."
("uplevel" body line 1)
invoked from within
"uplevel 1 $code "
(procedure "start_server" line 2)
invoked from within
"start_server {overrides {cluster-enabled yes cluster-ping-interval 100 cluster-node-timeout 3000 cluster-databases 16 cluster-slot-stats-enabled yes} ..."
("uplevel" body line 1)
invoked from within
"uplevel 1 $code"
(procedure "start_multiple_servers" line 5)
invoked from within
"start_multiple_servers $node_count $options $code"
(procedure "start_cluster" line 17)
invoked from within
"start_cluster 1 0 {tags {external:skip cluster tls:skip}} {
test "Packet with missing gossip messages don't cause invalid read" {
set base..."
(file "tests/unit/cluster/packet.tcl" line 84)
invoked from within
"source $path"
(procedure "execute_test_file" line 4)
invoked from within
"execute_test_file $data"
(procedure "test_client_main" line 10)
invoked from within
"test_client_main $::test_server_port "
Killing still running Valkey server 10700
Killing still running Valkey server 11659
Killing still running Valkey server 11705
Killing still running Valkey server 12735
Killing still running Valkey server 12775
Killing still running Valkey server 12808
Killing still running Valkey server 12858
Killing still running Valkey server 12901
Killing still running Valkey server 12940
Killing still running Valkey server 12972
Killing still running Valkey server 13004
Killing still running Valkey server 13036
Killing still running Valkey server 13073
Killing still running Valkey server 14952
Killing still running Valkey server 16631
Killing still running Valkey server 16670
Killing still running Valkey server 16686
Killing still running Valkey server 16702
Killing still running Valkey server 16718
Killing still running Valkey server 16734
Killing still running Valkey server 16753
Killing still running Valkey server 16771
Killing still running Valkey server 17094
Killing still running Valkey server 17112
Killing still running Valkey server 17133
Killing still running Valkey server 17262
Killing still running Valkey server 17278
Killing still running Valkey server 17316
Killing still running Valkey server 17349
Killing still running Valkey server 17379
Killing still running Valkey server 17410
Killing still running Valkey server 17430
Killing still running Valkey server 17443
Killing still running Valkey server 17459
Killing still running Valkey server 17478
Killing still running Valkey server 17491
Killing still running Valkey server 17507
Killing still running Valkey server 17526
Killing still running Valkey server 17545
Killing still running Valkey server 17561
Killing still running Valkey server 17577
Killing still running Valkey server 17716
Killing still running Valkey server 17778
Killing still running Valkey server 17825
Killing still running Valkey server 17848
Killing still running Valkey server 17896
Killing still running Valkey server 17931
Killing still running Valkey server 17953
Killing still running Valkey server 17973
Killing still running Valkey server 17991
Killing still running Valkey server 18010
Killing still running Valkey server 18064
Killing still running Valkey server 18082
Killing still running Valkey server 18134
Killing still running Valkey server 18118
Killing still running Valkey server 18150
Killing still running Valkey server 18166
Killing still running Valkey server 18188
Killing still running Valkey server 18209
```
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
This issue was encountered while processing #3121. Currently in all our
commands with KSPEC_FK_KEYNUM, key step is 1. So this bug does not
currently
affect any core commands.
If we have commands with different key step values, calculting the last
key
in here will casue problems since we are not including step in the
calculation.
Signed-off-by: Binbin <binloveplay1314@qq.com>
When dual-channel-replication is enabled, and replica-announce-ip is
set, the RDB/AOF channel does not announce itself at this endpoint. This
defaults to the IP address behind the NAT, or the Kubernetes Pod IP in
our case.
This means that if Sentinel is polling the primary for connected
replicas, it will first see the ephemeral pod IP, then revert to the
announce-ip - leaving behind the pod IP as a down replica.
This PR configures the RDB/AOF channel to also announce itself at the
announce-ip to prevent the stale replica.
## Testing
I evaluated writing unit tests for this, but I am not sure of a way we
can test an IP address different to localhost (127.0.0.1) that would
fail without the fix. I did test on Kubernetes against 9.0 tag and
verified the fix there too.
### Status quo
On 9.0 image tag:
```
$ kubectl get pods -n valkey-baseline -o custom-columns=NAME:.metadata.name,POD-IP:.status.podIP
NAME POD-IP
valkey-primary-5bd78c8566-llb6k 10.244.0.25
valkey-replica-0 10.244.0.17
valkey-replica-1 10.244.0.13
$ kubectl get services -n valkey-baseline -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.spec.clusterIP
NAME CLUSTER-IP
valkey-primary 10.96.147.28
valkey-replica-0 10.96.66.233
valkey-replica-1 10.96.57.230
```
Logs below show that pod IP for valkey-primary-5bd78c8566-llb6k
`10.244.0.25:6379` is being used for dual-channel replication. This
should be its cluster IP `10.96.147.28` as this is what is set in
replica-announce-ip.
```
1:M 14 Nov 2025 17:57:51.750 * Replica 10.96.147.28:6379 asks for synchronization
1:M 14 Nov 2025 17:57:51.751 * Replica 10.244.0.25:6379 asks for synchronization
1:M 14 Nov 2025 17:57:56.135 * Dual channel replication: Sending to replica 10.244.0.25:6379 RDB end offset 1763269 and client-id 35
1:M 14 Nov 2025 17:57:56.140 * Replica 10.96.147.28:6379 asks for synchronization
```
### This fix
```
$ kubectl get pods -n valkey-test -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.status.podIP
NAME POD-IP
valkey-primary-594c9597b5-qqvdk 10.244.0.26
valkey-replica-0 10.244.0.10
valkey-replica-1 10.244.0.18
$ kubectl get services -n valkey-test -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.spec.clusterIP
NAME CLUSTER-IP
valkey-primary 10.96.125.142
valkey-replica None
valkey-replica-0 10.96.155.74
valkey-replica-1 10.96.64.111
valkey-sentinel None
```
Logs show that the Cluster IP is now being used for dual-channel
replication.
```
1:M 14 Nov 2025 17:57:49.923 * Replica 10.96.125.142:6379 asks for synchronization
1:M 14 Nov 2025 17:57:49.924 * Replica 10.96.125.142:6379 asks for synchronization
1:M 14 Nov 2025 17:57:54.913 * Dual channel replication: Sending to replica 10.96.125.142:6379 RDB end offset 1771247 and client-id 36
1:M 14 Nov 2025 17:57:54.916 * Replica 10.96.125.142:6379 asks for synchronization
```
Fixes#2338
Signed-off-by: Joseph Heyburn <jdheyburn@gmail.com>
Update deps/libvalkey to version 0.4.0
Squashed 'deps/libvalkey/' changes from b012f8e85..45c2ed15c
45c2ed15c Release 0.4.0 (#286)
40d6590d7 Implement runtime dynamic loading for RDMA libraries (#284)
62e757d17 Release 0.3.0 (#283)
a554f0942 Fix potential uint32_t underflow issue (#280)
8f9051ae0 Correcting command parser bug (#277)
29023eb36 Add valkey-json, valkey-bloom, valkey-search to cmddef.h
ae756bc89 Update cmddef.h to Valkey 9.0.0
21abd737e Replace problematic alloca() with fixed stack alloc
38191079c Fix compilation on Solaris with Sun/Solaris Studio
ef5de0312 Make libvalkey initialization thread-safe
ae341dea5 Support slotmap updates using CLUSTER NODES in RESP3 (#262)
36f6e2292 Fix the long-blocking read for Valkey RDMA. (#233)
c090c28be Use a uintptr_t hop for casting pointers to ints
daa7f11ac Avoid heap buffer overflow in valkeyAsyncFormattedCommand (#245)
15974930d Add option to select a logical database (#244)
983d67e4f Install the macosx adapter on Apple platforms only
...
git-subtree-dir: deps/libvalkey
git-subtree-split: 45c2ed15cab9fa0ea1a6cabc8460f5eea6240de5
Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
This change amends #3086 in which `commandlog-request-larger-than` was
mixed up with `commandlog-reply-larger-than` while checking for config
values and in tests.
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Currently after #2652 in copy avoidance path we unconditionally track
`c->net_output_bytes_curr_cmd` even when `commandlog-request-larger-than
-1`. This PR provides ability to skip that accounting in copy avoidance
path based on config value. If `commandlog-request-larger-than -1` then
performance is same as before #2652.
Also added tracking of `c->net_output_bytes_curr_cmd` in IO thread if it
is not tracked in main thread based on decision made in main thread.
Read Performance (write performance is the same):
```
With this change:
Summary:
throughput summary: 2191732.75 requests per second
latency summary (msec):
avg min p50 p95 p99 max
1.720 0.072 1.743 1.919 2.647 23.983
Unstable:
Summary:
throughput summary: 1658197.25 requests per second
latency summary (msec):
avg min p50 p95 p99 max
2.299 0.120 2.343 2.503 3.319 4.791
Config:
commandlog-request-larger-than -1
^^ without this performance is just like on unstable branch
databases 1
save ""
appendonly no
rdbcompression no
activedefrag no
maxclients 1000
io-threads 9
protected-mode no
hz 10
maxmemory 2gb
```
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
When using XREAD STREAMS <stream> + on an empty stream created with
MKSTREAM, valkey returns an error instead of nil.
This happens because is missing a check on the stream length.
The fix adds the length check on the condition.
Fixes#2728
Signed-off-by: diego-ciciani01 <diego.ciciani@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In releaseBufReferences(), clusterSlotStatsAddNetworkBytesOutForSlot() is called in the IO
thread after writing the reply to the client, and (since #2652) it's also done in the normal
code path in the main thread (afterCommand() -> clusterSlotStatsAddNetworkBytesOutForUserClient()).
This means the output bytes are recorded twice in the cluster slot stats.
Fixes#3043. Bug was introduced in #2652 (9.0.1).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9.
The latest Fedora now has TCL 9.0 and it's working now, including the
TCL TLS package. (This wasn't working earlier due to some packaging
errors for TCL packages in Fedora, which have been fixed now.)
This PR also removes the custom compilation of TCL 8 used in our Daily
jobs and uses the system default TCL version instead. The TCL version
depends on the OS. For the latest Fedora, you get 9.0, for macOS you get
8.5 and for most other OSes you get 8.6.
The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was
never released.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
The metric `used_memory_dataset` turned into an insanely large number close to 2^64 (actually
overflowed negative value), as reported in #2994.
## Double-Counted database memory
When server starts, the global variable `server.initial_memory_usage` is used to record a memory
baseline in InitServerLast. This `server.initial_memory_usage` has clearly included initial database
memory, since databases are created in initServer.
In function getMemoryOverheadData, the `mem_total` is firstly assigned the baseline, which includes
initial database memory. And then all extra memory usage of databases are added to mem_total. The initial
database memory are therefore counted TWICE.
This eventually caused wrongly larger `used_memory_overhead`. For a database with only a couple of keys,
the `used_memory_overhead` is easily larger than `used_memory` and causes an overflowed `used_memory_dataset`.
## Missed Empty Databases
In function getMemoryOverheadData(), kvstores without any allocated hashtable are ignored from calculation:
```c
if (db == NULL || !kvstoreNumAllocatedHashtables(db->keys)) continue;
```
However, even the kvstore has no allocated hashtable, there are still some memory allocated by kvstoreCreate(),
including `hashtable_size_index`, which can be larger than 128 KiB.
On the contrary, this caused wrongly smaller `used_memory_overhead` for an empty database. When we insert only
ONE key to the database, the database is suddenly taken into account, and `used_memory_overhead` will increase
(for `used_memory_dataset` decrease) by more than 128 KiB due to the single key insertion.
Signed-off-by: Ace Breakpoint <chemistudio@gmail.com>
Signed-off-by: bpint <chemistudio@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
When loading AOF in cluster mode, keys inside a MULTI/EXEC block could
be
inserted into wrong hash slots, causing key duplication and data
corruption.
The root cause was the slot caching optimization in getKeySlot(). This
optimization reuses a cached slot value to avoid recalculating the hash
for every key operation. However, when replaying AOF, a transaction may
contain commands affecting keys in different slots. The cached slot from
a previous command (e.g., SET k1) would incorrectly be used for
subsequent
commands in the transaction (e.g., SET k0), causing k0 to be stored in
k1's
slot.
The existing code already skipped this optimization for replicated
clients
(commands from primary) using isReplicatedClient(). This change extends
that to also skip for AOF clients by using mustObeyClient() instead,
which
covers both replicated clients and the AOF client.
Fixes#2995, introduced in #1949.
Signed-off-by: aditya.teltia <teltia.aditya22@gmail.com>
There is a crash in freeReplicationBacklog:
```
Discarding previously cached primary state.
ASSERTION FAILED
'listLength(server.replicas) == 0' is not true
freeReplicationBacklog
```
The reason is that during dual channel operation, the RDB channel is protected.
In the chained replica case, `disconnectReplicas` is called to disconnect all
replica clients, but since the RDB channel is protected, `freeClient` does not
actually free the replica client. Later, we encounter an assertion failure in
`freeReplicationBacklog`.
```
void replicationAttachToNewPrimary(void) {
/* Replica starts to apply data from new primary, we must discard the cached
* primary structure. */
serverAssert(server.primary == NULL);
replicationDiscardCachedPrimary();
/* Cancel any in progress imports (we will now use the primary's) */
clusterCleanSlotImportsOnFullSync();
disconnectReplicas(); /* Force our replicas to resync with us as well. */
freeReplicationBacklog(); /* Don't allow our chained replicas to PSYNC. */
}
```
Dual channel replication was introduced in #60.
Signed-off-by: Binbin <binloveplay1314@qq.com>
When import-mode is yes, we might be able to set an expired TTL. At the
same time,
commands like EXPIREAT/EXPIRE do not restrict TTL from being negative.
After we
set import-mode to no, server will crash at:
```
int activeExpireCycleTryExpire(serverDb *db, robj *val, long long now, int didx) {
long long t = objectGetExpire(val);
serverAssert(t >= 0);
```
In this case, we restrict ttl from being negative in
expireGenericCommand, we simply
change the expiration time to 0 to mark the key as expired since in
import-mode, the
import-source client can always read the expired keys anyway.
import-mode was introduced in #1185
---------
Signed-off-by: cjx-zar <jxchenczar@foxmail.com>
Previously, only changes to argv[0] were considered when deciding
whether to re-prepare the command and recompute the cluster slot. This
caused issues where changes to the argument count (argc) would not
trigger the necessary prepare in case additional keys were injected to
the command
This fix adds a check to ensure that changes to argc are properly
considered.
Signed-off-by: Patrik Hermansson <phermansson@gmail.com>
GitHub has deprecated older macOS runners, and macos-13 is no longer supported.
1. The latest version of cross-platform-actions/action does allow
running on ubuntu-latest (Linux runner) and does not strictly require macOS.
2. Previously, cross-platform-actions/action@v0.22.0 used runs-on:
macos-13. I checked the latest version of cross-platform-actions, and
the official examples now use runs-on: ubuntu. I think we can switch from macOS to Ubuntu.
---------
Signed-off-by: Vitah Lin <vitahlin@gmail.com>
In the original implementation of Hash Field Expiration
(https://github.com/valkey-io/valkey/pull/2089), the HSETEX command was
implemented to report keyspace notifications only for performed changes.
This is mostly aligned with other Hash commands (for example, HDEL will
also not report `hdel` event for items which does not exist)
The HSETEX case is somewhat different and is more like the `HSET` case.
During HSETEX, after the command validations pass, items are ALWAYS
"added" to the object, even though they might not actually be added.
This case is the same for when the hash object is empty or when all the
provided fields do not exist in the object (as reported
[here](https://github.com/valkey-io/valkey/pull/2998))
This PR changes the way `HSETEX` will report keyspace notifications so
that:
1. `hset` notification will ALWAYS be reported if all command
validations pass.
2. `hexpire` will be reported in case the command include an expiration
time (even past time)
3. `hxpired` will be reported in case the provided expiration time is in
the past (or 0)
4. `hdel` will be reported in case the hash exists (or created as part
of the command) and following the command execution it was left empty.
5. we will always return '1' as a return value of tHSETEX command which
passed all validations. Before that we returned 1 only if we applied the
change cross ALL the input fields, so in case some of them did not exist
and a past time was set we would return 0.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Jacob Murphy <jkmurphy@google.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
The HFE uses EXPIRY_NONE(-1) for fields without a TTL. A bug exists in
`HSETEX` and RDB loading where `expiry > 0` is used to check for an
expiration. This is problematic because `0` might be treated as no
expiry in `import-mode`, instead of an already expired timestamp,
leading to incorrect behavior.
---------
Signed-off-by: cjx-zar <jxchenczar@foxmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
There are currently several issues with the existing hash field
expiration mechanism:
1. `HINCRBY` is propagated to the replica "as-is". It mean it relies on
the fact that the state of the hash is the same on the primary and the
replica. HFE did change this assumption as the field might be expired
only when the replica will handle the propagated `hincrby`. the problem
is that the replica does not "expire" fields by it's own. it needs to
respect the request from the primary and always try to use the existing
field. This can lead to either miss-alignment with the value on the
primary and the replica AND even a disconnection since the replica might
hold and "expired" field which is not in "integer" format...
2. HINCRBYFLOAT is currently ALWAYS propagating `hset` - this means that
the expiration time of an entry will always be removed on the replica
side (it needs to propagate HSETEX when expiration time needs to be
maintained)
3. Currently all hash write commands which are mutating values might
overwrite an expired field. In such cases the existing implementation
will "silently" do so. The problem is that the user will not get any
key-space-notificaiton explaining the reason for the behavior. For
example, when `hincrby` is issued overwriting an expired field which was
not yet "cleaned" by active-expiration it will reset the counter to '0'
before incrementing it. this means that the user might ask: why is the
value '1' and not bigger, "I did not see any notification that the old
value expired"...
4. HSETEX with KEEPTTL suffers from a "somewhat" similar problem as
if the primary "replaced" the entry which is expired now but might not
have been expired when the primary applied it.
There are 2 options for a solution:
1. we could propagate `hdel` for every entry we are "overwritting"
(batch them if we can)
2. propagate the commands "by effect". For example - have `hincrby`
always propagate either HSET or HSETEX. This will not solve the '#(4)'
problem above though, for which we might HAVE to propagate `hdel`
I tend to go with the second option. The reason is that it is expected
to have less impact on replication stream and should include less
processing time on the replicas and network traffic. Specifically for
HSETEX with KEEPTTL we will have to propagate the `hdel` in case we
overwritten an expired field, but that would help limit the impact of
this propagation.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Sourav Singh Rawat <aidenfrostbite@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
if an expired time is used, the condition is ignored, and it directly
becomes the effect of the `hdel` command.
This is mainly important for cases where the user would like to protect
deleting the data in case the `HEXPIREAT` command is used and the user
would like to protect delay execution of this command to delete fields.
fixes: https://github.com/valkey-io/valkey/issues/3021
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
if the hrandfield(e.g. hrandfield myhash) command without other args
does not find a valid field, it will return an uninitialized lval.
```
debug set-active-expire no
hsetex myhash ex 1 fields 2 f1 v1 f2 v2
after 1s...
hrandfield myhash
[will return some uninitialized number]
```
related: https://github.com/valkey-io/valkey/issues/3021
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
In `dbSetValue()` the `old` pointer may be reassigned to point to the
incoming value object which was created without an embedded key, so
calling `dbUntrackKeyWithVolatileItems()` would call `objectGetKey()`
which returns NULL, causing a crash in `hashtableSdsHash()` when trying
to hash the NULL key.
Idea is to assign `old_was_hash_with_volatile` before the swap and use
`new` instead of `old` for untracking when theres no embedded key.
Introduced in #3003
Run with NULL ptr dereference:
https://github.com/valkey-io/valkey/actions/runs/20701343184/job/59424029880
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
In `dbSetValue` if the robj is a hash with volatile fields we need to
clean it up. It causes two core problems:
1. Type Confusion: Allows converting from hash to string with
`keys_with_volatile_items` tracked
2. Tracked Items with dangling pointer: Converting from `hash` to ie. a
string breaks this as `keys_with_volatile_items` is not cleaned up.
Check #3000
---------
Signed-off-by: frostzt <aidenfrostbite@gmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
HSETEX right now uses `strcmp` which does not account for
case-insensitivity replaced it with `strcasecmp`.
Fixes#2996
---------
Signed-off-by: frostzt <aidenfrostbite@gmail.com>
Signed-off-by: Sourav Singh Rawat <aidenfrostbite@gmail.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
If you send an `HSETEX key EXAT 0 FIELDS 1 foo bar`, it will create an
empty length string. This is not good behavior as other places in the
code will crash if an empty hash is accessed.
This change just makes it so that the key is freed if it ends up being
empty. This has some odd behavior w.r.t. to keyspace notifications
though.
Today, if you do `HSETEX key EXAT 0 FIELDS 1 foo bar` with a hash that
exists, it won't send an `HSET` notification or an `HEXPIRE`
notification unless foo is already present. It will also send a `DEL`
notification if foo is last the field, effectively deleting the key.
This isn't consistent with Redis, which will still send an `HSET` and
`HDEL` (*NOTE* not `hexpire`) notification if a key is added past the
expiration. Likewise, in the case where the key doesn't exist and
`HSETEX key EXAT 0 FIELDS 1 foo bar` is sent, Redis will send an HSET,
HDEL, and DEL notification in that order.
I think it makes sense to keep consistency with what we have today
(given that it's been out for awhile), but document the behavior.
Basically, if you send a command with an expiration in the past, we will
basically ignore the input unless it's "deleting" a key.
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Following Hash-Field-Expiration feature, a hash object can hold volatile
fields.
volatile fields which are already expired are deleted and reclaimed ONLY
by the active-expiration background job.
This means that hash object can contain items which have not yet
expired.
In case mutations are requesting to set a value on these
"already-expired" fields, they will be overwritten with the new value.
In such cases, though, it is requiered to update the global per-db
tracking map by removing the key if it has no more volatile fields.
This was implemented in all mutation cases of the hash commands but the
`INCRBY` and `INCRBYFLOAT`.
This can lead to a dangling object which has no volatile items, which
might lead to assertion during the active-expiration job:
example reproduction:
```
DEBUG SET-ACTIVE-EXPIRE 0
hset myhash f1 10
hexpire myhash 1 FIELDS 1 f1
sleep(10)
hincrby myhash f1 1
DEBUG SET-ACTIVE-EXPIRE 1
```
NOTE: we actually had tests for this scenario, only the test did not
include explicit assertion in case the item is still tracked after the
mutation.
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
When HEXPIRE commands are set with a time-in-the-past they are all
deleting the specified fields.
In such cases we allocate a temporal new argv in order to replicate
`HDEL`.
However in case no mutation was done (ie all fields do not exist) we do
not deallocate the temporal new_argv and there is a memory leak.
example:
```
HSET myhash field1 value1
1
HEXPIRE myhash 0 FIELDS 1 field2
-2
```
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
The CLUSTER SLOTS reply depends on whether the client is connected over
IPv6, but for a fake client there is no connection and when this command
is called from a module timer callback or other scenario where no real
client is involved, there is no connection to check IPv6 support on.
This fix handles the missing case by returning the reply for IPv4
connected clients.
Fixes#2912.
---------
Signed-off-by: Su Ko <rhtn1128@gmail.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Su Ko <rhtn1128@gmail.com>
Co-authored-by: KarthikSubbarao <karthikrs2021@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In #2078, we did not report large reply when copy avoidance is allowed.
This results in replies larger than 16384 not being recorded in the
commandlog large-reply. This 16384 is controlled by the hidden config
min-string-size-avoid-copy-reply.
Signed-off-by: Binbin <binloveplay1314@qq.com>
## Problem
IO thread shutdown can deadlock during server panic when the main thread
calls `pthread_cancel()` while the IO thread holds its mutex, preventing
the thread from observing the cancellation.
## Solution
Release the IO thread mutex before cancelling to ensure clean thread
termination.
## Testing
Reproducer:
```
bash
./src/valkey-server --io-threads 2 --enable-debug-command yes
./src/valkey-cli debug panic
```
Before: Server hangs indefinitely
After: Server terminates cleanly
Signed-off-by: Ouri Half <ourih@amazon.com>
Persist USE_FAST_FLOAT and PROG_SUFFIX to prevent a complete rebuild
next time someone types make or make test without specifying variables.
Fixes#2880
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
fedorarawhide CI reports these warnings:
```
networking.c: In function 'afterErrorReply':
networking.c:821:30: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
821 | char *spaceloc = memchr(s, ' ', len < 32 ? len : 32);
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
When we added the Hash Field Expiration feature in Valkey 9.0, some of
the new command docs included complexity description of O(1) even tough
they except multiple arguments.
(see discussion in
https://github.com/valkey-io/valkey/pull/2851#discussion_r2535684985)
This PR does:
1. align all the commands to the same description
2. fix the complexity description of some commands (eg HSETEX and
HGETEX)
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Only enable `HAVE_ARM_NEON` on AArch64 because it supports vaddvq and
all needed compiler intrinsics.
Fixes the following error when building for machine `qemuarm` using the
Yocto Project and OpenEmbedded:
```
| bitops.c: In function 'popcountNEON':
| bitops.c:219:23: error: implicit declaration of function 'vaddvq_u16'; did you mean 'vaddq_u16'? [-Wimplicit-function-declaration]
| 219 | uint32_t t1 = vaddvq_u16(sc);
| | ^~~~~~~~~~
| | vaddq_u16
| bitops.c:225:14: error: implicit declaration of function 'vaddvq_u8'; did you mean 'vaddq_u8'? [-Wimplicit-function-declaration]
| 225 | t += vaddvq_u8(vcntq_u8(vld1q_u8(p)));
| | ^~~~~~~~~
| | vaddq_u8
```
More details are available in the following log:
https://errors.yoctoproject.org/Errors/Details/889836/
Signed-off-by: Leon Anavi <leon.anavi@konsulko.com>
Fix a little miss in "Hash field TTL and active expiry propagates
correctly through chain replication" test in `hashexpire.tcl`.
The test did not wait for the initial sync of the chained replica and thus made the test flakey
Signed-off-by: Arad Zilberstein <aradz@amazon.com>
This PR fixes the freebsd daily job that has been failing consistently
for the last days with the error "pkg: No packages available to install
matching 'lang/tclx' have been found in the repositories".
The package name is corrected from `lang/tclx` to `lang/tclX`. The
lowercase version worked previously but appears to have stopped working
in an update of freebsd's pkg tool to 2.4.x.
Example of failed job:
https://github.com/valkey-io/valkey/actions/runs/19282092345/job/55135193499
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Related test failures:
*** [err]: Replica importing key containment (slot 0 from node 0 to 2) - DBSIZE command excludes importing keys in tests/unit/cluster/cluster-migrateslots.tcl
Expected '1' to match '0' (context: type eval line 2 cmd {assert_match "0" [R $node_idx DBSIZE]} proc ::test)
The reason is that we don't wait for the primary-replica synchronization
to complete before starting the next testcase.
---------
Signed-off-by: yzc-yzc <96833212+yzc-yzc@users.noreply.github.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
There’s an issue with the LTRIM command. When LTRIM does not actually
modify the key — for example, with `LTRIM key 0 -1` — the server.dirty
counter is not updated because both ltrim and rtrim values are 0. As a
result, the command is not propagated. However, `signalModifiedKey` is
still called regardless of whether server.dirty changes. This behavior
is unexpected and can cause a mismatch between the source and target
during propagation, since the LTRIM command is not sent.
Signed-off-by: Harry Lin <harrylhl@amazon.com>
Co-authored-by: Harry Lin <harrylhl@amazon.com>
This prevents crashes on the older nodes in mixed clusters where some
nodes are running 8.0 or older. Mixed clusters often exist temporarily
during rolling upgrades.
Fixes: #2341
Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>
After network failure nodes that come back to cluster do not always send
and/or receive messages from other nodes in shard, this fix avoids usage
of light weight messages to nodes with not ready bidirectional links.
When a light message comes before any normal message, freeing of cluster
link is happening because on the just established connection link->node
is not assigned yet. It is assigned in getNodeFromLinkAndMsg right after
the condition if (is_light).
So on a cluster with heavy pubsub load a long loop of disconnects is
possible, and we got this.
1. node A establishes cluster link to node B
2. node A propagates PUBLISH to node B
3. node B frees cluster link because of link->node == null as it has not
received non-light messages yet
4. go to 1.
During this loop subscribers of node B does not receive any messages
published to node A.
So here we want to make sure that PING was sent (and link->node was
initialized) on this connection before using lightweight messages.
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.com>
Since Valkey Sentinel 9.0, sentinel tries to abort an ongoing failover
when changing the role of a monitored instance. Since the result of the
command is ignored, the "FAILOVER ABORT" command is sent irrespective of
the actual failover status.
However, when using the documented pre 9.0 ACLs for a dedicated sentinel
user, the FAILOVER command is not allowed and _all_ failover cases fail.
(Additionally, the necessary ACL adaptation was not communicated well.)
Address this by:
- Updating the documentation in "sentinel.conf" to reflect the need for
an adapted ACL
- Only abort a failover when sentinel detected an ongoing (probably
stuck) failover. This means that standard failover and manual failover
continue to work with unchanged pre 9.0 ACLs. Only the new "SENTINEL
FAILOVER COORDINATED" requires to adapt the ACL on all Valkey nodes.
- Actually use a dedicated sentinel user and ACLs when testing standard
failover, manual failover, and manual coordinated failover.
Fixes#2779
Signed-off-by: Simon Baatz <gmbnomis@gmail.com>
Fixes an assert crash in _writeToClient():
serverAssert(c->io_last_written.data_len == 0 ||
c->io_last_written.buf == c->buf);
The issue occurs when clientsCronResizeOutputBuffer() grows or
reallocates c->buf while io_last_written still points to the old buffer
and data_len is non-zero. On the next write, both conditions in the
assertion become false.
Reset io_last_written when resizing the output buffer to prevent stale
pointers and keep state consistent.
fixes https://github.com/valkey-io/valkey/issues/2769
Signed-off-by: xbasel <103044017+xbasel@users.noreply.github.com>
This was introduced in #1826. This create an `Uninitialised value was
created by a heap allocation` in the CI.
Signed-off-by: Binbin <binloveplay1314@qq.com>
(cherry picked from commit e9ec72fcb0)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
Safe iterators pause rehashing, but don't pause auto shrinking. This
allows stale bucket references which then cause use after free (in this
case, via compactBucketChain on a deleted bucket).
This problem is easily reproducible via atomic slot migration, where we
call delKeysInSlot which relies on calling delete within a safe
iterator. After the fix, it no longer causes a crash.
Since all cases where rehashing is paused expect auto shrinking to also
be paused, I am making this happen automatically as part of pausing
reshashing.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
(cherry picked from commit 1e3e6dca72)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
When working on #2635 I errorneously duplicated the
setSlotImportingStateInAllDbs call for successful imports. This resulted
in us doubling the key count in the kvstore. This results in DBSIZE
reporting an incorrect sum, and also causes BIT corruption that can
eventually result in a crash.
The solution is:
1. Only call setSlotImportingStateInAllDbs once (in our
finishSlotMigrationJob function)
2. Make setSlotImportingStateInAllDbs idempotent by checking if the
delete from the kvstore importing hashtable is a no-op
This also fixes a bug where the number of importing keys is not lowered
after the migration, but this is less critical since it is only used
when resizing the dictionary on RDB load. However, it could result in
un-loadable RDBs if the importing key count gets large enough.
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
(cherry picked from commit 676c10131a)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
There will be two issues in this test:
```
test {FUNCTION - test function flush} {
for {set i 0} {$i < 10000} {incr i} {
r function load [get_function_code LUA test_$i test_$i {return 'hello'}]
}
set before_flush_memory [s used_memory_vm_functions]
r function flush sync
set after_flush_memory [s used_memory_vm_functions]
puts "flush sync, before_flush_memory: $before_flush_memory, after_flush_memory: $after_flush_memory"
for {set i 0} {$i < 10000} {incr i} {
r function load [get_function_code LUA test_$i test_$i {return 'hello'}]
}
set before_flush_memory [s used_memory_vm_functions]
r function flush async
set after_flush_memory [s used_memory_vm_functions]
puts "flush async, before_flush_memory: $before_flush_memory, after_flush_memory: $after_flush_memory"
for {set i 0} {$i < 10000} {incr i} {
r function load [get_function_code LUA test_$i test_$i {return 'hello'}]
}
puts "Test done"
}
```
The first one is the test output, we can see that after executing
FUNCTION FLUSH,
used_memory_vm_functions has not changed at all:
```
flush sync, before_flush_memory: 2962432, after_flush_memory: 2962432
flush async, before_flush_memory: 4504576, after_flush_memory: 4504576
```
The second one is there is a crash when loading the functions during the
async
flush:
```
=== VALKEY BUG REPORT START: Cut & paste starting from here ===
# valkey 255.255.255 crashed by signal: 11, si_code: 2
# Accessing address: 0xe0429b7100000a3c
# Crashed running the instruction at: 0x102e0b09c
------ STACK TRACE ------
EIP:
0 valkey-server 0x0000000102e0b09c luaH_getstr + 52
Backtrace:
0 libsystem_platform.dylib 0x000000018b066584 _sigtramp + 56
1 valkey-server 0x0000000102e01054 luaD_precall + 96
2 valkey-server 0x0000000102e01b10 luaD_call + 104
3 valkey-server 0x0000000102e00d1c luaD_rawrunprotected + 76
4 valkey-server 0x0000000102e01e3c luaD_pcall + 60
5 valkey-server 0x0000000102dfc630 lua_pcall + 300
6 valkey-server 0x0000000102f77770 luaEngineCompileCode + 708
7 valkey-server 0x0000000102f71f50 scriptingEngineCallCompileCode + 104
8 valkey-server 0x0000000102f700b0 functionsCreateWithLibraryCtx + 2088
9 valkey-server 0x0000000102f70898 functionLoadCommand + 312
10 valkey-server 0x0000000102e3978c call + 416
11 valkey-server 0x0000000102e3b5b8 processCommand + 3340
12 valkey-server 0x0000000102e563cc processInputBuffer + 520
13 valkey-server 0x0000000102e55808 readQueryFromClient + 92
14 valkey-server 0x0000000102f696e0 connSocketEventHandler + 180
15 valkey-server 0x0000000102e20480 aeProcessEvents + 372
16 valkey-server 0x0000000102e4aad0 main + 26412
17 dyld 0x000000018acab154 start + 2476
------ STACK TRACE DONE ------
```
The reason is that, in the old implementation (introduced in 7.0),
FUNCTION FLUSH
use lua_unref to remove the script from lua VM. lua_unref does not
trigger the gc,
it causes us to not be able to effectively reclaim memory after the
FUNCTION FLUSH.
The other issue is that, since we don't re-create the lua VM in FUNCTION
FLUSH,
loading the functions during a FUNCTION FLUSH ASYNC will result a crash
because
lua engine state is not thread-safe.
The correct solution is to re-create a new Lua VM to use, just like
SCRIPT FLUSH.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Co-authored-by: Ricardo Dias <ricardo.dias@percona.com>
(cherry picked from commit 6d4846b4d1)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
#2329 intoduced a bug that causes a blocked client in cluster mode to
receive two MOVED redirects instead of one. This was not seen in tests,
except in the reply schema validator.
The fix makes sure the client's pending command is cleared after sending
the MOVED redirect, to prevent if from being reprocessed.
Fixes#2676.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
(cherry picked from commit 394c809b98)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
DEBUG LOADAOF sometimes works but it results in -LOADING responses to
the primary so there are lots of race conditions. It isn't something we
should be doing anyways. To test, I just disconnect the replica before
loading the AOF, then reconnect it.
Fixes#2712
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
(cherry picked from commit 15c77bf158)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
This test was failing, and causing the next test to throw an exception.
It is failing since we never waited for the slot migration to connect
before proceeding.
Fixes#2692
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
(cherry picked from commit f846c8698d)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
The race condition causes the client to be used and subsequently double
freed by the slot migration read pipe handler. The order of events is:
1. We kill the slot migration child process during CANCELSLOTMIGRATIONS
1. We then free the associated client to the target node
1. Although we kill the child process, it is not guaranteed that the
pipe will be empty from child to parent
1. If the pipe is not empty, we later will read that out in the
slotMigrationPipeReadHandler
1. In the pipe read handler, we attempt to write to the connection. If
writing to the connection fails, we will attempt to free the client
1. However, the client was already freed, so this a double free
Notably, the slot migration being aborted doesn't need to be triggered
by `CANCELSLOTMIGRATIONS`, it can be any failure.
To solve this, we simply:
1. Set the slot migration pipe connection to NULL whenever it is
unlinked
2. Bail out early in slot migration pipe read handler if the connection
is NULL
I also consolidate the killSlotMigrationChild call to one code path,
which is executed on client unlink. Before, there were two code paths
that would do this twice (once on slot migration job finish, and once on
client unlink). Sending the signal twice is fine, but inefficient.
Also, add a test to cancel during the slot migration snapshot to make
sure this case is covered (we only caught it during the module test).
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
(cherry picked from commit 8f68208e03)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
When the hash object does not exist FXX should simply fail the check
without creating the object while FNX should be trivial and succeed.
Note - also fix a potential compilation warning on some COMPILERS doing
constant folding of variable length array when size is const expression.
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
(cherry picked from commit 71717cab5d)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
* Add cross version compatibility test to run with Valkey 7.2 and 8.0
* Add mechanism in TCL test to skip tests dynamically - #2711
---------
Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
(cherry picked from commit 6441a9f5a5)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
With #1401, we introduced additional filters to CLIENT LIST/KILL
subcommand. The intended behavior was to pick the last value of the
filter. However, we introduced memory leak for all the preceding
filters.
Before this change:
```
> CLIENT LIST IP 127.0.0.1 IP 127.0.0.1
id=4 addr=127.0.0.1:37866 laddr=127.0.0.1:6379 fd=10 name= age=0 idle=0 flags=N capa= db=0 sub=0 psub=0 ssub=0 multi=-1 watch=0 qbuf=0 qbuf-free=0 argv-mem=21 multi-mem=0 rbs=16384 rbp=16384 obl=0 oll=0 omem=0 tot-mem=16989 events=r cmd=client|list user=default redir=-1 resp=2 lib-name= lib-ver= tot-net-in=49 tot-net-out=0 tot-cmds=0
```
Leak:
```
Direct leak of 11 byte(s) in 1 object(s) allocated from:
#0 0x7f2901aa557d in malloc (/lib64/libasan.so.4+0xd857d)
#1 0x76db76 in ztrymalloc_usable_internal /workplace/harkrisp/valkey/src/zmalloc.c:156
#2 0x76db76 in zmalloc_usable /workplace/harkrisp/valkey/src/zmalloc.c:200
#3 0x4c4121 in _sdsnewlen.constprop.230 /workplace/harkrisp/valkey/src/sds.c:113
#4 0x4dc456 in parseClientFiltersOrReply.constprop.63 /workplace/harkrisp/valkey/src/networking.c:4264
#5 0x4bb9f7 in clientListCommand /workplace/harkrisp/valkey/src/networking.c:4600
#6 0x641159 in call /workplace/harkrisp/valkey/src/server.c:3772
#7 0x6431a6 in processCommand /workplace/harkrisp/valkey/src/server.c:4434
#8 0x4bfa9b in processCommandAndResetClient /workplace/harkrisp/valkey/src/networking.c:3571
#9 0x4bfa9b in processInputBuffer /workplace/harkrisp/valkey/src/networking.c:3702
#10 0x4bffa3 in readQueryFromClient /workplace/harkrisp/valkey/src/networking.c:3812
#11 0x481015 in callHandler /workplace/harkrisp/valkey/src/connhelpers.h:79
#12 0x481015 in connSocketEventHandler.lto_priv.394 /workplace/harkrisp/valkey/src/socket.c:301
#13 0x7d3fb3 in aeProcessEvents /workplace/harkrisp/valkey/src/ae.c:486
#14 0x7d4d44 in aeMain /workplace/harkrisp/valkey/src/ae.c:543
#15 0x453925 in main /workplace/harkrisp/valkey/src/server.c:7319
#16 0x7f2900cd7139 in __libc_start_main (/lib64/libc.so.6+0x21139)
```
Note: For filter ID / NOT-ID we group all the option and perform
filtering whereas for remaining filters we only pick the last filter
option.
---------
Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>
(cherry picked from commit 27a4ba055e)
Signed-off-by: cherukum-amazon <cherukum@amazon.com>
We have relaxed the `cluster-ping-interval` and `cluster-node-timeout`
so that cluster has enough time to stabilize and propagate changes.
Fixes this test occasional failure when running with valgrind:
[err]: Node #10 should eventually replicate node #5 in
tests/unit/cluster/slave-selection.tcl
#10 didn't became slave of #5
Backported to the 9.0 branch in #2731.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
If we don't wait for the replica to resync, the migration may be
cancelled by the time the replica resyncs, resulting in a test failure
when we can't find the migration on the replica.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Seeing test failures due to this on the 9.0.0 branch:
```
[exception]: Executing test client: ERR Syntax error. Use: LOLWUT [columns rows] [real imaginary].
ERR Syntax error. Use: LOLWUT [columns rows] [real imaginary]
```
It turns out we are just providing the version as an argument, instead
of specifying which version to run on
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
For now, introduce this and have it do nothing. Eventually, we can use
this to negotiate supported capabilities on either end. Right now, there
is nothing to send or support, so it just accepts it and doesn't reply.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
# Problem
In the current slot migration design, replicas are completely unaware of
the slot migration. Because of this, they do not know to hide importing
keys, which results in exposure of these keys to commands like KEYS,
SCAN, RANDOMKEY, and DBSIZE.
# Design
The main part of the design is that we will now listen for and process
the `SYNCSLOTS ESTABLISH` command on the replica. When a `SYNCSLOTS
ESTABLISH` command is received from the primary, we begin tracking a new
slot import in a special `SLOT_IMPORT_OCCURRING_ON_PRIMARY` state.
Replicas use this state to track the import, and await for a future
`SYNCSLOTS FINISH` message that tells them the import is
successful/failed.
## Success Case
```
Source Target Target Replica
| | |
|------------ SYNCSLOTS ESTABLISH -------------->| |
| |----- SYNCSLOTS ESTABLISH ------>|
|<-------------------- +OK ----------------------| |
| | |
|~~~~~~~~~~~~~~ snapshot as AOF ~~~~~~~~~~~~~~~~>| |
| |~~~~~~ forward snapshot ~~~~~~~~>|
|----------- SYNCSLOTS SNAPSHOT-EOF ------------>| |
| | |
|<----------- SYNCSLOTS REQUEST-PAUSE -----------| |
| | |
|~~~~~~~~~~~~ incremental changes ~~~~~~~~~~~~~~>| |
| |~~~~~~ forward changes ~~~~~~~~~>|
|--------------- SYNCSLOTS PAUSED -------------->| |
| | |
|<---------- SYNCSLOTS REQUEST-FAILOVER ---------| |
| | |
|---------- SYNCSLOTS FAILOVER-GRANTED --------->| |
| | |
| (performs takeover & |
| propagates topology) |
| | |
| |------- SYNCSLOTS FINISH ------->|
(finds out about topology | |
change & marks migration done) | |
| | |
```
## Failure Case
```
Source Target Target Replica
| | |
|------------ SYNCSLOTS ESTABLISH -------------->| |
| |----- SYNCSLOTS ESTABLISH ------>|
|<-------------------- +OK ----------------------| |
... ... ...
| | |
| <FAILURE> |
| | |
| (performs cleanup) |
| | ~~~~~~ UNLINK <key> ... ~~~~~~~>|
| | |
| | ------ SYNCSLOTS FINISH ------->|
| | |
```
## Full Sync, Partial Sync, and RDB
In order to ensure replicas that resync during the import are still
aware of the import, the slot import is serialized to a new
`cluster-slot-imports` aux field. The encoding includes the job name,
the source node name, and the slot ranges being imported. Upon loading
an RDB with the `cluster-slot-imports` aux field, replicas will add a
new migration in the `SLOT_IMPORT_OCCURRING_ON_PRIMARY` state.
It's important to note that a previously saved RDB file can be used as
the basis for partial sync with a primary. Because of this, whenever we
load an RDB file with the `cluster-slot-imports` aux field, even from
disk, we will still add a new migration to track the import. If after
loading the RDB, the Valkey node is a primary, it will cancel the slot
migration. Having this tracking state loaded on primaries will ensure
that replicas partial syncing to a restarted primary still get their
`SYNCSLOTS FINISH` message in the replication stream.
## AOF
Since AOF cannot be used as the basis for a partial sync, we don't
necessarily need to persist the `SYNCSLOTS ESTABLISH` and `FINISH`
commands to the AOF.
However, considering there is work to change this (#59#1901) this
design doesn't make any assumptions about this.
We will propagate the `ESTABLISH` and `FINISH` commands to the AOF, and
ensure that they can be properly replayed on AOF load to get to the
right state. Similar to RDB, if there are any pending "ESTABLISH"
commands that don't have a "FINISH" afterwards upon becoming primary, we
will make sure to fail those in `verifyClusterConfigWithData`.
Additionally, there was a bug in the existing slot migration where slot
import clients were not having their commands persisted to AOF. This has
been fixed by ensuring we still propagate to AOF even for slot import
clients.
## Promotion & Demotion
Since the primary is solely responsible for cleaning up unowned slots,
primaries that are demoted will not clean up previously active slot
imports. The promoted replica will be responsible for both cleaning up
the slot (`verifyClusterConifgWithData`) and sending a `SYNCSLOTS
FINISH`.
# Other Options Considered
I also considered tracking "dirty" slots rather than using the slot
import state machine. In this setup, primaries and replicas would simply
mark each slot's hashtable in the kvstore as dirty when something is
written to it and we do not currently own that slot.
This approach is simpler, but has a problem in that modules loaded on
the replica would still not get slot migration start/end notifications.
If the modules on the replica do not get such notifications, they will
not be able to properly contain these dirty keys during slot migration
events.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
The CVE fixes had a formatting and external test issue that wasn't
caught because private branches don't run those CI steps.
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
New client flags in reported by CLIENT INFO and CLIENT LIST:
* `i` for atomic slot migration importing client
* `E` for atomic slot migration exporting client
New flags in return value of `ValkeyModule_GetContextFlags`:
* `VALKEYMODULE_CTX_FLAGS_SLOT_IMPORT_CLIENT`: Indicate the that client
attached to this context is the slot import client.
* `VALKEYMODULE_CTX_FLAGS_SLOT_EXPORT_CLIENT`: Indicate the that client
attached to this context is the slot export client.
Users could use this to monitor the underlying client info of the slot
migration, and more clearly understand why they see extra clients during
the migration.
Modules can use these to detect keyspace notifications on import
clients. I am also adding export flags for symmetry, although there
should not be keyspace notifications. But they would potentially be
visible in command filters or in server events triggered by that client.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
**Issue History:**
1. The flaky test issue "defrag didn't stop" was originally detected in
February 2025: https://github.com/valkey-io/valkey/issues/1746
Solution for 1746: https://github.com/valkey-io/valkey/pull/1762
2. Similar issue occurred recently:
https://github.com/valkey-io/valkey/actions/runs/16585350083/job/46909359496#step:5:7640
**Investigation:**
1. First, the issue occurs specifically to Active Defrag stream in
cluster mode.
2. After investigating `test_stream` in `memefficiency.tcl`, I found the
root cause is in defrag logic rather than the test itself - There are
still failed tests with the same error even if I tried different
parameters for the test.
3. Then I looked at malloc-stats and identified potential defrag issues,
particularly in the 80B bin where utilization only reaches ~75% after
defrag instead of the expected near 100%, while other bins show proper
defrag behavior - 80B actually is the size of a new stream(confirmed in
`t_stream.c`) that we add during test.
4. For 80B, after adding 200000 streams and fragmenting, `curregs `=
100030, after a lot of defrag cycles, there are still 122
nonfull-slabs/511 slabs with the remaining 446 items not defragged
(average 4/nonfull-slab).
**Detailed malloc-stats:**
- Total slabs: 511
- Non-full slabs: 122
- Full slabs: 511-122=389
- Theoretical maximum per slab: 256 items
- Allocated items in non-full slabs: 100030-389*256=446
- Average items per non-full slab: 446/122=3.66
**Root Cause:**
**There are some immovable items which prevent complete defrag**
**Problems in old defrag logic:**
1. The previous condition (we don't defrag if slab utilization > 'avg
utilization' * 1.125), the 12.5% threshold doesn’t work well with low
utilizations.
- Let's imagine we have 446 items in 122 nonfull-slabs (avg 3.66
items/nonfull-slab), let's say, e.g. we have 81 slabs with 5 items each
+41 slabs with 1 item each)
- 12.5% threshold: 3.66*1.125=4.11
- If those 41 single items are immovable, they actually lower the
average, so the rest 81 slabs will be above the threshold (5>4.11) and
will not be defragged - defrag didn't stop.
2. Distribution of immovable items across slabs was causing inconsistent
defragmentation and flaky test outcome.
- If those 41 single items are movable, they will be moved and the avg
will be 5, then 12.5% threshold: 5*1.125=5.625, so the rest 81 slabs
will fall below the threshold (5<5.625) and will be defragged - defrag
success.
- This can explain why we got flaky defrag tests.
**Final solution :**
1. Add one more condition before the old logic in `makeDefragDecision
`to trigger defragmentation when slab is less than 1/8 full (1/8
threshold (12.5%) chosen to align with existing utilization threshold
factor) - Ensures no low-utilization slabs left without defragged, and
stabilize the defrag behavior.
2. The reason why we have immovable items and how to handle them is
going to be investigate later.
3. Be sure to rebuild Valkey before testing it.
**Local test result:**
- Before fix:
pass rate 80.8% (63/78)
- After fix:
Test only stream: pass rate 100% (200/200)
Test the whole memefficiency.tcl: pass rate 100% (100/100)
Resolves#2398 , the "defrag didn't stop" issue, with help from @JimB123
@madolson
---------
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
Signed-off-by: asagegeLiu <liusalisa6363@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
As requested, here is a version of lolwut for 9 that visualizes a Julia
set with ASCII art.
Example:
```
127.0.0.1:6379> lolwut version 9
.............
......................
............................
......:::--:::::::::::::::.......
.....:::=+*@@@=--------=+===--::....
....:::-+@@@@&*+=====+%@@@@@@@@=-::....
.....:::-=+@@@@@%%*++*@@@@@@@@@&*=--::....
.....::--=++#@@@@@@@@##@@@@@@@@@@@@@@=::....
......:-=@&#&@@@@@@@@@@@@@@@@@@@@@@@@@%-::...
......::-+@@@@@@@@@@@@@@@@@@&&@@@#%#&@@@-::....
.......::-=+%@@@@@@@@@@@@@@@@#%%*+++++%@+-:.....
.......::-=@&@@@@@@@@@@@@@@@@&*++=====---::.....
.......:::--*@@@@@@@@@@@@@@@@@%++===----::::.....
........::::-=+*%&@@@@@@@@@&&&%*+==----:::::......
........::::--=+@@@@@@@@@@&##%*++==---:::::.......
.......:::::---=+#@@@@@@@@&&&#%*+==---:::::.......
........:::::---=++*%%#&&@@@@@@@@@+=---::::........
.......:::::----=++*%##&@@@@@@@@@@%+=--::::.......
......::::-----==++#@@@@@@@@@@@@@&%*+=-:::........
......:::---====++*@@@@@@@@@@@@@@@@@@+-:::.......
.....::-=++==+++**%@@@@@@@@@@@@@@@@#*=--::.......
....:-%@@%****%###&@@@@@@@@@@@@@@@@&+--:.......
....:-=@@@@@&@@@@@@@@@@@@@@@@@@@@@@@@=::......
...::+@@@@@@@@@@@@@@@&&@@@@@@@@%**@+-::.....
....::-=+%#@@@@@@@@@&%%%&@@@@@@*==-:::.....
....::--+%@@@@@@@%++==++*#@@@@&=-:::....
....:::-*@**@@+==----==*%@@@@+-:::....
.....:::---::::::::--=+@=--::.....
.........::::::::::::::.......
.........................
..................
...
Ascii representation of Julia set with constant 0.41 + 0.29i
Don't forget to have fun! Valkey ver. 255.255.255
```
You can pass in arbitrary rows and colums (it's best when rows is 2x
number of columns) and an arbitrary julia constant so it is repeatable.
Worst case it takes about ~100us on my m2 macbook, which should be fine
to make sure it's not taking too many system resources.
---------
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
The problem is that ACKs run on a set loop (once every second) and this
will happen every loop with hz 1.
Instead, we can do the ACK after running the main logic. We can also do
an additional optimization where we don't send an ACK from source to
target if we already sent some data this cron loop, since the target
will reset the ack timer on any data over the connection.
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
In standalone mode, after a switch over, the command that was originally
blocking on primary returns -REDIRECT instead of -UNBLOCKED when the
client has the redirect capability.
Similarly, in cluster mode, after a switch over, the blocked commands
receive a -MOVED redirect instead of -UNBLOCKED.
After this fix, the handling of blocked connections during a switch over
between standalone and cluster is nearly identical. This can be
summarized as follows:
Standalone:
1. Client that has the redirect capability blocked on the key on the
primary node will receive a -REDIRECT after the switch over completes
instead of -UNBLOCKED.
2. Readonly clients blocked on the primary or replica node will remain
blocked throughout the switch over.
Cluster:
1. Client blocked on the key served by the primary node will receive a
-MOVED instead of a probabilistic -UNBLOCKED error.
2. Readonly clients blocked on the key served by primary or replica node
will remain blocked throughout the switch over.
---------
Signed-off-by: cjx-zar <56825069+cjx-zar@users.noreply.github.com>
Co-authored-by: Simon Baatz <gmbnomis@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
This is already handled by `clusterRedirectBlockedClientIfNeeded`. With
the work we are doing in #2329, it makes sense to have an explicit test here
to prevent regression.
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Each insertion of a skiplist node requires generating a random level
(via the `zslRandomLevel` function), and some commands (such as
`zunionstore`) call the `zslRandomLevel` function multiple times.
Therefore, optimizing `zslRandomLevel` can significantly improve the
performance of these commands.
The main optimization approach is as follows:
1. Original logic: Each iteration called the `random` function, with a
0.25 probability of continuing to call `random` again. In the worst-case
scenario, it required up to 32 calls (though the probability of this
occurring is extremely low).
2. Optimized logic: We only need to call the `genrand64_int64` function
once. Each iteration uses only 2 bits of randomness, effectively
achieving the equivalent of 32 iterations in the original algorithm.
3. Additionally, the introduction of `__builtin_clzll` significantly
reduces CPU usage, which compiles into a single, highly efficient CPU
instruction (e.g., LZCNT on x86, CLZ on ARM) on supported hardware
platforms
4. Although I've explained a lot, the actual code changes are quite
minimal, so just look at the code directly.
---------
Signed-off-by: chzhoo <czawyx@163.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Adding comprehensive unit tests for SHA-256 implementation.
These tests verify:
1. Basic functionality with known test vectors (e.g., "abc")
2. Handling of large input data (4KB repeated 1000 times)
3. Edge case with repeated single-byte input (1 million 'a' characters)
The tests ensure compatibility with standard SHA-256 implementations and
will help detect regressions during future code changes.
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
We probably should close the correct `slot_migration_pipe_read`. It
should resolve the valgrind errors.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Following the decision in #2189, we need to fix this test because the
`extended-redis-compatibility` config option is not going to be
deprecated in 9.0.
This commit changes the test to postpone the deprecation of
`extended-redis-compatibility` until 10.0 release.
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
The reason is that the replication stream may not have yet reached
the replica for execution. We could add a wait_for_condition. We decided
to replace those assert calls with assert_replication_stream to verify
the contents of the replication stream rather than the commandstats.
```
*** [err]: Flush slot command propagated to replica in tests/unit/cluster/cluster-flush-slot.tcl
```
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
reduce the req and warmup time to finish in 6 hrs as the github workflow
times out after 6 hrs
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
When we adding atomic slot migration in #1949, we reused a lot of rdb save code,
it was an easier way to implement ASM in the first time, but it comes with some
side effect. Like we are using CHILD_TYPE_RDB to do the fork, we use rdb.c/rdb.h
function to save the snapshot, these mess up the logs (we will print some logs
saying we are doing RDB stuff) and mess up the info fields (we will say we are
rdb_bgsave_in_progress but actually we are doing slot migration).
In addition, it makes the code difficult to maintain. The rdb_save method uses
a lot of rdb_* variables, but we are actually doing slot migration. If we want
to support one fork with multiple target nodes, we need to rewrite these code
for a better cleanup.
Note that the changes to rdb.c/rdb.h are reverting previous changes from when
we was reusing this code for slot migration. The slot migration snapshot logic
is similar to the previous diskless replication. We use pipe to transfer the
snapshot data from the child process to the parent process.
Interface changes:
- New slot_migration_fork_in_progress info field.
- New cow_size field in CLUSTER GETSLOTMIGRATIONS command.
- Also add slot migration fork to the cluster class trace latency.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Co-authored-by: Jacob Murphy <jkmurphy@google.com>
Set free method for deferred_reply list to properly clean up
ClientReplyValue objects when the list is destroyed
Signed-off-by: Uri Yagelnik <uriy@amazon.com>
With #2604 merged, the `Node #10 should eventually replicate node #5`
started passing successfully with valgrind, but I guess we are seeing a
new daily failure from a `New Master down consecutively` test that runs
shortly after.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Resolves#2545
Followed the steps to reproduce the issue, and was able to get non-zero
`total_net_repl_output_bytes`.
```
(base) ~/workspace/valkey git:[fix-bug-2545]
src/valkey-cli INFO | grep total_net_repl_output_bytes
total_net_repl_output_bytes:1788
```
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
As discussed in https://github.com/valkey-io/valkey/issues/2579
Notably, I am exposing this feature as "Atomic Slot Migration" to
modules. If we want to call it something else, we should consider that
now (e.g.. "Replication-Based Slot Migration"?)
Also note: I am exposing both target and source node in the event. It is
not guaranteed that either target or source would be the node the event
fires on (e.g. replicas will fire the event after replica containment is
introduced). Even though it could be potentially inferred from CLUSTER
SLOTS, it should help modules parse it this way. Modules should be able
to infer whether it is occurring on primary/replica from `ctx` flags, so
not duplicating that here.
Closes#2579
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
When we introduced the new Hash fields expiration functionality,
we decided to combine the current active expiration job between generic
keys and hash fields.
During that job we run a tight loop. In each loop iteration we scan over
maximum of 20 keys (with default expire effort) and try to "expire"
them.
For hash fields expiration job, the "expire" of a hash key, means
expiring maximum of 80 fields (with default expire effort).
The problem is that we might do much more work per each iteration of
hash fields expiration job.
The current code is shared between the 2 jobs, and currently we only
perform time check every 16 iterations.
as a result the CPU of fields active expiration can spike and consume
much higher CPU% than the current 25% bound allows.
Example:
Before this PR
| Scenario | AVG CPU | Time to expire all fields |
|----------------------------------------------------|---------|---------------------------|
| Expiring 10M volatile fields from a single hash | 20.18% | 26 seconds
|
| Expiring 10M volatile fields from 10K hash objects | 32.72% | 7
seconds |
After this PR
Scenario | AVG CPU | Time to expire all fields
| Scenario | AVG CPU | Time to expire all fields |
|----------------------------------------------------|---------|---------------------------|
| Expiring 10M volatile fields from a single hash | 20.23%. | 26 seconds
|
| Expiring 10M volatile fields from 10K hash objects | 20.76%. | 11
seconds |
*NOTE*
The change introduced here make the field job check the time every
iteration. We offer compile time option to use efficient time check
using TSC (X86) or VCR (ARM) on most modern CPU, so the impact is
expected to be low. Still, in order to avoid degradation for existing
workloads, the code change was made so it will not impact the existing
generic keys active expiration job.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Interactive use of valkey-cli often involves working with long keys
(e.g. MY:INCREDIBLY:LONG:keythattakesalongtimetotype). In shells like
bash, zsh, or psql, users can quickly move the cursor word by word with
**Alt/Option+Left/Right**, **Ctrl+Left/Right** or **Alt/Option+b/f**.
This makes editing long commands much more efficient.
Until now, valkey-cli (via linenoise) only supported single-character
cursor moves, which is painful for frequent key editing.
This patch adds such support, with simple code changes in linenoise. It
now supports both the Meta (Alt/Option) style and CSI (control sequence
introducer) style:
| | Meta style | CSI style (Ctrl) | CSI style (Alt) |
| --------------- | ---------- | ---------------- | --------- |
| move word left | ESC b | ESC [1;5D | ESC [1;3D |
| move word right | ESC f | ESC [1;5C | ESC [1;3C |
Notice that I handle these two styles differently since people have
different preference on the definition of "what is a word".
Specifically, I define:
- "sub-word": just letters and digits. For example "my:namespace:key"
has 3 sub-words. This is handled by Meta style.
- "big-word": as any character that is not space. For example
"my:namespace:key" is just one single big-word. This is handled by CSI
style.
## How I verified
I'm using MacOS default terminal (`$TERM = xterm-256color`). I
customized the terminal keyboard setting to map option + left to `\033b`
, and ctrl + left to `\033[1;5D` so that I can produce both the Meta
style and CSI style. This code change should also work for
Linux/BSD/other terminal users.
Now the valkey-cli works like the following. `|` shows where the cursor
is currently at.
Press Alt + left (escape sequence `ESC b` ):
```
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
```
Press Ctrl + left (escape sequence `ESC [1;5D` ):
```
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
```
Press Alt + right (escape sequence `ESC f` ):
```
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
```
Press Ctrl + right (escape sequence `ESC [1;5C` ):
```
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
set cache:item itemid
|
```
---------
Signed-off-by: Zhijun <dszhijun@gmail.com>
New config options:
* cluster-announce-client-port
* cluster-announce-client-tls-port
If enabled, clients will always get to see the configured port for a
node instead of the internally announced port(s), the same way that
`cluster-announce-client-ipv4` and `cluster-announce-client-ipv6` work.
Cluster-internal communication uses the non-client variant of these
options.
The configuration is propagated throughout the cluster using new ping
extensions.
Closes#2377
---------
Signed-off-by: Marvin Rösch <marvinroesch99@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In #1604, we attempt to read future Valkey RDB formats, but we rejected
foreign RDB formats, because of the risk that the opcodes and types
added by other projects collide with the new types and opcodes added in
recent Valkey versions.
This change accepts foreign RDB versions but limits the types and
opcodes to the ones that we can understand, to prevent misinterpretation
of types/opcodes which could lead to undefined behavior. If unsupported
RDB types or opcodes are seen, we error out.
Additional changes:
* Improve error reporting when encountering unknown RDB types in relaxed
version check mode.
* Tests for loading various RDB files.
* Improvement to valkey-check-rdb to accept future and foreign RDB
versions, including tests.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
As per #2459, this PR removes deprecation and `deprecated_since`
element and `DEPRECATED` doc flag from commands. Closes#2459.
---------
Signed-off-by: Kyle J. Davis <kyledvs@amazon.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
`bitops.c`: `serverPopcount()` used `popcountAVX2()`, which as the name
implies requires AVX2 support, on AVX-only machines, causing an "illegal
instruction" error.
Added a `__builtin_cpu_supports("avx2")` check and falling back to the
platform agnostic version if AVX2 is not supported.
Fixes#2570
Signed-off-by: Ted Lyngmo <ted@lyncon.se>
In Valkey 9.0 we added HFE support which is currently using a per slot
hashtable in order to track keys (hash objects) which contains volatile
fields. in order to optimize the RDB load we should use the same method
for expires and generic keys kvstores.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Delete the out-of-date comment explaining why changed latency from 5 to
40 in #2421 , which is a leftover of #2553
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
This flag.deny_blocking check causes some code to becomde dead code.
Some of the code below checks islua or ismulti, but they are marked
with flag.deny_blocking and will return early.
In addition, cleanup some documents, some of them are inaccurate,
and restore the code of blocking_auth_cb in tests/modules/auth.c,
this reply should be returned from core. The reply used to from the
core and was changed to from the module in #1819, and now it is from
the core again.
Cleanup some dead code around #1819.
Signed-off-by: Binbin <binloveplay1314@qq.com>
In this commit we introduce a new module API event called
`ValkeyModuleEvent_AuthenticationAttempt` to track successful/failed
authentication attempts.
This event will fill a struct, called `ValkeyModuleAuthenticationInfo`,
with the client ID of the connection, the username, the module name that
handle the authentication event, and the result of the authentication
attempt.
Fixes: #2211
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
This commit refactors the scripting engine to support multiple cached
module contexts per engine, rather than relying on a single cached
`ValkeyModuleCtx` object.
Previously, having only one cached context object caused data races over
the state stored in the context object, because it's possible that a
script that is running for a long time to yield and the server event
loop may call the `scriptingEngineCallGetMemoryInfo` function to get the
scripting engine memory information, which re-uses the same cached
context object. Another possible data-race is caused by the asynchronous
scripts flush, which calls the `scriptingEngineCallFreeFunction`
function in an background thread, and also re-uses the cached context
object.
To address this, a cache array of module contexts was introduced in the
scripting engine structure, with each slot dedicated to a specific use
case—such as script execution, memory info queries, or function freeing.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Changed the defrag hit threshold from 512 to 1 in the `defragLaterStep`
& `defragStageKvstoreHelper` function to reduce defrag latency. (idea
from Jim)
1. Previously, the defrag loop would continue processing up to 512
successful defragmentations before checking if the time limit was
exceeded. Now it checks after every single successful allocation move.
2. The trade-off is slightly more frequent time checks, but the time
check (~19ns) is negligible compared to the actual defragmentation work
(even a single 8-byte reallocation takes ~43ns and the
allocatorShouldDefrag function call takes ~49ns per block). This
overhead is minimal compared to the latency improvement gained from
better time management during active defragmentation.
3. Also revert the change from
https://github.com/valkey-io/valkey/pull/2421/files to test.
4. Solved compilation issue with unsigned by changing the type of the
local variable `prev_defragged `to match the type of
`server.stat_active_defrag_hits`
Closes#2444
---------
Signed-off-by: Alina Liu <alinalq@dev-dsk-alinalq-2b-2db84246.us-west-2.amazon.com>
Co-authored-by: Alina Liu <alinalq@dev-dsk-alinalq-2b-2db84246.us-west-2.amazon.com>
In script, client will be replaced with its caller, so commandlog needs
to use the metrics of the client that currently executing the command.
Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
Previously CONFIG RESETSTATS only resets the slot statistics in
cluster part, this PR makes it reset cluster bus messages at well.
Additionally, we also reset stat_cluster_links_buffer_limit_exceeded.
Now we will reset:
- cluster_stats_messages_sent*
- cluster_stats_messages_received*
- total_cluster_links_buffer_limit_exceeded
Closes#2439.
Signed-off-by: Hongrui <2086160503@qq.com>
The old SLOT_EXPORT_AUTHENTICATING added in #1949, when processed by the source node,
we will send the AUTH command and then reads the response. If the target node is blocked
during this process, the source node will also be blocked. We should use a read handler
to handle this. We split SLOT_EXPORT_AUTHENTICATING into SLOT_EXPORT_SEND_AUTH and
SLOT_EXPORT_READ_AUTH_RESPONSE to avoid this issue.
Signed-off-by: Binbin <binloveplay1314@qq.com>
If all cluster nodes have functions, slot migration will fail
since the target will return the function already exists error
when doing the FUNCTION LOAD.
And in addition, the target's replica could panic when it executes
the FUNCTION LOAD propagated from the primary (see
propagation-error-behavior).
Introduced in #1949.
Signed-off-by: Binbin <binloveplay1314@qq.com>
It prints an extra ": " after the message, which is a bit weird, i
thought it was printing cluster-bus port information, but it was not.
```
Moving slot 5458 from 127.0.0.1:30001 to 127.0.0.1:30003:
Moving slot 5459 from 127.0.0.1:30001 to 127.0.0.1:30003:
Moving slot 5460 from 127.0.0.1:30001 to 127.0.0.1:30003:
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
Try to fix the failures seen for `test "PSYNC2 #3899 regression: verify
consistency"`.
This change resets the query buffer parser state in
`replicationCachePrimary()` which is called when the connection to the
primary is lost. Before #2092, this was done by `resetClient()`.
The solution was inspired by the discussion about the regression
mentioned (discussion from 2017) and the related commits from that time:
3dd60a77d06510ff4b1d524a14731837b9aa0d4b,
62845798cc,
8baea97eff.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
tiny change. It failed once for me (a little time passed and it returned
2 seconds instead of 3), so I figured it's probably a little flaky for
others too
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Both LMOVE and BLMOVE can return null values because the source key is
empty.
PR changes include
- change the LMOVE reply_schema to include the possibility of a nil
return value
- Add comment to BLMOVE reply_schema to indicate it can return nil
because the source does not exist
This fixes#2532
---------
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
This gets rid of the need to use a void* as a carrier for the worker
number. Instead a pointer to the relevant worker data is passed to the
started thread.
Fixes#2529
---------
Signed-off-by: Ted Lyngmo <ted@lyncon.se>
In C99, we had to use `#define static_assert(expr, lit) extern char
__static_assert_failure[(expr) ? 1 : -1]` for static assertions.
However, we now have native support for static_assert with
_Static_assert. Previously one of the correct #defines was getting
called first, setting it to _Static_assert, however after
https://github.com/valkey-io/valkey/commit/116de0a7940cc56c01b2f30f3e7070cef3d5eaf7
the first import defining the symbol was "serverassert.h", which
included the old style.
This change removes all unnecessary imports and always defines
static_assert as _Static_assert.
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
This may result in meaningless slot migration job, we should
return an error to user in advance to avoid operation error.
Also `by myself` is not correct English grammar and `myself`
is a internal code terminology, changed to `by this node`.
Was introduced in #1949.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Instead of parsing only one command per client before executing it,
parse multiple commands from the query buffer and batch-prefetch the
keys accessed by the commands in the queue before executing them.
This is an optimization for pipelined commands, both with and without
I/O threads. The optimization is currently disabled for the replication
stream, due to failures (probably caused by how the replication offset
is calculated based on the query buffer offset).
* When parsing commands from the input buffer, multiple commands are
parsed and stored in a command queue per client.
* In single-threaded mode (I/O threads off) keys are batch-prefetched
before the commands in the queue are executed. Multi-key commands like
MGET, MSET and DEL benefit from this even if pipelining is not used.
* Prefetching when I/O threads are used does prefetching for multiple
clients in parallel. This code takes client command queues into account,
improving prefetching when pipelining is used. The batch size is
controlled by the existing config `prefetch-batch-max-size` (default
16), which so far only was used together with I/O threads. The config is
moved to a different section in `valkey.conf`.
* When I/O threads are used and the maximum number of keys are
prefetched, a client's command is executed, then the next one in the
queue, etc. If there are more commands in the queue for which the keys
have not been prefetched (say the client sends 16 pipelined MGET with 16
keys in each) keys for the next few commands in the queue are prefetched
before the commands is executed if prefetching has not been done for the
next command. (This utilizes the code path used in single-threaded
mode.)
Code improvements:
* Decoupling of command parser state and command execution state:
* The variables reqtype, multibulklen and bulklen refer to the current
position in the query buffer. These are no longer reset in resetClient
(which runs after each command being executed). Instead, they are
reset in the parser code after each completely parsed command.
* The command parser code is partially decoupled from the client struct.
The query buffer is still one per client, but the resulting argument
vector is stored in caller-defined variables.
Fixes#2044
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
I believe this was a debug log at the time, and its printing
was quite annoying locally. The test is quite simple so i think
we can just remove it.
Signed-off-by: Binbin <binloveplay1314@qq.com>
# Adds On-demand Benchmark Workflow
This PR introduces a new GitHub Actions workflow that enables on-demand
performance benchmarking for pull requests through label-based triggers.
This uses the new framework
[valkey-perf-benchmark](https://github.com/valkey-io/valkey-perf-benchmark)
developed for standard benchmarks and PR benchmarking.
## What is being added by this PR
- **Workflow File**: `.github/workflows/benchmark_on_label.yml`
- **Trigger**: Activated when specific labels are added to PRs
- **Supported Labels**:
- `run-benchmark` - Runs standard benchmarks
## Features
### On-Demand Execution
- Benchmarks run only when explicitly requested via PR labels
- No automatic execution to avoid unnecessary resource usage
### Performance Comparison
- Compares PR performance against the `unstable` baseline
- Generates detailed comparison reports
- Posts results directly as PR comments for easy review
### Flexible Configuration
- Currently uses github runners by will use dedicated performance
runners (`ec2-perf-ubuntu-24`)
- Configurable benchmark suites via JSON config files
### Artifact Management
- Uploads benchmark results as workflow artifacts
- Preserves both baseline and PR metrics for analysis
- Includes comparison markdown for offline review
### Automatic Cleanup
- Removes trigger labels after execution (success or failure)
- Prevents accidental re-runs from stale labels
## Usage
To run benchmarks on a PR:
1. Add the `run-benchmark` label for standalone and cluster, non-tls
mode
2. Wait for the workflow to complete
3. Review results in the automated PR comment
This workflow enhances our CI/CD pipeline by providing easy access to
performance testing without impacting regular development workflows.
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Signed-off-by: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.com>
* Add a flag `VALKEYMODULE_CLIENTINFO_FLAG_READONLY` to
`ValkeyModuleClientInfo.flags` and set it in
`ValkeyModule_GetClientInfoById()`.
* Add an optimization for accessing the current client by ID, to avoid
looking it up in a radix tree.
Closes#2487
---------
Signed-off-by: Allen Samuels <allenss@amazon.com>
## Problem
Test `Cluster module receive message API -
VM_RegisterClusterMessageReceiver` fails sporadically in CI with
"Expected '2' to be equal to '1'". The test failed in Daily CI 4 days
ago but hasn't failed since, indicating flaky behavior that I cannot
reproduce locally.
## Hypothesis
The failing line `assert_equal 2 [count_log_message 0 "* <cluster> DONG
(type 2) RECEIVED*"]` counts DONG message entries in node 0's log file
and expects exactly 2. The failure suggests a possible race condition
where there's a timing gap between when cluster statistics are updated
and when the corresponding log entries become visible in the log file.
## Solution
Add `wait_for_condition` to ensure log messages are written before
checking count:
```tcl
wait_for_condition 50 100 {
[count_log_message 0 "* <cluster> DONG (type 2) RECEIVED*"] eq 2
} else {
fail "node 1 didn't log 2 DONG messages"
}
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.com>
In the current implementation, the second list was never rewound again
once it was iterated. So for the first element of `ranges1`, `ranges2`
was iterated fully. But when the second element of `ranges1` was
processed, the `ranges2` was not rewound again.
With this change, for every element of `ranges1`, we start from the
beginning for `ranges2`
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Command: `./runtest --single unit/bitops --loops 3`
Unstable
```
[ignore]: large memory flag not provided
[-1/1 done]: unit/bitops (4 seconds)
[ignore]: large memory flag not provided
[0/1 done]: unit/bitops (4 seconds)
[ignore]: large memory flag not provided
[1/1 done]: unit/bitops (4 seconds)
The End
Execution time of different units:
4 seconds - unit/bitops
4 seconds - unit/bitops
4 seconds - unit/bitops
```
After fix
```
[1/3 done]: unit/bitops (4 seconds)
[ignore]: large memory flag not provided
[2/3 done]: unit/bitops (4 seconds)
[ignore]: large memory flag not provided
[3/3 done]: unit/bitops (4 seconds)
The End
Execution time of different units:
4 seconds - unit/bitops
4 seconds - unit/bitops
4 seconds - unit/bitops
```
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
currently hsetex is verifying the number of fields matches the provided
number of fields by using div. instead it can match to the
multiplication in order to prevent rounding the verified value down.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Fixes: #2460
With this change we avoid divergence in cluster and replication layer
view. I've observed node can be marked as primary in cluster while it
can be marked as replica in replication layer view and have a active
replication link. Without this change, we used to end up in a invalid
replica chain in replication layer.
---------
Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>
This change avoids additional failure report creation if the node is
already marked as failed. The failure report count has never been used
after a node has been marked as failed. So, there is no value addition
in maintaining it further. This reduces operation of both add and delete
failure report. Hence, the performance benefit.
We can observe an avg. of 10% reduction in p99 CPU utilization (in a 2000
nodes cluster (1000 primary/ 1000 replica) with 330 nodes in failed
state with this change.
---------
Signed-off-by: Seungmin Lee <sungming@amazon.com>
Fixes#2372
## Description
This PR adds an automated workflow that assigns PR authors to their own
pull requests when opened or reopened.
## Changes
- Added `.github/workflows/auto-author-assign.yml` workflow
- Uses `toshimaru/auto-author-assign@v2.1.1` action
- Triggers on `pull_request_target` events (opened, reopened)
- Requires `pull-requests: write` permission
## Benefits
- Improves PR tracking and organization
- Automatically assigns responsibility to PR authors
- Reduces manual assignment overhead for maintainers
- Helps contributors track their own PRs more easily
## Testing
✅ **Tested on my fork before submission:**
1. Merged the workflow into my fork's unstable branch
2. Created a test PR within my fork
3. Verified that I was automatically assigned as the assignee
4. Screenshot showing successful auto-assignment:
<img width="1278" height="684" alt="Screenshot 2025-08-01 at 3 39 05 PM"
src="https://github.com/user-attachments/assets/9ad643be-5eac-4ad6-bec7-184cf22e9cbd"
/>
The workflow executed successfully and assigned me to the test PR as
expected.
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
If we want to expand kvstoreHashtableExpand, we need to make sure the
hashtable exists. Currently, when processing RDB slot-info, our expand
has no effect because the hashtable does not exist (we initialize it only
when we need it).
We also update kvstoreExpand to use the kvstoreHashtableExpand to make
sure there is only one code path. Also see #1199 for more details.
Signed-off-by: Binbin <binloveplay1314@qq.com>
several fixes:
1. fix not using bool input type for hashTypeIgnoreTTL - probably lost
during the 3 HFE PR merges
2. remove vset change hashtable encoding to single - The current code is
a bug. The entry is probably expired (or about to be expired soon) so we
can leave it as a hashtable till it does.
3. enable incremental rehashing for volatile item keys kvstore - This is
the center issue of this PR. without it the activeexpiry might not scan
the kvstore which is very fragmented with lots of empty buckets.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
In clusters with a very short node timeout such as 2-3 seconds, the
extra failover delay of 500-1000 milliseconds (500 + random value 0-500;
total 750 on average) before initiating a failover is a significant
extra downtime to the cluster. This PR makes this delay relative to node
timeout, using a shorter failover delay for a smaller configured node
timeout. The formula is `fixed_delay = min(500, node_timeout / 30)`.
| Node timeout | Fixed failover delay |
|---------------|----------------------|
| 15000 or more | 500 (same as before) |
| 7500 | 250 |
| 3000 | 100 |
| 1500 | 50 |
Additional change: Add an extra 500ms delay to new replicas that may not
yet know about the other replicas. This avoids the scenario where a new
replica with no data wins the failover. This change turned out to be
needed to for the stability of some test cases.
The purposes of the failover delay are
1. Allow FAIL to propagate to the voting primaries in the cluster
2. Allow replicas to exchange their offsets, so they will have a correct
view of their own rank.
A third (undocumented) purpose of this delay is to allow newly added
replicas to discover other replicas in the cluster via gossip and to
compute their rank, to realize it's are not the best replica. This case
is mitigated by adding another 500ms delay to new replicas, i.e. if it
has replication offset 0.
A low node timeout only makes sense in fast networks, so we can assume
that the above needs less time than in a cluster with a higher node
timeout.
These delays don't affect the correctness of the algorithm. They are
just there to increase the probability that a failover will succeed by
making sure that the FAIL message has enough time to propagate in the
cluster and to the random part is to reduce the probability that two
replicas initiates the failover at the same time.
The typical use case is when data consistency matters and writes can't
be skipped. For example, in some application, we buffer writes in the
application during node failures to be able to apply them when the
failover is completed. The application can't buffer them for a very long
time, so we need the cluster to be up again within e.g. 5 seconds from
the time a node starts to fail.
I hope this PR can be considered safer than #2227, although the two
changes are orthogonal.
Part of issue #2023.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
We now pass in rdbSnapshotOptions options in this function, and
options.conns
is now malloc'ed in the caller side, so we need to zfree it when
returning early
due to an error. Previously, conns was malloc'ed after the error
handling, so we
don't have this.
Introduced in #1949.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
Previously, the config names and values were stored in a temporary dict.
This ensures that no duplicates are returned, but it also makes the
order random.
In this PR, the config names and values still stored in the temporary
dict, but then they are copied to an array, which is sorted, before the
reply is sent.
Resolves#2042
---------
Signed-off-by: yzc-yzc <96833212+yzc-yzc@users.noreply.github.com>
We recently introduced a new template to create `test failures` issues
from a template. This change makes this template visible in the
`CONTRIBUTING.md` file. Also, added a tip to paste the stack trace since
outputs of CI links can expire.
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
* Use pipelines of length 1000 instead of up to 200000.
* Use CLIENT REPLY OFF instead of reading and discarding the replies.
Fixes#2205
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Similar to dicts, we disallow resizing while the hashtable is
rehashing. In the previous code, if a resize was triggered during
rehashing, like if the rehashing wasn't fast enough, we would do
a while loop until the rehashing was complete, which could be a
potential issue when doing resize.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
The change will ensure that the slot is present on the node before the
slot is populated. This will avoid the errors during populating the
slot.
Resolves#2480
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Previously, each slot migration was logged individually, which could
lead to log spam in scenarios where many slots are migrated at once.
This commit enhances the logging mechanism to group consecutive slot
migrations into a single log entry, improving log readability and
reducing noise.
Log snippets
```
1661951:S 13 Aug 2025 15:47:10.132 * Slot range [16383, 16383] is migrated from node c3926da75f7c3a0a1bcd07e088b0bde09d48024c () in shard 7746b693330c0814178b90b757e2711ebb8c6609 to node 2465c29c8afb9231525e281e5825684d0bb79f7b () in shard 39342c039d2a6c7ef0ff96314b230dfd7737d646.
1661951:S 13 Aug 2025 15:47:10.289 * Slot range [10924, 16383] is migrated from node 2465c29c8afb9231525e281e5825684d0bb79f7b () in shard 39342c039d2a6c7ef0ff96314b230dfd7737d646 to node c3926da75f7c3a0a1bcd07e088b0bde09d48024c () in shard 7746b693330c0814178b90b757e2711ebb8c6609.
1661951:S 13 Aug 2025 15:47:10.524 * Slot range [10924, 16383] is migrated from node c3926da75f7c3a0a1bcd07e088b0bde09d48024c () in shard 7746b693330c0814178b90b757e2711ebb8c6609 to node 2465c29c8afb9231525e281e5825684d0bb79f7b () in shard 39342c039d2a6c7ef0ff96314b230dfd7737d646.
```
---------
Signed-off-by: Ping Xie <pingxie@google.com>
In #2431 we changed the assert to a if condition, and the test cause
some trouble, now we just remove the assert (if condition) and disable
the test for now due to #2441.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Currently HSETEX always generate `hset` notification. In order to align
with generic `set` command, it should only generate `hset` if the
provided time-to-live is a valid future time.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
(1) The old logic may result in the RDMA event being acknowledged
unexpectly in the following two scenarios.
* ibv_get_cq_event get an EAGAIN error.
* ibv_get_cq_event get one event but it may ack multiple times in the
pollcq loop.
(2) In the benchmark result of valkey over RDMA, the tail latency is as
high as 177 milliseconds(almost 80x of TCP). This results from incorrect
benchmark client setup which includes the connection setup time into the
benchmark latency recording. This patch fixes this crazy tail latency
issue by modifying the valkey-benchmark.c. This change only affects
benchmark over RDMA as updates are regulated under Macro USE_RDMA.
There are following updates on valkey RDMA but I am willing to create
separated pull requests.
---------
Signed-off-by: Ruihong Wang <ruihong@google.com>
<textx="378"y="322"font-family="Inter,system-ui,sans-serif"font-size="30"fill="#ffffff"opacity=".66">Ultra-fast key-value store with Redis API compatibility</text>
- These instructions apply to the entire repository unless a deeper `AGENTS.md` overrides them.
## Repo overview
- This is the Valkey server codebase.
- The main implementation lives under `src/`.
- Unit tests live under `src/unit`
- Integration tests live under `tests/`.
- Top-level `Makefile` forwards most targets into `src/Makefile`.
## Working guidelines
- Keep changes minimal and easy to backport.
- Match the style of the surrounding code instead of introducing new patterns.
- Avoid unrelated refactors in the same change.
## Build
- Default build: `make`
- Clean rebuild when build settings or bundled deps change: `make distclean && make`
## Unit tests
- Unit tests live under `src/unit/` and use GoogleTest (gtest/gmock).
- Build and run all unit tests: `make -C src test-unit`
- Unit tests cover data-structure and low-level logic changes.
- A single test filter can be run with: `make -C src test-unit && ./src/unit/valkey-unit-gtests --gtest_filter='<TestSuite>.<TestName>'`
## Integration tests
- Integration tests live under `tests/` and are written in Tcl.
- Run the full integration suite: `make test` (from the repo root).
- Run a single test file: `./runtest --single <path/to/test.tcl>`
- Additional specialized suites:
- Cluster tests: `./runtest-cluster`
- Sentinel tests: `./runtest-sentinel`
- Module API tests: `./runtest-moduleapi`
- For targeted validation, run the smallest relevant test scope first before broader suites.
## Code style
- Follow the repository conventions described in `DEVELOPMENT_GUIDE.md`.
- Most formatting is enforced by `clang-format`.
- CI uses `clang-format-18` across `*.c`, `*.h`, `*.cpp`, and `*.hpp` files.
- When touching C/C++ sources or headers, run `clang-format-18 -i` on the modified files before finalizing when the tool is available.
- Use comments for non-obvious behavior and rationale, not for restating code.
## Tests
- Code changes should include relevant tests when the repo already has a matching test location.
- Data-structure and low-level logic changes usually belong in `src/unit/` (C++ gtest).
- End-to-end behavior changes usually belong in `tests/` (Tcl integration tests).
- If behavior or commands change, check whether related documentation also needs updating.
## Files to avoid touching unless required
- Do not commit local runtime artifacts such as `dump.rdb`, `nodes.conf`, `*.log`, or ad hoc cluster directories unless the task explicitly requires them.
- Treat vendored dependency code under `deps/` as special-case changes; modify it only when the task clearly requires it.
## Pull Requests
Always push to the user's fork. Never push to the upstream valkey-io/valkey repository. Never push directly to unstable. If a user fork does not exist, ask the contributor to create one.
@@ -6,7 +6,8 @@ should be provided by the operating system.
* **linenoise** is a readline replacement. It is developed by the same authors of KV but is managed as a separated project and updated as needed.
* **lua** is Lua 5.1 with minor changes for security and additional libraries.
* **hdr_histogram** Used for per-command latency tracking histograms.
* **fast_float** is a replacement for strtod to convert strings to floats efficiently.
* **ffc.h** is a C99 port of the fast_float library, used as a replacement for strtod to convert strings to floats efficiently.
* **gtest-parallel** is a script for running googletest tests in parallel.
How to upgrade the above dependencies
===
@@ -107,17 +108,47 @@ We use a customized version based on master branch commit e4448cf6d1cd08fff51981
2. Copy updated files from newer version onto files in /hdr_histogram.
3. Apply the changes from 1 above to the updated files.
fast_float
ffc.h
---
The fast_float library provides fast header-only implementations for the C++ from_chars functions for `float` and `double` types as well as integer types. These functions convert ASCII strings representing decimal values (e.g., `1.3e10`) into binary types. The functions are much faster than comparable number-parsing functions from existing C++ standard libraries.
Specifically, `fast_float` provides the following function to parse floating-point numbers with a C++17-like syntax (the library itself only requires C++11):
@@ -69,7 +69,7 @@ Adhere to the existing coding style and make sure to mimic best possible.
### Code formatting
When making a change, please use `git clang-format` or [format-files.sh](./scripts/format-files.sh) to format your changes properly.
This repository is currently using `clang-format` 18.1.3 to format the code, which can be installed using `pip install clang-format==18.1.3` or other preferred method.
This repository is currently using `clang-format` 18.1.8 to format the code, which can be installed using `pip install clang-format==18.1.8` or other preferred method.
This document describes using `libkv` in standalone (non-cluster) mode, including an overview of the synchronous and asynchronous APIs. It is not intended as a complete reference. For that it's always best to refer to the source code.
The synchronous API has a pretty small surface area, with only a few commands to use. In general they are very similar to the way printf works, except they construct `RESP` commands.
### Connecting
There are several convenience functions to connect in various ways (e.g. host and port, Unix socket, etc). See [include/kv/kv.h](../include/kv/kv.h) for more details.
```c
kvContext *kvConnect(const char *host, int port);
kvContext *kvConnectUnix(const char *path);
// There is also a convenience struct to specify various options.
kvContext *kvConnectWithOptions(kvOptions *opt);
```
When connecting to a server, libkv will return `NULL` in the event that we can't allocate the context, and set the `err` member if we can connect but there are issues. So when connecting it's simple to handle error states.
There are a variety of options you can specify when connecting to the server, which are delivered via the `kvOptions` helper struct. This includes information to connect to the server as well as other flags.
```c
kvOptions opt = {0};
// You can set primary connection info
if (tcp) {
KV_OPTIONS_SET_TCP(&opt, "localhost", 6379);
} else {
KV_OPTIONS_SET_UNIX(&opt, "/tmp/kv.sock");
}
// You may attach any arbitrary data to the context
KV_OPTIONS_SET_PRIVDATA(&opt, my_data);
```
There are also several flags you can specify when using the `kvOptions` helper struct.
| Flag | Description |
| --- | --- |
| `KV_OPT_NONBLOCK` | Tells libkv to make a non-blocking connection. |
| `KV_OPT_REUSEADDR` | Tells libkv to set the [SO_REUSEADDR](https://man7.org/linux/man-pages/man7/socket.7.html) socket option |
| `KV_OPT_PREFER_IPV4`<br>`KV_OPT_PREFER_IPV6`<br>`KV_OPT_PREFER_IP_UNSPEC` | Informs libkv to either prefer IPv4 or IPv6 when invoking [getaddrinfo](https://man7.org/linux/man-pages/man3/gai_strerror.3.html). `KV_OPT_PREFER_IP_UNSPEC` will cause libkv to specify `AF_UNSPEC` in the getaddrinfo call, which means both IPv4 and IPv6 addresses will be searched simultaneously.<br>Libkv prefers IPv4 by default. |
| `KV_OPT_NO_PUSH_AUTOFREE` | Tells libkv to not install the default RESP3 PUSH handler (which just intercepts and frees the replies). This is useful in situations where you want to process these messages in-band. |
| `KV_OPT_NOAUTOFREEREPLIES` | **ASYNC**: tells libkv not to automatically invoke `freeReplyObject` after executing the reply callback. |
| `KV_OPT_NOAUTOFREE` | **ASYNC**: Tells libkv not to automatically free the `kvAsyncContext` on connection/communication failure, but only if the user makes an explicit call to `kvAsyncDisconnect` or `kvAsyncFree` |
| `KV_OPT_MPTCP` | Tells libkv to use multipath TCP (MPTCP). Note that only when both the server and client are using MPTCP do they establish an MPTCP connection between them; otherwise, they use a regular TCP connection instead. |
### Executing commands
The primary command interface is a `printf`-like function that takes a format string along with a variable number of arguments. This will construct a `RESP` command and deliver it to the server.
Commands may also be constructed by sending an array of arguments along with an optional array of their lengths. If lengths are not provided, libkv will execute `strlen` on each argument.
// Handle error conditions similarly to `kvCommand`
```
### Using replies
The `kvCommand` and `kvCommandArgv` functions return a `kvReply` on success and `NULL` in the event of a severe error (e.g. a communication failure with the server, out of memory condition, etc).
If the reply is `NULL` you can inspect the nature of the error by querying `kvContext->err` for the error code and `kvContext->errstr` for a human readable error string.
When a `kvReply` is returned, you should test the `kvReply->type` field to determine which kind of reply was received from the server. If for example there was an error in the command, this reply can be `KV_REPLY_ERROR` and the specific error string will be in the `reply->str` member.
### Reply types
- `KV_REPLY_ERROR` - An error reply. The error string is in `reply->str`.
- `KV_REPLY_STATUS` - A status reply which will be in `reply->str`.
- `KV_REPLY_INTEGER` - An integer reply, which will be in `reply->integer`.
- `KV_REPLY_DOUBLE` - A double reply which will be in `reply->dval` as well as `reply->str`.
- `KV_REPLY_NIL` - a nil reply.
- `KV_REPLY_BOOL` - A boolean reply which will be in `reply->integer`.
- `KV_REPLY_BIGNUM` - As of yet unused, but the string would be in `reply->str`.
- `KV_REPLY_STRING` - A string reply which will be in `reply->str`.
- `KV_REPLY_VERB` - A verbatim string reply which will be in `reply->str` and who's type will be in `reply->vtype`.
- `KV_REPLY_ARRAY` - An array reply where each element is in `reply->element` with the number of elements in `reply->elements`.
- `KV_REPLY_MAP` - A map reply, which structurally looks just like `KV_REPLY_ARRAY` only is meant to represent keys and values. As with an array reply you can access the elements with `reply->element` and `reply->elements`.
- `KV_REPLY_SET` - Another array-like reply representing a set (e.g. a reply from `SMEMBERS`). Access via `reply->element` and `reply->elements`.
- `KV_REPLY_ATTR` - An attribute reply. As of yet unused by kv-server.
- `KV_REPLY_PUSH` - An out of band push reply. This is also array-like in nature.
### Disconnecting/cleanup
When libkv returns non-null `kvReply` struts you are responsible for freeing them with `freeReplyObject`. In order to disconnect and free the context simply call `kvFree`.
`kvCommand` and `kvCommandArgv` each make a round-trip to the server, by sending the command and then waiting for a reply. Alternatively commands may be pipelined with the `kvAppendCommand` and `kvAppendCommandArgv` functions.
When you use `kvAppendCommand` the command is simply appended to the output buffer of `kvContext` but not delivered to the server, until you attempt to read the first response, at which point the entire buffer will be delivered.
```c
// No data will be delivered to the server while these commands are being appended.
for (size_t i = 0; i < 100000; i++) {
if (kvAppendCommand(c, "INCRBY key:%zu %zu", i, i) != KV_OK) {
// The entire output buffer will be delivered on the first call to `kvGetReply`.
for (size_t i = 0; i < 100000; i++) {
if (kvGetReply(c, (void**)&reply) != KV_OK) {
fprintf(stderr, "Error reading reply %zu: %s\n", i, c->errstr);
exit(1);
} else if (reply->type != KV_REPLY_INTEGER) {
fprintf(stderr, "Error: Non-integer reply to INCRBY?\n");
exit(1);
}
printf("INCRBY key:%zu => %lld\n", i, reply->integer);
freeReplyObject(reply);
}
```
`kvGetReply` can also be used in other contexts than pipeline, for example when you want to continuously block for commands for example in a subscribe context.
As previously mentioned, when there is a communication error libkv will return `NULL` and set the `err` and `errstr` members with the nature of the problem. The specific error types are as follows.
- `KV_ERR_IO` - A problem with the connection.
- `KV_ERR_EOF` - The server closed the connection.
- `KV_ERR_PROTOCOL` - There was an error parsing the reply.
- `KV_ERR_TIMEOUT` - A connect, read, or write timeout.
- `KV_ERR_OOM` - Out of memory.
- `KV_ERR_OTHER` - Some other error (check `c->errstr` for details).
### Thread safety
Libkv context structs are **not** thread safe. You should not attempt to share them between threads, unless you really know what you're doing.
### Reader configuration
Libkv contexts have a few more mechanisms you can customize to your needs.
#### Maximum input buffer size
Libkv uses a buffer to hold incoming bytes, which is typically restored to the configurable max buffer size (`16KB`) when it is empty. To avoid continually reallocating this buffer you can set the value higher, or to zero which means "no limit".
```c
context->reader->maxbuf = 0;
```
#### Maximum array elements
By default, libkv will refuse to parse array-like replies if they have more than 2^32-1 or 4,294,967,295 elements. This value can be set to any arbitrary 64-bit value or zero which just means "no limit".
```c
context->reader->maxelements = 0;
```
#### RESP3 Push Replies
The `RESP` protocol introduced out-of-band "push" replies in the third version of the specification. These replies may come at any point in the data stream. By default, libkv will simply process these messages and discard them.
If your application needs to perform specific actions on PUSH messages you can install your own handler which will be called as they are received. It is also possible to set the push handler to NULL, in which case the messages will be delivered "in-band". This can be useful for example in a blocking subscribe loop.
**NOTE**: You may also specify a push handler in the `kvOptions` struct and set it on initialization .
Internally libkv uses a layer of indirection from the standard allocation functions, by keeping a global structure with function pointers to the allocators we are going to use. By default they are just set to `malloc`, `calloc`, `realloc`, etc.
These can be overridden like so
```c
kvAllocFuncs my_allocators = {
.mallocFn = my_malloc,
.callocFn = my_calloc,
.reallocFn = my_realloc,
.strdupFn = my_strdup,
.freeFn = my_free,
};
// libkv will return the previously set allocators.
kvAllocFuncs old = kvSetAllocators(&my_allocators);
```
They can also be reset to the glibc or musl defaults
```c
kvResetAllocators();
```
**NOTE**: The `vk_calloc` function handles the case where `nmemb` * `size` would overflow a `size_t` and returns `NULL` in that case.
## Asynchronous API
Libkv also has an asynchronous API which supports a great many different event libraries. See the [examples](../examples) directory for specific information about each individual event library.
### Connecting
Libkv provides an `kvAsyncContext` to manage asynchronous connections which works similarly to the synchronous context.
The asynchronous context _should_ hold a connect callback function that is called when the connection attempt completes, either successfully or with an error.
It _can_ also hold a disconnect callback function that is called when the connection is disconnected (either because of an error or per user request).
The context object is always freed after the disconnect callback fired.
### Executing commands
Executing commands in an asynchronous context work similarly to the synchronous context, except that you can pass a callback that will be invoked when the reply is received.
For a graceful disconnect use `kvAsyncDisconnect` which will block new commands from being issued.
The connection is only terminated when all pending commands have been sent, their respective replies have been read, and their respective callbacks have been executed.
After this, the disconnection callback is called with the status, and the context object is freed.
To terminate the connection forcefully use `kvAsyncFree` which also will block new commands from being issued.
There will be no more data sent on the socket and all pending callbacks will be called with a `NULL` reply.
After this, the disconnection callback is called with the `KV_OK` status, and the context object is freed.
## TLS support
TLS support is not enabled by default and requires an explicit build flag as described in [`README.md`](../README.md#building).
Libkv implements TLS on top of its `kvContext` and `kvAsyncContext`, so you will need to establish a connection first and then initiate a TLS handshake.
See the [examples](../examples) directory for how to create the TLS context and initiate the handshake.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Hanzo Dev
