Fetch DOCKERHUB_USERNAME, DOCKERHUB_TOKEN, and DIGITALOCEAN_ACCESS_TOKEN
from kms.hanzo.ai at runtime via Universal Auth. Re-enable deploy-hanzo
job targeting statefulset/redis-master container kv on hanzo-k8s cluster.
Split Docker Hub push as continue-on-error with GHCR as primary registry.
Production redis uses bitnami/redis via Helm chart. The hanzoai/kv
image has a different entrypoint and is not a drop-in replacement.
Build+push to GHCR still works. Deploy will be re-enabled once K8s
manifests are migrated to use hanzoai/kv natively.
Replace direct GitHub secrets (DOCKERHUB_USERNAME, DOCKERHUB_TOKEN,
DIGITALOCEAN_ACCESS_TOKEN) with KMS Universal Auth fetch at runtime.
Only KMS_CLIENT_ID and KMS_CLIENT_SECRET remain as GitHub secrets.
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>
## Add `ALLOW_BUSY` flag to `SELECT` command
### Motivation
When a client decides to switch databases (e.g., from DB 0 to DB 1), it
must issue `SELECT` command.
However it is possible that when the command was sent, the server was
running a very long script, causing it to respond with `-BUSY` error.
This leads to several issues:
1. pipelining clients can face inconsistency when the select is followed
by a pipeline of commands to write to this database. in case only the
select gets the `-BUSY` error and some of the rest of the commands are
process AFTER the long script executes, these commands will be processed
on the wrong database context.
2. Although clients can maintain logic to handle the prior issue, it
complicates client logic and places a "head of line" delay, since the
client will need to wait for the select return in order to continue
pipeline commands, potentially forcing it to allocate a different
connection per database. Also this is probably NOT being handled by any
known client library ATM.
3. With the introduction of multi-database support in cluster mode,
clients managing connections across multiple shards need to maintain a
consistent database selection across all their connections. When a
client decides to switch databases (e.g., from DB 0 to DB 1), it must
issue `SELECT` to every shard in the cluster.
Currently, if any shard is busy (e.g., executing a long-running Lua
script or module command), the `SELECT` call to that shard will be
rejected with a `-BUSY` error. This creates a **split-brain scenario**
where some connections have switched to the new database while others
remain on the old one.
### Why `ALLOW_BUSY` is safe for `SELECT`
`SELECT` is a **connection-local, metadata-only operation**. It simply
changes which database index the current client connection points to.
It:
- **Does not read or write any keys** - there is no data conflict with a
running script.
- **Is already flagged as `fast`** - it runs in O(1) and will not
contribute to or extend any busy condition.
- **Is already flagged as `loading`** - the server already recognizes
that `SELECT` is safe to execute during sensitive server states (dataset
loading from disk), establishing a precedent that this command is
harmless to run when other commands are being rejected.
- **Is already flagged as `stale`** - similarly, `SELECT` is allowed on
stale replicas, further confirming it is treated as a safe, non-data
operation.
### Changes
- Added `allow_busy` to the `command_flags` for `SELECT` in
`src/commands/select.json`.
Signed-off-by: Gabi Ganam <ggabi@amazon.com>
Co-authored-by: Gabi Ganam <ggabi@amazon.com>
Large-memory tests were failing with "I/O error reading reply" after the
first test `EVAL - JSON string encoding a string larger than 2GB`. Three
tests that shared a single server instance led to cascading failures
when the first test's 3GB allocation left the server in a
memory-stressed state.
This PR isolates each test by starting its own server. Additionally,
I've reduced the JSON encoding test size from 3GB to 2.25GB. This
included changing the allocated 1GB string (concatenated 3x) to a 750MB
string to avoid memory exhaustion while still testing strings greater
than 2GB like we intended.
---------
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
in #2807 we added safe-iterator invalidation when a hashtable is
deleted.
When we create a safeIterator we initialize it to it->table=0 and
it-index=-1.
HOWEVER say someone creates a safe iterator never progress it and later
on just delete the iterator using `hashtableReleaseIterator`.
the iterator will NOT be untracked since, `hashtableCleanupIterator`
will skip the untrack:
```c
if (!(iter->index == -1 && iter->table == 0)) {
if (isSafe(iter)) {
hashtableResumeRehashing(iter->hashtable);
untrackSafeIterator(iter);
} else {
assert(iter->fingerprint == hashtableFingerprint(iter->hashtable));
}
}
```
and then when the hashtable is freed the iterator will be referenced
leading to a use-after-free.
For example:
```c
it = hashtableCreateIterator(ht, HASHTABLE_ITER_SAFE);
hashtableReleaseIterator(it);
hashtableRelease(ht);
```
This PR fix it by ALWAYS untrack safe iterators which have a valid
hashtable when they are being cleaned up.
Signed-off-by: Ran Shidlansik <ranshid@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>
This PR enables `USE_LIBBACKTRACE=yes` across all CI builds and builds
upon the changes introduced in #3034. Alpine-based jobs previously
attempted to install `libbacktrace-dev`, which does not exist in
Alpine’s apk repositories.
This caused these two errors in the daily tests below:
-
https://github.com/valkey-io/valkey/actions/runs/22045858351/job/63694456995
-
https://github.com/valkey-io/valkey/actions/runs/22045858351/job/63694457018
To resolve this, Alpine jobs now build GNU libbacktrace from source
inside the container before compiling Valkey. This aligns Alpine
behavior with other environments (Ubuntu jobs) and now avoids utilizing
non-existent Alpine packages.
An alternative approach we can consider is to disable `USE_LIBBACKTRACE`
for Alpine-based tests.
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
We made some changes to the workflow where the label was getting removed
every time we ran it. This changes handles it, removes
`pull_request_target` and doesn't re-trigger on adding another label.
I tried it here: https://github.com/sarthakaggarwal97/valkey/pull/65
The code is already merged in my unstable.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
At one point in development there was assumption that intset of database
IDs would have IDs only in `[0, server.dbnum-1]` range, but
[here](https://github.com/valkey-io/valkey/pull/2309#issuecomment-3684557935)
we decided to change upper bound to `INT32_MAX` for ACL compatibility
reasons between nodes.
Because of that change, assumption is not true anymore and we should
explicitly check each database list to contain access to full `[0,
server.dbnum-1]` range.
Also added test for that.
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
It's worth mentioning that enabling `maxmemory-clients` will affect the
performance so people can perform its own benchmark before enabling it.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Replace the implementation of byte-reverse functions.
The new implementation compiles to a single bswap instruction. The old
implementation didn't.
The new implementation is small and all in the .h file. The .c file is
deleted.
On little-endian systems, this affects streams, module timers and
cluster code and a few more things, where `htonu64()` is used for
storing stream-id or timestamps in a rax in big-endian order.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Only call gettimeofday() periodically and add monotonic delta. This
avoids a syscall if we have a fast monotonic clock, which we have now by
default on most platforms.
`ustime()` is called once per command execution. In a busy server
running 1M RPS, `ustime()` is called every microsecond. By calling
`gettimeofday()` only once every millisecond, we save 99.9% of these
syscalls.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
With this we get more detailed backtrace information, including
information about static functions. Off by default - to enable you must
enable at compile time:
make USE_LIBBACKTRACE=yes
Signed-off-by: Rain Valentine <rsg000@gmail.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>
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>
Currently if we just run make test, it fails because Lua is not built
automatically. Thanks to @zuiderkwast for pointing the fix.
Make Test Passed
```
Test Summary: 5895 passed, 0 failed
\o/ All tests passed without errors!
Cleanup: may take some time... OK
```
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
I think a lot of new contributors are not aware how they can run the
extended list of tests in their forked repo branch with other test
arguments.
This change could be helpful to some people.
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
We have been seeing github actions runners being OOM when large memory
tests are run with ASan. The operation eventually is being canceled
during the test.
This change moves the large-memory tests with ASan and UBSan to separate
jobs, so we get a dedicated runner with its own timeout. We can tweak
the number of simultaneous test clients for these tests without
affecting the other test jobs.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
This fixes a regression where PFADD returned 1 instead of INVALIDOBJ for
corrupted sparse HLL payloads. In small strings refactor (#2516) we
incorrectly changed the datatype from int to bool. That function still
uses -1 as its error signal.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
There should be a way for people to monitor the amount of output buffer
data accumulating at the source node during the migration process.
This PR also do a change in slotExportTryDoPause, previously, we used
`getClientOutputBufferMemoryUsage` to determine if the offset was sufficient
to trigger the pause, it also adding the listNode overhead. Now we use
client->bytes.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Adding more client flags in the information populated by
`ValkeyModule_GetClientInfoById()`:
VALKEYMODULE_CLIENTINFO_FLAG_PRIMARY
VALKEYMODULE_CLIENTINFO_FLAG_REPLICA
VALKEYMODULE_CLIENTINFO_FLAG_MONITOR
VALKEYMODULE_CLIENTINFO_FLAG_MODULE
VALKEYMODULE_CLIENTINFO_FLAG_AUTHENTICATED
VALKEYMODULE_CLIENTINFO_FLAG_EVER_AUTHENTICATED
VALKEYMODULE_CLIENTINFO_FLAG_FAKE
Closes#2590
---------
Signed-off-by: martinrvisser <mvisser@hotmail.com>
Signed-off-by: martinrvisser <martinrvisser@users.noreply.github.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
The problem: when running `./runtest --large-memory --tags large-memory`
the `--tags large-memory` filter would look for tests with
`large-memory` tag, it found the nested tag in the test context but
`start_server` didn't have `large-memory`, so the filtering logic had
issues. And that's why we did not notice how we introduced this failure:
```
[err]: CVE-2025-32023: Sparse HLL XZERO overflow triggers crash in tests/unit/hyperloglog.tcl
Expected an error matching '*INVALIDOBJ*' but got '1' (context: type eval line 24 cmd {assert_error {*INVALIDOBJ*} {r pfadd hll_overflow foo}} proc ::test)
```
Tags `large-memory`, `needs:other-server`, `compatible-redis`, and
`network` now restricted to top-level `start_server`. Prevents `--tags`
filtering issues when these tags appear in nested contexts. Added
validation that errors on nested usage. Refactored tests that fail new
validation. Also fixed all `lsearch` calls to use `-exact` flag to
preventing bugs like "tls" matching "tls:skip".
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
**Fixes #3127**
## Description
Adds a `valgrind:skip` tag to automatically skip tests when running with
`--valgrind`. This allows skipping before the server starts, rather than
checking `$::valgrind` inside tests.
## Changes
- Added tag check in `server.tcl`
- Replaced `!$::valgrind` checks with the new tag in affected tests
- Updated README documentation
- Refactored tests in `hash.tcl` by adding `valgrind:skip` tag inline
- Added `valgrind:skip` tag to a test in `incr.tcl`
## Testing
Verified tests are correctly skipped with `--valgrind` and run normally
without it.
Signed-off-by: Mangat Toor <immangat@amazon.com>
Co-authored-by: Mangat Toor <immangat@amazon.com>
This reverts #3069 commit e95429a728.
Clearly, it doesn't handle TLS-related aspects, which caused it
to fail some TLS tests. Furthermore, this check is also incorrect
under the "announce client port" setting.
There isn't much value in doing this and there is no need to extend
the helper function.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Fixing TLS module test failure, closes#3165
Stabilizes TLS unit tests around enable/disable toggling in module mode.
Code change summary:
`tls.c` has two `tlsResetCertInfo()` implementations under different #if
branches.
- OpenSSL (built‑in TLS) branch calls tlsRefreshAllCertInfo() because it
can read certs from SSL contexts.
- Non‑OpenSSL stub (module build) can’t access certs, so it only clears
on disable and relies on the module to populate cert info.
---------
Signed-off-by: Yiwen Zhang <yiwen_zhang@apple.com>
Co-authored-by: Yiwen Zhang <yiwen_zhang@apple.com>
Fixes a crash that occurs when `genClusterInfoString()` is called before
the cluster node has fully initialized. The myself pointer may be `NULL`
in these edge cases, causing a segfault when calling `nodeEpoch(myself)`.
Related to #3170
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthakaggarwal97@gmail.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
Enhanced the popular helper `wait_for_cluster_propagation` to support
the scenario where we intentionally stop a server and wait for the other
nodes to converge without it.
Signed-off-by: Zhijun <dszhijun@gmail.com>
It might be a good idea to improve the code quality to carefully check
for exceptions when converting strings to numbers by calling the
strtoull function.
EINVAL checks if a number was converted incorrectly, and checks if the
entire buf is a number, but requires ERANGE in addition.
It's safe to check for overflows, which can occur when a user enters an
absurdly large value like 9999999999999999999999999gb.
Suggested fix: Check both EINVAL and ERANGE
Signed-off-by: namuk2004 <namuk2004@naver.com>
I was working on ASAN large memory tests when I countered this issue.
The issue was that the hardcoded `999` key could land in an early
bucket. Then shrink rehash could finish early, and later inserts could
trigger a new expansion rehash, resetting rehash_idx low. The test now
picks the survivor key dynamically as the key mapped to the highest
bucket index.
```
[test_hashtable.c] Memory leak detected of 336 bytes
=================================================================
==3901==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 80 byte(s) in 1 object(s) allocated from:
#0 0x7fb0556fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x563bfdf4c47d in ztrymalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:156
#2 0x563bfdf4c47d in valkey_malloc /home/runner/work/valkey/valkey/src/zmalloc.c:185
#3 0x563bfdd42eaf in hashtableCreate /home/runner/work/valkey/valkey/src/hashtable.c:1217
#4 0x563bfdaa1cbf in test_empty_buckets_rehashing unit/test_hashtable.c:232
#5 0x563bfdae772b in runTestSuite unit/test_main.c:36
#6 0x563bfda86b20 in main unit/test_main.c:108
#7 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#8 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#9 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2)
Indirect leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214
#2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257
#3 0x563bfdd40967 in resize /home/runner/work/valkey/valkey/src/hashtable.c:741
#4 0x563bfdd45eb1 in hashtableExpandIfNeeded /home/runner/work/valkey/valkey/src/hashtable.c:1446
#5 0x563bfdd45eb1 in hashtableExpandIfNeeded /home/runner/work/valkey/valkey/src/hashtable.c:1433
#6 0x563bfdd45eb1 in insert /home/runner/work/valkey/valkey/src/hashtable.c:1041
#7 0x563bfdd45eb1 in hashtableAddOrFind /home/runner/work/valkey/valkey/src/hashtable.c:1554
#8 0x563bfdd45eb1 in hashtableAdd /home/runner/work/valkey/valkey/src/hashtable.c:1539
#9 0x563bfdaa1e3b in test_empty_buckets_rehashing unit/test_hashtable.c:254
#10 0x563bfdae772b in runTestSuite unit/test_main.c:36
#11 0x563bfda86b20 in main unit/test_main.c:108
#12 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#13 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#14 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2)
Indirect leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214
#2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257
#3 0x563bfdd3f553 in bucketConvertToChained /home/runner/work/valkey/valkey/src/hashtable.c:908
#4 0x563bfdd3f553 in findBucketForInsert /home/runner/work/valkey/valkey/src/hashtable.c:1021
#5 0x563bfdd45d9e in insert /home/runner/work/valkey/valkey/src/hashtable.c:1045
#6 0x563bfdd45d9e in hashtableAddOrFind /home/runner/work/valkey/valkey/src/hashtable.c:1554
#7 0x563bfdd45d9e in hashtableAdd /home/runner/work/valkey/valkey/src/hashtable.c:1539
#8 0x563bfdaa1e3b in test_empty_buckets_rehashing unit/test_hashtable.c:254
#9 0x563bfdae772b in runTestSuite unit/test_main.c:36
#10 0x563bfda86b20 in main unit/test_main.c:108
#11 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#12 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#13 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2)
Indirect leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x7fb0556fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x563bfdf4c922 in ztrycalloc_usable_internal /home/runner/work/valkey/valkey/src/zmalloc.c:214
#2 0x563bfdf4c922 in valkey_calloc /home/runner/work/valkey/valkey/src/zmalloc.c:257
#3 0x563bfdd40967 in resize /home/runner/work/valkey/valkey/src/hashtable.c:741
#4 0x563bfdaa1df8 in test_empty_buckets_rehashing unit/test_hashtable.c:248
#5 0x563bfdae772b in runTestSuite unit/test_main.c:36
#6 0x563bfda86b20 in main unit/test_main.c:108
#7 0x7fb05522a1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#8 0x7fb05522a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#9 0x563bfda8a5c4 in _start (/home/runner/work/valkey/valkey/src/valkey-unit-tests+0x17c5c4) (BuildId: 44cfc183e6e82e499bcc9f6adc094d7f774ee9d2)
SUMMARY: AddressSanitizer: 336 byte(s) leaked in 4 allocation(s).
```
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Address #2683
For the test, I found two problems.
### 1. Test Assumes Winner Has Rank #0
In my test run, in the failing case:
- Replica -3 (rank 0) won epoch 10 first
- Replica -6 (rank 1) won epoch 11 second
- Replica -3 saw higher epoch and stepped down
- Final result: rank 1 became master
Rank #0 doesn't guarantee being the final master.
### 2. Pattern Matching Bug
The pattern `*Start of election*rank #0*` incorrectly matches "primary
rank #0":
Log: `Start of election delayed for 350 milliseconds (rank #1, primary
rank #0, offset 2172)`
This line has rank `#1`, but the pattern matches because of "primary
rank `#0`" at the end.
## Solution
- Fixed the format problem by checking if `(rank #0` or `(rank #1` so
that we won't accidentally match the primary rank
- Only check to make sure replica 3 and replica 6 have different ranks
without assuming the replica with rank 0 will become the master.
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
A hashtable bucket has usually multiple entries, we can just do
one step = one bucket chain, regardless of empty or not.
But in shrinking case, there is a case that the ht0 can be very
empty (but the table size is huge), when doing one step rehash,
if there are a lot of empty buckets, we can do more step in this
case to make the rehash faster.
We need to make sure the rehash is done before we need to resize
again. There is another case that can cause issue, like when in
the shrinking case, the new ht1 is small, but the hash collisions
become more often when we adding elements, we need to rehash the
empty buckets in ht0 as quickly as possible (using more CPU).
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
When ZREM / SREM / HDEL delete a large number of elements, we can apply the same
optimization as ZREMRANGEBY*, which is to temporarily pause the autoshrink of
the corresponding hashtable to avoid triggering resize/rehash repeatedly during
the deletion process.
This theoretically leads to some performance improvements. Furthermore, by only
triggering a resize once at the end (i.e., only when `hashtableResumeAutoShrink`
is called), we can get the most right resize, preventing further resizes during
the resize/rehash process.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Closes#3146
The following two test cases are flaky
- `evict clients only until below limit` - uses exact math expecting
exactly half the clients evicted
- `evict clients in right order (large to small)` - uses exact math
expecting specific clients evicted in order
It's fine to skip them in TLS because the core logic being tested
(client eviction) doesn't change based on TLS vs non-TLS.
The `decrease maxmemory-clients causes client eviction` test case could
potentially be flaky as well (has not shown flakiness on CI yet), but
since it has more tolerant assertion: `connected_clients > 0 &&
connected_clients < $client_count`, I think it's okay not to bother
skipping it.
Other test cases are not flaky because they use large thresholds or
check binary outcomes (yes/no eviction), not exact counts.
Signed-off-by: Zhijun <dszhijun@gmail.com>
It is possible to have an empty table with only empty buckets
remaining, and we are still rehashing those empty buckets. If
table 0 is empty (both used and child_buckets are empty), we can
simply finish the rehashing process, since we no longer have any
elements (buckets) to rehash.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Follow-up to my previous CMake PR
https://github.com/valkey-io/valkey/pull/2816.
**Changes:**
1. **`.github/workflows/ci.yml`** - Removed symlinks, use
`./build-release/runtest` instead of `./runtest`
2. **`tests/support/set_executable_path.tcl`** - Added
`::VALKEY_TLS_MODULE` variable
3. **Fixed hardcoded paths in 5 test files:**
- `tests/unit/tls.tcl` - server and TLS module paths
- `tests/unit/fuzzer.tcl` - benchmark path
- `tests/unit/cluster/cli.tcl` - CLI path
- `tests/support/server.tcl` - TLS module path
- `tests/instances.tcl` - TLS module path
**Result:** All tests passed. The only failure was an unrelated flaky
test (`client-eviction.tcl`) that's been failing since TLS was added to
the cmake job - tracked in issue #3146.
---------
Signed-off-by: Zhijun <dszhijun@gmail.com>
It is useful to show hashtable stats even if the table is empty since
in addition to the elements themselves, stats such as the buckets are
still useful. Also in this PR, we added rehash index info in table 0
so that we can see this info when doing the debug.
Signed-off-by: Binbin <binloveplay1314@qq.com>
We should check expiration time value before we lookup the key,
otherwise we will return nil in this wrong case:
```
> getex key ex abcd
(nil)
> set key value
OK
> getex key ex abcd
(error) ERR value is not an integer or out of range
> hset foo f v
(integer) 1
> getex foo ex abcdef
(error) WRONGTYPE Operation against a key holding the wrong kind of value
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
Reverts "Fix the memory leak in the VM_GetCommandKeysWithFlags function
(#3088)"
This reverts commit d1b4e6bf8b.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In dual channel replication, when the replica abort the replication,
if the local buffer is huge, free it in async way to avoid the latency.
Dual channel replication was introduced in #60.
Signed-off-by: Binbin <binloveplay1314@qq.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>
This PR fixes various issues with Lua module built on macOS using CMake
/ Apple Clang.
## Summary of changes
The Lua module build configuration has been updated to support macOS in
addition to Linux. Two key changes were made to resolve build issues on
Apple platforms.
First, the valkeylua shared library now explicitly includes `sha1.c` and
`rand.c` source files and links against the fpconv library, ensuring all
required symbols are available at link time.
Second, the --disable-new-dtags linker flag is now conditionally applied
only on Unix systems excluding macOS, since this flag is not supported
by the Apple linker.
In addition, enable position independent code compilation for the fpconv
static library by setting the `POSITION_INDEPENDENT_CODE` property. This
allows the library to be linked into shared libraries and
position-independent executables.
Last, the Lua library filename definition was previously hardcoded to
use the `.so` extension for all platforms. This change conditionally
sets the library name based on the target platform: `.so` for Linux/Unix
systems and `.dylib` for macOS (and other non-Linux Unix systems). This
ensures that the correct dynamic library extension is used when loading
the Lua module at runtime.
** Generated by CodeLite. **
Signed-off-by: Eran Ifrah <eifrah@amazon.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>
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>
If valgrind is used then test for used_active_time_io_thread fails
because valgrind is slow, so skip it
```
[err]: Force the use of IO threads and assert active IO thread usage in tests/unit/io-threads.tcl
Expected (7.941596 - 5.876588) < (1000/1000) (context: type eval line 53 cmd {assert {($used_active_time - $initial_active_times($i)) < ($sleep_time_ms/1000)}} proc ::test)
```
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Before 4757aefe0e, even if setlocale fails, we
won't do anything.
```
setlocale(LC_COLLATE,"");
```
And now we will exit the process if the setlocale fails, this resulted in process
startup failures on some machines. (some kind of breaking change?)
```
if (setlocale(LC_COLLATE,server.locale_collate) == NULL) {
serverLog(LL_WARNING, "Failed to configure LOCALE for invalid locale name.");
exit(1);
}
```
In this commit, if we fail to set the locale_collate through environment variables,
we maintain backward compatibility and do not exit.
In a case where we did exit before, we don't exit now, seems safe. It's not a breaking
change and the behavior in this case was undocumented.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Embedding the skiplist header reduces memory jumps, thus optimizing
sorted set query speed.
```
// Before
typedef struct zskiplist {
struct zskiplistNode *header, *tail;
unsigned long length;
int level; /* Height of the skiplist. */
} zskiplist;
// After
typedef struct zskiplist {
unsigned long length;
struct zskiplistNode *tail;
/* Since the span at level 0 is always 1 or 0 ,
* this field is instead used for storing the height of the skiplist. */
struct zskiplistLevel {
struct zskiplistNode *forward;
unsigned long span;
} level[ZSKIPLIST_MAXLEVEL];
} zskiplist;
```
---------
Signed-off-by: chzhoo <czawyx@163.com>
Addresses: https://github.com/valkey-io/valkey/issues/2908
This decouples the low-level LRU/LFU code from the high-level `server.h`
file.
* LRU/LFU specific config values were removed from `server.h` and
relocated to `lrulfu.h`
* A new LRULFU API was created to update the time (and policy) rather
than accessing globals from `server.`
This doesn't address the root of the original test failure in that
`server.h` may compile with different offsets (due to `off_t`) based on
the include order. See:
https://github.com/valkey-io/valkey/issues/2908#issuecomment-3647452825
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
This PR adds the certificates validation at TLS load, rejects invalid
(expired/not-yet-valid) certificates:
Apply to all TLS config paths:
- Server certificates `tls-cert-file`
- Server-side client certificates `tls-client-cert-file`
- CA certificate file `tls-ca-cert-file`
- CA certificate directory `tls-ca-cert-dir` (now eagerly loaded to be
consistent with file-based CAs)
Apply to both scenarios:
- Server startup (initial TLS load)
- Runtime reload vis `CONFIG SET`
### Implementation
- Added `isCertValid` function to check if an X509 certificate is within
its validity period (not expired, not future-dated)
- Added `areAllCaCertsValid` function to iterate through all loaded CA
certificates and validate them
- Added `loadCaCertDir` function to eagerly load all certificates from a
directory into the X509_STORE
- Modified `createSSLContext` to validate:
- Server/client certificates immediately after loading
- All CA certificates after loading from file/directory
### Test results
#### 1. Server startup (initial TLS load)
```
tls-cert-file ./tests/tls/server-expired.crt
41522:M 31 Dec 2025 16:13:18.851 # Server TLS certificate is invalid. Aborting TLS configuration.
41522:M 31 Dec 2025 16:13:18.851 # Failed to configure TLS. Check logs for more info.
tls-client-cert-file ./tests/tls/client-expired.crt
41557:M 31 Dec 2025 16:14:43.296 # Client TLS certificate is invalid. Aborting TLS configuration.
41557:M 31 Dec 2025 16:14:43.296 # Failed to configure TLS. Check logs for more info.
tls-ca-cert-file ./tests/tls/ca-expired.crt
tls-ca-cert-dir ./tests/tls/ca-expired
41567:M 31 Dec 2025 16:15:15.635 # One or more loaded CA certificates are invalid. Aborting TLS configuration.
41567:M 31 Dec 2025 16:15:15.635 # Failed to configure TLS. Check logs for more info.
```
#### 2. Runtime reload via CONFIG SET
```
127.0.0.1:6379> config set tls-cert-file ./tests/tls/server-expired.crt
(error) ERR CONFIG SET failed (possibly related to argument 'tls-cert-file') - Unable to update TLS configuration. Check server logs.
62975:M 02 Jan 2026 20:10:43.588 # Server TLS certificate is invalid. Aborting TLS configuration.
62975:M 02 Jan 2026 20:10:43.588 # Failed applying new configuration. Possibly related to new tls-cert-file setting. Restoring previous settings.
127.0.0.1:6379> config set tls-client-cert-file ./tests/tls/client-expired.crt
(error) ERR CONFIG SET failed (possibly related to argument 'tls-client-cert-file') - Unable to update TLS configuration. Check server logs.
62975:M 02 Jan 2026 20:10:57.972 # Client TLS certificate is invalid. Aborting TLS configuration.
62975:M 02 Jan 2026 20:10:57.972 # Failed applying new configuration. Possibly related to new tls-client-cert-file setting. Restoring previous settings.
127.0.0.1:6379> config set tls-ca-cert-file ./tests/tls/ca-expired.crt
127.0.0.1:6379> config set tls-ca-cert-dir ./tests/tls/ca-expired
(error) ERR CONFIG SET failed (possibly related to argument 'tls-ca-cert-file') - Unable to update TLS configuration. Check server logs.
62975:M 02 Jan 2026 20:10:50.175 # One or more loaded CA certificates are invalid. Aborting TLS configuration.
62975:M 02 Jan 2026 20:10:50.175 # Failed applying new configuration. Possibly related to new tls-ca-cert-file setting. Restoring previous settings.
```
---------
Signed-off-by: Yang Zhao <zymy701@gmail.com>
On some Intel systems it was observed that TSC `ticksPerMicrosecond`
derived from `/proc/cpuinfo` was different from calibrated
`ticksPerMicrosecond` or kernel derived one, so it was decided to always
calibrate and look for `constant_tsc` flag.
Resolves#2597
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
I/O threads when active do busy polling for work. This puts the CPU time
near 100% per thread even when they aren't fully utilized. Tracking the
time an IO thread spends doing useful work vs waiting for work helps
understand the utilization of individual threads.
INFO fields:
used_active_time_io_thread_%d
... where %d is the IO thread ID number (between 1 and the number of I/O
threads).
To get a percentage over a period of time, compare the delta of this
metric against a delta of the server's uptime or the INFO field
`server_time_usec`.
---------
Signed-off-by: Deepak Nandihalli <deepak.nandihalli@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>
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
#(1). the replica will receive the propagated command, but will not know
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>
As the hashtable grows, rehash is triggered. Rehash consumes a
significant amount of CPU time, resulting in noticeable performance
degradation. Much of this CPU time is spent on random memory access
(when accessing hashtable entries). This CPU usage can be optimized
through concurrent memory access.
```
// The original rehash process:
For each bucket and its child buckets, all entries are processed as follows:
1) Access the entry, which involves random memory access.
2) Compute the hash value of the entry.
3) Based on the hash value, insert the entries into the new hashtable.
// The optimized rehash process:
The original steps are transformed into batch processing and phased execution, leveraging
the CPU's multi-issue capability to achieve concurrent memory access. The steps are as follows:
1) Access multiple entries serially within a for loop. Since each entry access is independent,
modern CPU architectures can perform concurrent memory access.
2) Compute the hash values for all entries.
3) Based on the hash values, insert the entries into the new hashtable.
```
---------
Signed-off-by: chzhoo <czawyx@163.com>
We are already double running the tests with CMake, and we are building
CMake with TLS, so just making it so we run the tests with TLS. This
seems like an simple update so that we are always running the TLS tests.
Signed-off-by: Madelyn Olson <madelyneolson@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>
Memory leak in the VM_GetCommandKeysWithFlags function when
MAX_KEYS_BUFFER is reached.
We will malloc the memory so we should always free the getKeysResult.
```
/* Resize if necessary */
if (numkeys > result->size) {
if (result->keys != result->keysbuf) {
/* We're not using a static buffer, just (re)alloc */
result->keys = zrealloc(result->keys, numkeys * sizeof(keyReference));
} else {
/* We are using a static buffer, copy its contents */
result->keys = zmalloc(numkeys * sizeof(keyReference));
if (result->numkeys) memcpy(result->keys, result->keysbuf, result->numkeys * sizeof(keyReference));
}
result->size = numkeys;
}
```
Closes#3087.
Signed-off-by: arshidkv12 <arshidkv12@gmail.com>
Signed-off-by: Arshid <arshidkv12@gmail.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
The test case added in #3042 fails on platforms without MPTCP support.
This change automatically skips the MPTCP tests on platforms without
MTPCP.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
Currently, the weekly runs do not progress if there is a failed workflow
as github CI treats `fail-fast` to be true by default. With this change,
we continue to test all the branches even after failure.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
### Overview
This PR adds support for automatic background TLS reloading, closes
https://github.com/valkey-io/valkey/issues/2649
TLS validity checks and fail-fast behavior on invalid certificates are
handled separately in #2999.
- New configuration
- `tls-auto-reload-interval <seconds>`
- `0` disabled (default, backward compatible)
- `>0` check interval in seconds
- TLS materials change detection in background
- SHA-256 fingerprint checking for certificate files
- `inode + mtime` checking for CA certificate directories and key files
- Skips reload if materials haven't changed
`tlsCheckMaterialsAndUpdateCache`
- TLS contexts reload
- CPU-intensive certificate parsing happens in dedicated BIO worker
thread `BIO_TLS_RELOAD`
- Main thread never blocks, atomically swaps SSL contexts
- Two-phase reload: background preparation `tlsConfigureAsync` + main
thread application `tlsApplyPendingReload`
**Note**: Original TLS load and reload still remain in main thread using
`tlsConfigureSync`, including:
- Initial TLS load (server startup)
- Runtime reload via CONFIG SET
---------
Signed-off-by: Yang Zhao <zymy701@gmail.com>
As we can see, it is entirely possible for the microseconds to have a
value of 0:
```
*** [err]: The microsecond part of the TIME command will not overflow in tests/unit/introspection-2.tcl
Expected '0' to be more than '0' (context: type eval line 4 cmd {assert_morethan $microseconds 0} proc ::test)
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
We should mention it in valkey.conf otherwise people may got it wrong.
Also, when the primary node receives a REPLCONF CAPA dual-channel but
repl-diskless-sync is not enabled (we will ignore the capa in this case),
we will also print a notice message so that people can be aware of it from
the logs. We believe if a user enables dual-channel-replication-enabled,
they intend to use dual channel replication.
Closes#3044.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Fix flaky tests in `slot-stats.tcl` and `introspection.tcl`.
Both tests share a similar flaky issue - when using a deferring client,
after `$rd <command>` returns, we **cannot** guarantee that the server
has already executed the command.
#### 1. "CLUSTER SLOT-STATS network-bytes-in, in-line buffer processing"
Add `$rd read` after `$rd flush` to wait for the server response,
ensuring the command is fully processed before checking slot stats.
#### 2. "MONITOR log blocked command only once"
Skip any leading `info` commands from MONITOR output before asserting
`blpop`. Since we cannot guarantee command execution order from the
deferring client's perspective, `info` commands triggered by
`wait_for_blocked_clients_count` may appear before the expected `blpop`.
Signed-off-by: yzc-yzc <96833212+yzc-yzc@users.noreply.github.com>
The metric tracks the number of seconds the main thread spends doing
active work as supposed to waiting for work. The active time is exposed
as the INFO field `used_active_time_main_thread` in the CPU section.
When I/O threads are used, the main thread uses a busy-loop when it's
waiting for work, so the reported CPU usage is near 100% even if the
thread has capacity to handle more work. This new metric attempts to
provide a useful metric of how loaded the main thread is by excluding
the time the thread is just waiting for work.
The busy loop consists of the cycle (beforeSleep, non-blocking epoll,
afterSleep). Only the duration of the loops that handle at least one
event loop event (network, file or timer event) or some work from I/O
threads (execution of commands received from clients by I/O threads) is
counted as active time.
Implements the main thread part of #2065.
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Add VM_ClusterKeySlotC that takes a data pointer and len as parameters.
Purpose: A module can avoid creating a ValkeyModuleString in a hot code
path.
Closes#2901
---------
Signed-off-by: Su Ko <rhtn1128@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-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>
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>
The patch fixes the error by adding `pull‑requests: write` to the
permissions block of `weekly.yml`. Github rejects if the reusable
workflow's (here daily.yml) permissions are not provided.
We recently changed the permissions in `daily.yml` where we gave write
permissions in https://github.com/valkey-io/valkey/pull/2907
With this change, we bring parity to the permissions since `weekly.yml`
uses calls `daily.yml` workflow call method.
Fixes: https://github.com/valkey-io/valkey/actions/runs/20890545320
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
This change restores maxmemory to 0 at the end of both OOM scripting
tests, preventing memory configuration from leaking into subsequent
tests.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Before #2858, we has this check:
```
static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) {
if (run_ctx->flags & SCRIPT_ALLOW_OOM) {
/* Allow running any command even if OOM reached */
return C_OK;
}
/* If we reached the memory limit configured via maxmemory, commands that
* could enlarge the memory usage are not allowed, but only if this is the
* first write in the context of this script, otherwise we can't stop
* in the middle. */
if (server.maxmemory && /* Maxmemory is actually enabled. */
!mustObeyClient(run_ctx->original_client) && /* Don't care about mem for replicas or AOF. */
!(run_ctx->flags & SCRIPT_WRITE_DIRTY) && /* Script had no side effects so far. */
server.pre_command_oom_state && /* Detected OOM when script start. */
(run_ctx->c->cmd->flags & CMD_DENYOOM)) {
*err = sdsdup(shared.oomerr->ptr);
return C_ERR;
}
return C_OK;
}
```
If we reached the memory limit configured via maxmemory, commands that
could enlarge the memory usage are not allowed, but only if this is the
first write in the context of this script, otherwise we can't stop
in the middle.
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
**The problem/use-case that the feature addresses**
I teach Valkey to people frequently. The general process is to start on
standalone Valkey (or 'try me' Valkey) to get started. However, in the
more advanced sections, I introduce the concepts of required to develop
for a Valkey Cluster. The key learning here is to understand slots (e.g.
multi-key operations, using hash tags, etc.). Standing up a cluster is a
hassle for someone just learning, and, while I usually demo `CLUSTER
KEYSLOT` for the users, they can't run that command on their single
instance since all of the `CLUSTER` commands are blocked. I think Valkey
students would learn a lot more quickly if they could experiment with
`CLUSTER KEYSLOT` themselves in standalone mode.
In this case, `CLUSTER KEYSLOT` doesn't require anything in the cluster
- it just does the math on the key to determine the slot number.
**Description of the feature**
This feature unblocks `CLUSTER KEYSLOT` on single instances.
**Alternatives you've considered**
- Having a cluster on try-me valkey (too many resources)
- Having single instance clusters (discussed before, but far more
complex than this trivial change)
**Additional information**
Their might be less repetitious logic, but the entire bits that
generates `CLUSTER KEYSLOT` result is just 3 lines, so repetitious feels
fine?
---------
Signed-off-by: Kyle J. Davis <kyledvs@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>
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>
By default, servers in a cluster don't have human-readable nodename
unless explicitly set by user. Most logging statements in the cluster
are in the form of `serverLog(LL_NOTICE, "Node %.40s (%s) did
something..", node->name, node->human_nodename, ...);` , and thus this
would always result in an empty `()` like the following:
```
* Node dd4f43d9e4a1a2875a514733c08d4870c21db682 () is now a replica of node 1b5d2a4865c04aaa1ccf06db66f22a043411ded7 () in shard d3fc557d12ab3c5a8850c7b0c9afb252b473cad1
* Clear FAIL state for node dd4f43d9e4a1a2875a514733c08d4870c21db682 (): replica is reachable again.
* NODE 524978a9924b5841c4c93f83581d202cfbce1443 () possibly failing.
* FAIL message received from 747e0f97a58cd5a7ac4c2130417fc47a807802ed () about 1b5d2a4865c04aaa1ccf06db66f22a043411ded7 ()
```
**This PR adds a helper function `humanNodename` specifically for
logging purpose that can print the node's ip:port when its nodename is
not explicitly set.** Now the logs can greatly help developers to
quickly recognize which node is doing stuff:
```
* NODE 03b3811da7b33451f6e4988295b0158f410d2c00 (127.0.0.1:7001) possibly failing.
* Marking node 03b3811da7b33451f6e4988295b0158f410d2c00 (127.0.0.1:7001) as failing (quorum reached).
* NODE b2f0fc93632d12c06039335b025365d484ff9049 (127.0.0.1:7002) possibly failing.
* Marking node b2f0fc93632d12c06039335b025365d484ff9049 (127.0.0.1:7002) as failing (quorum reached).
# Cluster state changed: fail
```
There’s no behavior change in cluster state, gossip, failover, etc, and
**the human nodename of each node is not changed**. This PR is for
logging only and is safe:
- For developers who never cared about the empty (), this is just a
small quality-of-life improvement: suddenly the logs tell them which
node is which, without extra node ID lookups.
- For developers who rely on custom names they prefer, nothing changes –
their explicit nodename takes precedence.
Signed-off-by: Zhijun <dszhijun@gmail.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>
Resolves: https://github.com/valkey-io/valkey/issues/2228
Visualization:
https://github.com/sarthakaggarwal97/valkey/actions/runs/19113712295
Currently, there are no tests running on the already released branches.
We often do backport for bug fixes and CVEs in these older versions, and
end up with multiple CI tests failures on these branches.
The PR adds support for running weekly tests on already released
versions `>= 7.2`. The workflow will execute the "daily" test workflow
for each of these branches on `Sunday 06:00 UTC`.
The idea is to continuously monitor our released versions through weekly
test runs (during the time when is lesser activity on github runners).
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@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>
Achieved 20-30% reduction in overhead.
This was accomplished by reusing the 8B robj->ptr memory to store
embedded data. For 16B key and 16B value, this reduces the per-item
memory overhead by ~20%. Overall performance was not measurably changed.
Raising the threshold for embedding strings produces ~30% reduction in
per-item overhead for affected value sizes.
Here's what I did:
1. Modify all `robj->ptr` access to use get/set methods. Most of this
was done with a script (code listed below), with a few manual touches.
Manual and programmatic changes are in separate commits to make review
easier.
2. Next I changed `objectGetVal()` to calculate the location instead of
using `robj->ptr`, and modified the embedding code to start writing
embedded data 8B earlier, overwriting the ptr location. I changed
`objectSetVal()` to assert that no value is embedded - all of the code
that assigned `o->ptr` would have been incorrect for embedded strings
anyway. (Except for one instance in module.c which I made a separate
method for.)
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Introduced `clusterMsgHeader` and `toClusterMsg()` `toClusterMsgLight()`
helpers to unify clusterMsg parsing and make casting explicit.
switched packet read/validation/processing paths to use the common
16-byte header first, then cast to clusterMsg/clusterMsgLight only after
determining message type.
this is a refactor to improve readability and avoid accidental
miscasts between full vs light headers.
issue : #2837
---------
Signed-off-by: Su Ko <rhtn1128@gmail.com>
Use the `lm_asprintf `method to solve the warm problem:
```
debug_lua.c: In function ‘ldbEval’:
debug_lua.c:470:13: warning: ignoring return value of ‘asprintf’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
470 | asprintf(&err_msg, "Error compiling code: %s", lua_tostring(lua, -1));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
---------
Signed-off-by: skyfirelee <739609084@qq.com>
In https://github.com/valkey-io/valkey/pull/2876 we added cluster info
stats to the info command. However these statistics where added to the
"cluster" section which is part of the default info sections. Since the
calculation of the cluster info is done "by-demand" it can lead to high
server side CPU consumption when many clients are using the "info"
command on a large cluster.
In this PR we separated the cluster info to a dedicated "cluster_stats"
section, which is not enabled by default.
Alternatives considered:
1. Revert https://github.com/valkey-io/valkey/pull/2876 - this is
possible as the information is available via `CLUSTER INFO` command. The
only downside is that some "control plane" adaptations need to be placed
in order to gather this information in addition to the "already
collected" server wide `INFO` data.
2. Optimize cluster info status collection - We could dynamically update
and maintain these statistics during cluster topology state changes. The
main "expensive" statistics which are needed to be collected are:
slots_assigned, slots_ok, slots_pfail, slots_fail, nodes_pfail,
nodes_fail, voting_nodes_pfail, voting_nodes_fail
We could track and constantly update these during cluster pings. The
main downside is potential bugs and bad accounting which might be caused
by additional complexity and unhandled edge cases.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.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>
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>
`sdssplitargs` relies on a string ending in `\0` and requires the
parameter to be of type sds. To avoid sds allocation, the
`sdsnsplitargs` method with len is supported.
---------
Signed-off-by: lizhiqiang.sf <lizhiqiang.sf@bytedance.com>
Signed-off-by: skyfirelee <739609084@qq.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>
`xtrim` on a stream with a length less than `maxlen` and exits directly
to avoid useless seek operations
Signed-off-by: lizhiqiang.sf <lizhiqiang.sf@bytedance.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>
This one was found by
[afl++](https://github.com/AFLplusplus/AFLplusplus). Executing `sort key
by *` with a non-existing key, `vectorlen` gets initially set to `0` which results
in end being set to -1. Since we're operating on an empty list here, we can just
skip the entire sorting step altogether.
---------
Signed-off-by: Fusl <fusl@meo.ws>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
## Overview:
This PR introduces two data structures, FIFO and mutexQueue, as
mentioned in #2878 , along with a significant refactoring of the
existing bio utility that demonstrates their practical value.
New Creative Components:
* FIFO - A high-performance queue that's significantly more space and
time efficient than adlist.
* mutexQueue - A thread-safe wrapper around FIFO that encapsulates
synchronization primitives into a reusable abstraction. This creative
design eliminates repetitive mutex/condition operations throughout the
codebase.
Refactored Components:
* `bio.c` - refactored using mutexQueue, with **56** lines of code
eliminated while achieving more maintainable code.
These prepare the prerequisite data structures for bgIteration.
## Details:
1. FIFO - `fifo.h`, `fifo.c`, `test_fifo.c`
2. mutexQueue - `mutexqueue.h`, `mutexqueue.c`, `test_mutexqueue.c`
3. `bio.c` Refactoring:
Before: Manual management of 3 synchronization primitives per queue
After: Single mutexQueue abstraction handles everything
Key improvements:
* Simplified synchronization - mutexQueue encapsulates mutex locking and
condition variable signaling, eliminating manual coordination in:
- `bioInit()`
- `bioSubmitJob()`
- `bioProcessBackgroundJobs()`
- `bioPendingJobsOfType()`
* Thread-safe counters - Replaced manual locking of `bio_jobs_counter`
with atomic operations
* `bioDrainWorker()` now uses polling on the queue abstraction instead
of managing low-level synchronization
## Testing:
* Unit tests for FIFO in `test_fifo.c`: all pass,
`COMPARE_PERFORMANCE_TO_ADLIST`shows the **improvement = 78.69% (List:
122 ms, FIFO: 26 ms)**
* Unit tests for mutexQueue in `test_mutexqueue.c:` all pass
* `bio.c` functionality preserved with refactored implementation
---------
Signed-off-by: Alina Liu <liusalisa6363@gmail.com>
Co-authored-by: Jim Brunner <brunnerj@amazon.com>
## Add Fuzzing Capability to Valkey
### Overview
This PR adds a fuzzing capability to Valkey, allowing developers and
users to stress test their Valkey deployments with randomly generated
commands. The fuzzer is integrated with the existing valkey-benchmark
tool, making it easy to use without requiring additional dependencies.
### Key Features
• **Command Generator**: Automatically generates Valkey commands by
retrieving command information directly from the server
• **Two Fuzzing Modes**:
- normal: Generates only valid commands, doesn't modify server
configurations
- aggressive: Includes malformed commands and allows CONFIG SET
operations
• **Multi-threaded Testing**: Each client runs in a dedicated thread to
maximize interaction between clients and enable testing of complicated
scenarios
• **Integration with valkey-benchmark**: Uses the existing CLI interface
### Implementation Details
• Added new files:
- `fuzzer_command_generator.h/c`: Dynamically generates valkey commands.
- `fuzzer_client.c`: Orchestrate all the client threads, report test
progress, and handle errors.
• Modified existing files:
- valkey-benchmark.c: Added fuzzing mode options and integration
### Command Generation Approach
The fuzzer dynamically retrieves command information from the server,
allowing it to adapt to different Valkey versions and custom modules.
Since the command information generated from JSON files is sometimes
limited, not all generated commands will be valid, but approximately 95%
valid command generation is achieved.
It is important to generate valid commands to cover as much code path as
possible and not just the invalid command/args path. The fuzzer
prioritizes generating syntactically and semantically correct commands
to ensure thorough testing of the server's core functionality, while
still including a small percentage of invalid commands in `aggressive`
mode to test error handling paths
#### Config modification
For CONFIG SET command, the situation is more complex as the server
currently provides limited information through CONFIG GET *. Some
hardcoded logic is implemented that will need to be modified in the
future. Ideally, the server should provide self-inspection commands to
retrieve config keys-values with their properties (enum values,
modifiability status, etc.).
### Issue Detection
The fuzzer is designed to identify several types of issues:
• Server crashes
• Server memory corruptions / memory leaks(when compiled with ASAN)
• Server unresponsiveness
• Server malformed replies
For unresponsiveness detection, command timeout limits are implemented
to ensure no command blocks for excessive periods. If a server doesn't
respond within 30 seconds, the fuzzer signals that something is wrong.
### Proven Effectiveness
When running against the latest unstable version, the fuzzer has already
identified several issues, demonstrating its effectiveness:
* https://github.com/valkey-io/valkey/issues/2111
* https://github.com/valkey-io/valkey/issues/2112
* https://github.com/valkey-io/valkey/pull/2109
* https://github.com/valkey-io/valkey/pull/2113
* https://github.com/valkey-io/valkey/pull/2108
* https://github.com/valkey-io/valkey/pull/2137
* https://github.com/valkey-io/valkey/issues/2106
* https://github.com/valkey-io/valkey/pull/2347
* https://github.com/valkey-io/valkey/pull/2973
* https://github.com/valkey-io/valkey/pull/2974
### How to Use
Run the fuzzer using the valkey-benchmark tool with the --fuzz flag:
```bash
# Basic usage (10000 commands 1000 commands per client, 10 clients)
./src/valkey-benchmark --fuzz -h 127.0.0.1 -p 6379 -n 10000 -c 10
# With aggressive fuzzing mode
./src/valkey-benchmark --fuzz --fuzz-level aggressive -h 127.0.0.1 -p 6379 -n 10000 -c 10
# With detailed logging
./src/valkey-benchmark --fuzz --fuzz-log-level debug -h 127.0.0.1 -p 6379 -n 10000 -c 10
```
The fuzzer supports existing valkey-benchmark options, including TLS and
cluster mode configuration.
---------
Signed-off-by: Uri Yagelnik <uriy@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>
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>
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>
Compilation warning in eval.c:244 when extracting shebang flags -
attempting to allocate 18446744073709551615 bytes (SIZE_MAX) due to
unsigned integer underflow.
```
eval.c: In function ‘evalExtractShebangFlags’:
eval.c:244:27: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
244 | *out_engine = zcalloc(engine_name_len + 1);
| ^
zmalloc.c:256:7: note: in a call to allocation function ‘valkey_calloc’ declared here
256 | void *zcalloc(size_t size) {
| ^
cd modules/lua && make OPTIMIZATION="-O3 -flto=auto -ffat-lto-objects -fno-omit-frame-pointer"
```
Signed-off-by: lizhiqiang.sf <lizhiqiang.sf@bytedance.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>
In https://github.com/valkey-io/valkey/pull/2089 we added a deferred
logic for HGETALL since we cannot anticipate the size of the output as
it may contain expired hash items which should not be included.
As part of the work of https://github.com/valkey-io/valkey/issues/2022
this would greatly increase the time for HGETALL processing, thus we
introduce this minor improvement to avoid using deferred reply in case
the hash has NO volatile items.
---------
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Change the behaviour of the CI job triggered by the run-extra-tests
label.
Run the tests immediately when applying the run-extra-tests label to a
PR, without requiring an extra commit to be pushed to trigger the test
run.
When the extra tests have run, the job removes the label.
---------
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
## API changes and user behavior:
- [x] Default behavior for database access.
Default is `alldbs` permissions.
### Database Permissions (`db=`)
- [x] Accessing particular database
```
> ACL SETUSER test1 on +@all ~* resetdbs db=0,1 nopass
"user test1 on nopass sanitize-payload ~* resetchannels db=0,1 +@all"
```
- [x] (Same behavior without usage of `resetdbs`)
```
> ACL SETUSER test1 on +@all ~* db=0,1 nopass
"user test1 on nopass sanitize-payload ~* resetchannels db=0,1 +@all"
```
- [x] Multiple selector can be provided
```
> ACL SETUSER test1 on nopass (db=0,1 +@write +select ~*) (db=2,3 +@read +select ~*)
"user test1 on nopass sanitize-payload resetchannels alldbs -@all (~* resetchannels db=0,1 -@all +@write +select) (~* resetchannels db=2,3 -@all +@read +select)"
```
- [x] Restricting special commands which access databases as part of the
command.
The user needs to have access to both the commands and db(s) part of the
command to run these commands.
1. SWAPDB
2. SELECT
3. MOVE - (Select command would have went through for the source
database). Have access for the target database.
4. COPY
- [x] Restricting special commands which doesn't specify database
number, however, accesses multiple databases.
The user needs to have access to both the commands and all databases
(`alldbs`) to run these commands.
1. FLUSHALL - Access all databases
2. CLUSTER commands that access all databases:
- CANCELSLOTMIGRATIONS
- MIGRATESLOTS
- [x] New connection establishment behavior
New client connection gets established to DB 0 by default.
Authentication and authorisation are decoupled and the user can
connect/authenticate and further perform `SELECT` or other operation
that do not access keyspace.
(Do we want to extend HELLO?) Alternative suggestion by @madolson:
Extend `HELLO` command to pass the dbid to which the user should get
connected after authentication if they have right set of permission. I
think it will become a long poll for adoption.
- [x] Observability
Extend `ACL LOG` to log user which received denied permission error
while accessing a database.
- [x] Module API
* Introduce module API `int VM_ACLCheckPermissions(ValkeyModuleUser
*user, ValkeyModuleString **argv, int argc, int dbid,
ValkeyModuleACLLogEntryReason *denial_reason);`
* Stop support of `VM_ACLCheckCommandPermissions()`.
Resolves: #1336
---------
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Adds a new option `--cluster-use-atomic-slot-migration`. This will apply
to both `--cluster reshard` and `--cluster rebalance` commands.
We could do some more optimizations here, but for now we batch all the
slot ranges for one (source, target) pair and send them off as one
`CLUSTER MIGRATESLOTS` request. We then wait for this request to finish
through polling `CLUSTER GETSLOTMIGRATIONS` once every 100ms. We parse
`CLUSTER GETSLOTMIGRATIONS` and look for the most recent migration
affecting the requested slot range, then check if it is in progress,
failed, cancelled, or successful. If there is a failure or cancellation,
we give this error to the user.
Fixes#2504
---------
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
In clusterSaveConfigOrDie, we will exit directly when saving
fails. In the case of disk failure, the cluster node will exit
immediately if there are some changes around the cluster.
We call it in many places, mainly in clusterBeforeSleep, that is,
when the cluster configuration changes and we need to save.
Passive exit may bring unexpected effects, such as cluster down.
We think the risk of metadata becoming persistently out of date
is minimal. On the one hand, we have the CLUSTER_WRITABLE_DELAY
logic, which prevents a master node from being rejoined to the
cluster in an unsafe case within 2 seconds.
```
void clusterUpdateState(void) {
/* If this is a primary node, wait some time before turning the state
* into OK, since it is not a good idea to rejoin the cluster as a writable
* primary, after a reboot, without giving the cluster a chance to
* reconfigure this node. Note that the delay is calculated starting from
* the first call to this function and not since the server start, in order
* to not count the DB loading time. */
if (first_call_time == 0) first_call_time = mstime();
if (clusterNodeIsPrimary(myself) && server.cluster->state == CLUSTER_FAIL &&
mstime() - first_call_time < CLUSTER_WRITABLE_DELAY)
return;
```
The remaining potentially worse case is that the node votes twice
in the same epoch. Like we didn't save nodes.conf and the we have
voted for replica X. We reboot and during this time X wins the failover.
After reboot, node Y requests vote for the same epoch and we vote for Y.
Y wins the failover with the same epoch. We have two primaries with
the same epoch. And we get an epoch collissions. It is resolved and some
writes are lost. It's just like a failover, some writes can be lost.
It may be very rare and it is not very bad. We will use the same
CLUSTER_WRITABLE_DELAY logic to make an optimistic judgment and prevent
the node from voting after restarting.
Added a new clusterSaveConfigOrLog, if the save fails, instead of exiting,
we will now just print a warning log. We will replace the clusterSaveConfigOrDie
in clusterBeforeSleep with this. That is, the config save triggered by
beforeSleep now will not exit the process even if the save fails.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fixes#2938
- Root cause: On 32-bit builds, off_t depended on include order.
lrulfu.c included <stdint.h>/<stdbool.h> via lrulfu.h before server.h,
so _FILE_OFFSET_BITS=64 (from fmacros.h) was not in effect when glibc
headers were first seen. This made off_t 32-bit in that TU, while 64-bit
elsewhere, causing LTO linker “type mismatch” warnings and possible
misoptimization.
- Changes:
1. Include fmacros.h first in src/lrulfu.h to ensure feature macros
apply before any system header.
2. Add a compile-time check in src/server.h:
static_assert(sizeof(off_t) >= 8, "off_t must be 64-bit; ensure
_FILE_OFFSET_BITS=64 is in effect before system headers");
so we fail fast if include order regresses.
- Why this works: fmacros.h defines _FILE_OFFSET_BITS 64 and related
feature macros. Ensuring it is seen first gives a consistent 64-bit
off_t across all TUs. The static_assert
turns future include-order mistakes into early compile-time failures
instead of link-time notes/warnings.
- Testing:
- Built on 32-bit Debian: no LTO type-mismatch at link, binaries
produced successfully. Only GCC 11 ABI notes about _Atomic alignment
(“note: … changed in GCC 11.1”), which are informational (-Wpsabi) and
do not affect correctness.
- Risk: very low; only header include order + a defensive assert. No
runtime changes.
- Address CI feedback: add fmacros.h to unit sources that include
headers before server.h;
---------
Signed-off-by: Ada-Church-Closure <2574094394@qq.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Ada-Church-Closure <2574094394@qq.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
**Problem:** temp client does not preserve db between VM_Call() calls.
SELECT changes the database context of a temporary client created for
that specific call. However, this database change is not propagated back
to the context client, so subsequent commands in the same script will
execute in the wrong database.
Behaviour on unstable:
```
127.0.0.1:6379> eval "server.call('SELECT', '1'); return server.call('SET', 'lua_test', 'incorrect')" 0
OK
127.0.0.1:6379> select 0
OK
127.0.0.1:6379> get lua_test
"incorrect"
127.0.0.1:6379> select 1
OK
127.0.0.1:6379[1]> get lua_test
(nil)
```
Behaviour with fixes:
```
127.0.0.1:6379> eval "server.call('SELECT', '1'); return server.call('SET', 'lua_test', 'correct')" 0
OK
127.0.0.1:6379> select 0
OK
127.0.0.1:6379> get lua_test
(nil)
127.0.0.1:6379> select 1
OK
127.0.0.1:6379[1]> get lua_test
"correct"
```
Signed-off-by: Daniil Kashapov <daniil.kashapov.ykt@gmail.com>
Fixed incorrect memory allocation in getClusterNodesList in
src/cluster_legacy.c. Changed zmalloc((count + 1) * CLUSTER_NAMELEN) to
zmalloc((count + 1) * sizeof(char *)) to correctly allocate memory for
an array of pointers.
Signed-off-by: Deepak Nandihalli <deepak.nandihalli@gmail.com>
This commit fixes the build of the lua module when using CLANG to
compile the code. When building with clang and with LTO enabled, the lua
module build was failing in the linking phase of the shared library.
The problem was solved by using the LLVM linker, instead of the GNU
linker, to link the lua module shared library.
We also fix, in this commit, some compiler warnings that were being
generated when building with clang.
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
## Overview
Sharing memory between the module and engine reduces memory overhead by
eliminating redundant copies of stored records in the module. This is
particularly beneficial for search workloads that require indexing large
volumes of documents.
### Vectors
Vector similarity search requires storing large volumes of
high-cardinality vectors. For example, a single vector with 512
dimensions consumes 2048 bytes, and typical workloads often involve
millions of vectors. Due to the lack of a memory-sharing mechanism
between the module and the engine, valkey-search currently doubles
memory consumption when indexing vectors, significantly increasing
operational costs. This limitation introduces adoption friction and
reduces valkey-search's competitiveness.
## Memory Allocation Strategy
At a fundamental level, there are two primary allocation strategies:
- [Chosen] Module-allocated memory shared with the engine.
- Engine-allocated memory shared with the module.
For valkey-search, it is crucial that vectors reside in cache-aligned
memory to maximize SIMD optimizations. Allowing the module to allocate
memory provides greater flexibility for different use cases, though it
introduces slightly higher implementation complexity.
## Old Implementation
The old [implementation](https://github.com/valkey-io/valkey/pull/1804)
was based on ref-counting and introduced a new SDS type. After further
discussion, we
[agreed](https://github.com/valkey-io/valkey/pull/1804#issuecomment-2905115712)
to simplify the design by removing ref-counting and avoiding the
introduction of a new SDS type.
## New Implementation - Key Points
1. The engine exposes a new interface, `VM_HashSetViewValue`, which set
value as a view of a buffer which is owned by the module. The function
accepts the hash key, hash field, and a buffer along with its length.
2. `ViewValue` is a new data type that captures the externalized buffer
and its length.
## valkey-search Usage
### Insertion
1. Upon receiving a key space notification for a new hash or JSON key
with an indexed vector attribute, valkey-search allocates cache-aligned
memory and deep-copies the vector value.
2. valkey-search then calls `VM_HashSetViewValue` to avoid keeping two
copies of the vector.
### Deletion
When receiving a key space notification for a deleted hash key or hash
field that was indexed as a vector, valkey-search deletes the
corresponding entry from the index.
### Update
Handled similarly to insertion.
---------
Signed-off-by: yairgott <yairgott@gmail.com>
Signed-off-by: Yair Gottdenker <yairg@google.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
Co-authored-by: Yair Gottdenker <yairg@google.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Jim Brunner <brunnerj@amazon.com>
* A formatting error where there was a line break in the wrong place in
a code example in a doc comment (used in the generated API docs). The
error was introduced in an automatic code formatting commit.
* Improve API doc generation script by considering release candidates
when detecting "since" for each API function. This makes it possible to
run the script on a release candidate to have the docs ready before a GA
release.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
This PR restructures the Lua scripting functionality by extracting
it from the core Valkey server into a separate Valkey module. This
change enables the possibility of a backwards compatible Lua engine
upgrade, as well as, the flexibility in building Valkey without the
Lua engine.
**Important**: from a user's point of view, there's no difference in
using the `EVAL` of `FUNCTION/FCALL` scripts. This PR is fully backward
compatible with respect to the public API.
The main code change is the move and adaptation of the Lua engine source
files from `src/lua` to `src/modules/lua`. The original Lua engine code is
adapted to use the module API to compile and execute scripts.
The main difference between the original code and the new, is the
serialization and deserialization of Valkey RESP values into, and from,
Lua values. While in the original implementation the parsing of RESP
values
was done directly from the client buffer, in the new implementation the
parsing is done from the `ValkeyModuleCallReply` object and respective
API.
The Makefile and CMake build systems were also updated to build and
integrate the new Lua engine module, within the Valkey server build
workflow.
When the Valkey server is built, the Lua engine module is also built,
and, the Lua module is loaded automatically by the server upon startup.
When running `make install` the Lua engine module is installed in the
default system library directory.
There's a new build option, called `BUILD_LUA`, that if set to `no`
allows to
build Valkey server without building the Lua engine.
This modular architecture enables future development of additional Lua
engine modules with newer Lua versions that can be loaded alongside the
current engine, facilitating gradual migration paths for users.
Additional change: Unload all modules on shutdown (ignoring modules that
can't be unloaded). This is to avoid address sanitizer warnings about
leaked allocations.
Fixes: #1627
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Provide an RDB snapshot that the replica can handle, for resilience
during rolling upgrades.
This permits an older replica to do a full sync from a newer primary.
The primary takes the replica's announced version into account when
generating the snapshot. In particular, it allows latest Valkey to send
snapshots as RDB 11 to replicas running Valkey 7.2 and 8.x.
The implementation is structurally similar to how filtered snapshots
with REPLCONF RDB-FILTER-ONLY works (commits e1e12c1f1c, 2b4727e58b) and
to the feature negotiation that replicas initiate using REPLCONF CAPA.
If any new features that the replica can't handle (such as hash-field
expiration and atomic slot migration) are in use, the full sync is
aborted and the replica connection is closed.
This mechanism will allow us to do RDB changes more often. In the recent
years, we have been avoiding RDB changes. With this mechanism, there is
no need to avoid introducing RDB changes such as new encodings and new
compression algorithms (#1962).
In my experience, providing a way to undo an upgrade makes users less
worried and actually more willing to upgrade. This is true not least
when Valkey is a part of a larger system which is upgraded as a whole.
Valkey may be just one microservice of many within a larger system; not
uncommon in on-prem deployments. If anything goes wrong (even if it's
not Valkey itself) the user wants a way to roll back the whole system to
the last working state.
---------
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
For corrupted (human-made) or program-error-recovered nodes.conf files,
check for duplicate nodeids when loading nodes.conf. If a duplicate is
found, panic is triggered to prevent nodes from starting up
unexpectedly.
The node ID is used to identify every node across the whole cluster,
we do not expect to find duplicate nodeids in nodes.conf.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Currently, when parsing querybuf, we are not checking for CRLF,
instead we assume the last two characters are CRLF by default,
as shown in the following example:
```
telnet 127.0.0.1 6379
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
*3
$3
set
$3
key
$5
value12
+OK
get key
$5
value
*3
$3
set
$3
key
$5
value12345
+OK
-ERR unknown command '345', with args beginning with:
```
This should actually be considered a protocol error. When a bug
occurs in the client-side implementation, we may execute incorrect
requests (writing incorrect data is the most serious of these).
---------
Signed-off-by: Binbin <binloveplay1314@qq.com>
- Require a 2/3 supermajority vote for all Governance Major Decisions.
- Update Technical Major Decision voting to prioritize simple majority, limiting the use of "+2" approval.
- Define remediation steps for when the 1/3 organization limit is exceeded.
---------
Signed-off-by: Ping Xie <pingxie@outlook.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
This reverts commit 5648115862.
The implementation in #2366 made it possible to perform a partial
resynchronization after loading an AOF file, by storing the replication
offset in the AOF preamble and counting the bytes in the AOF command
stream in the same way as we count byte offset in a replication stream.
However, this approach isn't safe because some commands are replicated
but not stored in the AOF file. This includes the commands REPLCONF,
PING, PUBLISH and module-implemented commands where a module can control
to propagate a command to the replication stream only, to AOF only or to
both. This oversight led to data inconsistency, where the wrong
replication offset is used for partial resynchronization as explained in
issue #2904.
The revert caused small merge conflicts with 1cf1115cca which are
solved.
Fixes#2904.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
We have the same settings for the hard limit, and we should apply them to the soft
limit as well. When the `repl-backlog-size` value is larger, all replication buffers
can be handled by the replication backlog, so there's no need to worry about the
client output buffer soft limit in here. Furthermore, when `soft_seconds` is 0, in
some ways, the soft limit behaves the same (mostly) as the hard limit.
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>
This adds the workflow improvements for PR and Release benchmark where
it runs on `c8g.metal-48xl` for `ARM64` and `c7i.metal-48xl` for `X86`.
```
Cluster mode: disabled
TLS: disabled
io-threads: 1, 9
Pipelining: 1, 10
Clients: 1600
Benchmark Treads: 90
Data size: 16 ,96
Commands: SET, GET
```
c8g.metal-48xl Spec: https://aws.amazon.com/ec2/instance-types/c8g/
c7i.metal.48xl Spec: https://aws.amazon.com/ec2/instance-types/c7i/
```
vCPU: 192
NUMA nodes: 2
Memory (GiB): 384
Network Bandwidth (Gbps): 50
```
PR benchmarking will be executed on **ARM64** machine as it has been
seen to be more consistent.
Additionally, it runs 5 iterations for each tests and posts the average
and other statistical metrics like
- CI99%: 99% Confidence Interval - range where the true population mean
is likely to fall
- PI99%: 99% Prediction Interval - range where a single future
observation is likely to fall
- CV: Coefficient of Variation - relative variability (σ/μ × 100%)
_Note: Values with (n=X, σ=Y, CV=Z%, CI99%=±W%, PI99%=±V%) indicate
averages from X runs with standard deviation Y, coefficient of variation
Z%, 99% confidence interval margin of error ±W% of the mean, and 99%
prediction interval margin of error ±V% of the mean. CI bounds [A, B]
and PI bounds [C, D] show the actual interval ranges._
For comparing between versions, it adds a workflow which runs on both
**ARM64** and **X86** machine. It will also post the comparison between
the versions like this:
https://github.com/valkey-io/valkey/issues/2580#issuecomment-3399539615
---------
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
Signed-off-by: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com>
This makes it safe to delete hashtable while a safe iterator is
iterating it. This currently isn't possible, but this improvement is
required for fork-less replication #1754 which is being actively
worked on.
We discussed these issues in #2611 which guards against a different but
related issue: calling hashtableNext again after it has already returned
false.
I implemented a singly linked list that hashtable uses to track its
current safe iterators. It is used to invalidate all associated safe
iterators when the hashtable is released. A singly linked list is
acceptable because the list length is always very small - typically zero
and no more than a handful.
Also, renames the internal functions:
hashtableReinitIterator -> hashtableRetargetIterator
hashtableResetIterator -> hashtableCleanupIterator
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
Fixes#2792
Replace strcmp with byte-by-byte comparison to avoid accidental
heap-buffer-overflow errors.
Signed-off-by: murad shahmammadli <shmurad@amazon.com>
Co-authored-by: murad shahmammadli <shmurad@amazon.com>
General cleanup on LRU/LFU code. Improve modularity and maintainability.
Specifically:
* Consolidates the mathematical logic for LRU/LFU into `lrulfu.c`, with
an API in `lrulfu.h`. Knowledge of the LRU/LFU implementation was
previously spread out across `db.c`, `evict.c`, `object.c`, `server.c`,
and `server.h`.
* Separates knowledge of the LRU from knowledge of the object containing
the LRU value. `lrulfu.c` knows about the LRU/LFU algorithms, without
knowing about the `robj`. `object.c` knows about the `robj` without
knowing about the details of the LRU/LFU algorithms.
* Eliminated `server.lruclock`, instead using `server.unixtime`. This
also eliminates the periodic need to call `mstime()` to maintain the lru
clock.
* Fixed a minor computation bug in the old `LFUTimeElapsed` function
(off by 1 after rollover).
* Eliminate specific IF checks for rollover, using defined behavior for
unsigned rollover instead.
* Fixed a bug in `debug.c` which would perform LFU modification on an
LRU value.
---------
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Closes#2883
Support a new environment variable `VALKEY_PROG_SUFFIX` in the test
framework, which can be used for running tests if the binaries are
compiled with a program suffix. For example, if the binaries are
compiled using `make PROG_SUFFIX=-alt` to produce binaries named
valkey-server-alt, valkey-cli-alt, etc., run the tests against these
binaries using `VALKEY_PROG_SUFFIX=-alt ./runtest` or simply using `make
test`.
Now the test with the make variable `PROG_SUFFIX` works well.
```
% make PROG_SUFFIX="-alt"
...
...
CC trace/trace_aof.o
LINK valkey-server-alt
INSTALL valkey-sentinel-alt
CC valkey-cli.o
CC serverassert.o
CC cli_common.o
CC cli_commands.o
LINK valkey-cli-alt
CC valkey-benchmark.o
LINK valkey-benchmark-alt
INSTALL valkey-check-rdb-alt
INSTALL valkey-check-aof-alt
Hint: It's a good idea to run 'make test' ;)
%
% make test
cd src && /Library/Developer/CommandLineTools/usr/bin/make test
CC Makefile.dep
CC release.o
LINK valkey-server-alt
INSTALL valkey-check-aof-alt
INSTALL valkey-check-rdb-alt
LINK valkey-cli-alt
LINK valkey-benchmark-alt
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 39435
Testing unit/pubsub
```
Signed-off-by: Zhijun <dszhijun@gmail.com>
Historically, Valkey’s TCL test suite expected all binaries
(src/valkey-server, src/valkey-cli, src/valkey-benchmark, etc.) to exist
under the src/ directory. This PR enables Valkey TCL tests to run
seamlessly after a CMake build — no manual symlinks or make build
required.
The test framework accepts a new environment variable `VALKEY_BIN_DIR`
to look for the binaries.
CMake will copy all TCL test entrypoints (runtest, runtest-cluster,
etc.) into the CMake build dir (e.g. `cmake-build-debug`) and insert
`VALKEY_BIN_DIR` into these. Now we can either do
./cmake-build-debug/runtest at the project root or ./runtest at the
Cmake dir to run all tests.
A new CMake post-build target prints a friendly reminder after
successful builds, guiding developers on how to run tests with their
CMake binaries:
```
Hint: It is a good idea to run tests with your CMake-built binaries ;)
./cmake-build-debug/runtest
Build finished
```
A helper TCL script `tests/support/set_executable_path.tcl` is added to
support this change, which gets called by all test entrypoints:
`runtest`, `runtest-cluster`, `runtest-sentinel`.
---------
Signed-off-by: Zhijun <dszhijun@gmail.com>
Although we can infer this infomartion from the replica logs,
i think this still would be useful to see this information directly
in the primary logs.
So now we can see the psync offset range when psync fails and then
we can analyze and adjust the value of repl-backlog-size.
Signed-off-by: Binbin <binloveplay1314@qq.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>
In functions `clusterSendFailoverAuthIfNeeded` and
`clusterProcessPacket`, we iterate through **every slot bit**
sequentially in the form of `for (int j = 0; j < CLUSTER_SLOTS; j++)`,
performing 16384 checks even when only a few bits were set, and thus
causing unnecessary loop overhead.
This is particularly wasteful in function
`clusterSendFailoverAuthIfNeeded` where we need to ensure the sender's
claimed slots all have up-to-date config epoch. Usually healthy senders
would meet such condition, and thus we normally need to exhaust the for
loop of 16384 checks.
The proposed new implementation loads 64 bits (8 byte word) at a time
and skips empty words completely, therefore only performing 256 checks.
---------
Signed-off-by: Zhijun <dszhijun@gmail.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>
Enhance debugging for cluster logs
[1] Add human node names in cluster tests so that we can easily
understand which nodes we are interacting with:
```
pong packet received from: 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) from client: :0
node 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) announces that it is a primary in shard c6d1152caee49a5e70cb4b77d1549386078be603
Reconfiguring node 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) as primary for shard c6d1152caee49a5e70cb4b77d1549386078be603
Configuration change detected. Reconfiguring myself as a replica of node 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) in shard c6d1152caee49a5e70cb4b77d1549386078be603
```
[2] Currently there are logs showing the address of incoming
connections:
```
Accepting cluster node connection from 127.0.0.1:59956
Accepting cluster node connection from 127.0.0.1:59957
Accepting cluster node connection from 127.0.0.1:59958
Accepting cluster node connection from 127.0.0.1:59959
```
but we have no idea which nodes these connections refer to. I added a
logging statement when the node is set to the inbound link connection.
```
Bound cluster node 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) to connection of client 127.0.0.1:59956
```
[3] Add a debug log when processing a packet to show the packet type,
sender node name, and sender client port (this also has the benefit of
telling us whether this is an inbound or outbound link).
```
pong packet received from: 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) from client: :0
ping packet received from: 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) from client: 127.0.0.1:59973
fail packet received from: 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) from client: 127.0.0.1:59973
auth-req packet received from: 92a960ffd62f1bd04efeb260b30fe9ca6b9294ed (R4) from client: 127.0.0.1:59973
```
---------
Signed-off-by: Zhijun <dszhijun@gmail.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>
## Problem
When executing `FLUSHALL ASYNC` on a **writable replica** that has
a large number of expired keys directly written to it, the main thread
gets blocked for an extended period while synchronously releasing the
`replicaKeysWithExpire` dictionary.
## Root Cause
`FLUSHALL ASYNC` is designed for asynchronous lazy freeing of core data
structures, but the release of `replicaKeysWithExpire` (a dictionary tracking
expired keys on replicas) still happens synchronously in the main thread.
This synchronous operation becomes a bottleneck when dealing with massive
key volumes, as it cannot be offloaded to the lazyfree background thread.
This PR addresses the issue by moving the release of `replicaKeysWithExpire`
to the lazyfree background thread, aligning it with the asynchronous design
of `FLUSHALL ASYNC` and eliminating main thread blocking.
## User scenarios
In some operations, people often need to do primary-replica switches.
One goal is to avoid noticeable impact on the business—like key loss
or reduced availability (e.g., write failures).
Here is the process: First, temporarily switch traffic to writable replicas.
Then we wait for the primary pending replication data to be fully synced
(so primry and replicas are in sync), before finishing the switch. We don't
usually need to do the flush in this case, but it's an optimization that can
be done.
Signed-off-by: Scut-Corgis <512141203@qq.com>
Signed-off-by: jiegang0219 <512141203@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
After introducing the dual channel replication in #60, we decided in #915
not to add a new configuration item to limit the replica's local replication
buffer, just use "client-output-buffer-limit replica hard" to limit it.
We need to document this behavior and mention that once the limit is reached,
all future data will accumulate in the primary side.
Signed-off-by: Binbin <binloveplay1314@qq.com>
In dual channel replication, when the rdb channel client finish
the RDB transfer, it will enter REPLICA_STATE_RDB_TRANSMITTED
state. During this time, there will be a brief window that we are
not able to see the connection in the INFO REPLICATION.
In the worst case, we might not see the connection for the
DEFAULT_WAIT_BEFORE_RDB_CLIENT_FREE seconds. I guess there is no
harm to list this state, showing connected_slaves but not showing
the connection is bad when troubleshooting.
Note that this also affects the `valkey-cli --rdb` and `--functions-rdb`
options. Before the client is in the `rdb_transmitted` state and is
released, we will now see it in the info (see the example later).
Before, not showing the replica info
```
role:master
connected_slaves:1
```
After, for dual channel replication:
```
role:master
connected_slaves:1
slave0:ip=xxx,port=xxx,state=rdb_transmitted,offset=0,lag=0,type=rdb-channel
```
After, for valkey-cli --rdb-only and --functions-rdb:
```
role:master
connected_slaves:1
slave0:ip=xxx,port=xxx,state=rdb_transmitted,offset=0,lag=0,type=replica
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
This commit adds script function flags to the module API, which allows
function scripts to specify the function flags programmatically.
When the scripting engine compiles the script code can extract the flags
from the code and set the flags on the compiled function objects.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
The original test code only checks:
The original test code only checks:
1. wait_for_cluster_size 4, which calls cluster_size_consistent for every node.
Inside that function, for each node, cluster_size_consistent queries cluster_known_nodes,
which is calculated as (unsigned long long)dictSize(server.cluster->nodes). However, when
a new node is added to the cluster, it is first created in the HANDSHAKE state, and
clusterAddNode adds it to the nodes hash table. Therefore, it is possible for the new
node to still be in HANDSHAKE status (processed asynchronously) even though it appears
that all nodes “know” there are 4 nodes in the cluster.
2. cluster_state for every node, but when a new node is added, server.cluster->state remains FAIL.
Some handshake processes may not have completed yet, which likely causes the flakiness.
To address this, added a --cluster check to ensure that the config state is consistent.
Fixes#2693.
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Co-authored-by: Binbin <binloveplay1314@qq.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>
Addresses: https://github.com/valkey-io/valkey/issues/2588
## Overview
Previously we call `emptyData()` during a fullSync before validating the
RDB version is compatible.
This change adds an rdb flag that allows us to flush the database from
within `rdbLoadRioWithLoadingCtx`. THhis provides the option to only
flush the data if the rdb has a valid version and signature. In the case
where we do have an invalid version and signature, we don't emptyData,
so if a full sync fails for that reason a replica can still serve stale
data instead of clients experiencing cache misses.
## Changes
- Added a new flag `RDBFLAGS_EMPTY_DATA` that signals to flush the
database after rdb validation
- Added logic to call `emptyData` in `rdbLoadRioWithLoadingCtx` in
`rdb.c`
- Added logic to not clear data if the RDB validation fails in
`replication.c` using new return type `RDB_INCOMPATIBLE`
- Modified the signature of `rdbLoadRioWithLoadingCtx` to return RDB
success codes and updated all calling sites.
## Testing
Added a tcl test that uses the debug command `reload nosave` to load
from an RDB that has a future version number. This triggers the same
code path that full sync's will use, and verifies that we don't flush
the data until after the validation is complete.
A test already exists that checks that the data is flushed:
https://github.com/valkey-io/valkey/blob/unstable/tests/integration/replication.tcl#L1504
---------
Signed-off-by: Venkat Pamulapati <pamuvenk@amazon.com>
Signed-off-by: Venkat Pamulapati <33398322+ChiliPaneer@users.noreply.github.com>
Co-authored-by: Venkat Pamulapati <pamuvenk@amazon.com>
Co-authored-by: Harkrishn Patro <bunty.hari@gmail.com>
Test the SCAN consistency by alternating SCAN
calls to primary and replica.
We cannot rely on the exact order of the elements and the returned
cursor number.
---------
Signed-off-by: yzc-yzc <96833212+yzc-yzc@users.noreply.github.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
By default, when the number of elements in a zset exceeds 128, the
underlying data structure adopts a skiplist. We can reduce memory usage
by embedding elements into the skiplist nodes. Change the `zskiplistNode`
memory layout as follows:
```
Before
+-------------+
+-----> | element-sds |
| +-------------+
|
+------------------+-------+------------------+---------+-----+---------+
| element--pointer | score | backward-pointer | level-0 | ... | level-N |
+------------------+-------+------------------+---------+-----+---------+
After
+-------+------------------+---------+-----+---------+-------------+
+ score | backward-pointer | level-0 | ... | level-N | element-sds |
+-------+------------------+---------+-----+---------+-------------+
```
Before the embedded SDS representation, we include one byte representing
the size of the SDS header, i.e. the offset into the SDS representation
where that actual string starts.
The memory saving is therefore one pointer minus one byte = 7 bytes per
element, regardless of other factors such as element size or number of
elements.
### Benchmark step
I generated the test data using the following lua script && cli command.
And check memory usage using the `info` command.
**lua script**
```
local start_idx = tonumber(ARGV[1])
local end_idx = tonumber(ARGV[2])
local elem_count = tonumber(ARGV[3])
for i = start_idx, end_idx do
local key = "zset:" .. string.format("%012d", i)
local members = {}
for j = 0, elem_count - 1 do
table.insert(members, j)
table.insert(members, "member:" .. j)
end
redis.call("ZADD", key, unpack(members))
end
return "OK: Created " .. (end_idx - start_idx + 1) .. " zsets"
```
**valkey-cli command**
`valkey-cli EVAL "$(catcreate_zsets.lua)" 0 0 100000
${ZSET_ELEMENT_NUM}`
### Benchmark result
|number of elements in a zset | memory usage before optimization |
memory usage after optimization | change |
|-------|-------|-------|-------|
| 129 | 1047MB | 943MB | -9.9% |
| 256 | 2010MB| 1803MB| -10.3%|
| 512 | 3904MB|3483MB| -10.8%|
---------
Signed-off-by: chzhoo <czawyx@163.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
After #2829, valgrind report a test failure, it seems that the time is
not enough to generate a COB limit in valgrind.
Signed-off-by: Binbin <binloveplay1314@qq.com>
PSYNC_FULLRESYNC_DUAL_CHANNEL is also a full sync, as the comment says,
we need to allow it. While we have not yet identified the exact edge case
that leads to this line, but during a failover, there should be no difference
between different sync strategies.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Consider this scenario:
1. Replica starts loading the RDB using the rdb connection
2. Replica finishes loading the RDB before the replica main connection has
initiated the PSYNC request
3. Replica stops replicating after receiving replicaof no one
4. Primary can't know that the replica main connection will never ask for
PSYNC, so it keeps the reference to the replica's replication buffer block
5. Primary has a shutdown-timeout configured and requires to wait for the rdb
connection to close before it can shut down.
The current 60-second wait time (DEFAULT_WAIT_BEFORE_RDB_CLIENT_FREE) is excessive
and leads to prolonged resource retention in edge cases. Reducing this timeout to
5 seconds would provide adequate time for legitimate PSYNC requests while mitigating
the issue described above.
Signed-off-by: Binbin <binloveplay1314@qq.com>
This commit fixes the cluster slot stats for scripts executed by
scripting engines when the scripts access cross-slot keys.
This was not a bug in Lua scripting engine, but `VM_Call` was missing a
call to invalidate the script caller client slot to prevent the
accumulation of stats.
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
It's handy to be able to automatically do a warmup and/or test by
duration rather than request count. 🙂
I changed the real-time output a bit - not sure if that's wanted or not.
(Like, would it break people's weird scripts? It'll break my weird
scripts, but I know the price of writing weird fragile scripts.)
```
Prepended "Warming up " when in warmup phase:
Warming up SET: rps=69211.2 (overall: 69747.5) avg_msec=0.425 (overall: 0.393) 3.8 seconds
^^^^^^^^^^
Appended running request counter when based on -n:
SET: rps=70892.0 (overall: 69878.1) avg_msec=0.385 (overall: 0.398) 612482 requests
^^^^^^^^^^^^^^^
Appended running second counter when in warmup or based on --duration:
SET: rps=61508.0 (overall: 61764.2) avg_msec=0.430 (overall: 0.426) 4.8 seconds
^^^^^^^^^^^
```
To be clear, the report at the end remains unchanged.
---------
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
As we can see, we expected to get hexpired, but we got hexpire instead,
this means tht the expiration time has expired during execution.
```
*** [err]: HGETEX EXAT keyspace notifications for active expiry in tests/unit/hashexpire.tcl
Expected 'pmessage __keyevent@* __keyevent@9__:hexpired myhash' to match 'pmessage __keyevent@* __keyevent@*:hexpire myhash'
```
We should remove the EXAT and PXAT from these fixtures. And we indeed
have
the dedicated tests that verify that we get 'expired' when EX,PX are set
to 0
or EXAT,PXAT are in the past.
Signed-off-by: Binbin <binloveplay1314@qq.com>
## Description
This change introduces the ability for modules to check ACL permissions
against key prefix. The update adds a dedicated `prefixmatchlen` helper
and extends the core ACL selector logic to support a prefix‑matching
mode.
The new API `ValkeyModule_ACLCheckPrefixPermissions` is registered and
exposed to modules, and a corresponding implementation is added in
`module.c`. Existing internal callers that already perform prefix checks
(e.g., `VM_ACLCheckKeyPermissions`) are updated to use the new flag,
while all legacy paths remain unchanged.
The change also modifies the `aclcheck§ test module that exercises the
new prefix‑checking API, ensuring that read/write operations are
correctly allowed or denied based on the ACL configuration.
Key areas touched:
* ACL logic
* Module API
* Testing
# Motivation
The search module presently makes costly calls to verify index
permissions
(see https://github.com/valkey-io/valkey-search/blob/main/src/acl.cc#L295).
This PR introduces a more efficient approach for that.
---------
Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Binbin <binloveplay1314@qq.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>
GEOADD is allocating/destroying a string object for "ZADD"
each time it is called. Created a shared string instead.
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
This increases the times we check for the logs from 20 to 40.
I found that every `wait-for` check takes about 1.5 to 1.57 milliseconds
so when we were checking 2000 times after 1ms we were actually spending
(2000 * 1) + (2000 *1.75) = 5500ms time waiting.
this can be founds under: for 10 checks we took 35 ms more so thats
around 1.75 ms per check
```
Execution time: 2034 ms (failed)
[err]: 20 100 - Test dual-channel: primary tracking replica backlog refcount - start with empty backlog in tests/integration/dual-channel-replication-flaky.tcl
```
That is why increasing it to 40 100 will check for approx 4,070 ms which
is still less than the original 5500ms but should passes every single
time here:
https://github.com/roshkhatri/valkey/actions/runs/19279424967/job/55126976882
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
The AOF preamble mechanism replaces the traditional AOF base file with
an RDB snapshot during rewrite operations, which reduces I/O overhead
and improves loading performance.
However, when valkey loads the RDB-formatted preamble from the base AOF
file, it does not process the replication ID (replid) information within
the RDB AUX fields. This omission has two limitations:
* On a primary, it prevents the primary from accepting PSYNC continue
requests after restarting with a preamble-enabled AOF file.
* On a replica, it prevents the replica from successfully performing
partial sync requests (avoiding full sync) after restarting with a
preamble-enabled AOF file.
To resolve this, this commit aligns the AOF preamble handling with the
logic used for standalone RDB files, by storing the replication ID and
replication offset in the AOF preamble and restoring them when loading
the AOF file.
Resolves#2677
---------
Signed-off-by: arthur.lee <liziang.arthur@bytedance.com>
Signed-off-by: Arthur Lee <arthurkiller@users.noreply.github.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
This commit extends the Module API to expose the `SIMPLE_STRING` and
`ARRAY_NULL` reply types to modules, by passing the new flag `X` to
the `ValkeyModule_Call` function.
By only exposing the new reply types behind a flag we keep the
backward compatibility with existing module implementations and
allow new modules to working with these reply type, which are
required for scripts to process correctly the reply type of commands
called inside scripts.
Before this change, commands like `PING` or `SET`, which return `"OK"`
as a simple string reply, would be returned as string replies to
scripts.
To allow the support of the Lua engine as an external module, we need to
distinguish between simple string and string replies to keep backward
compatibility.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
The new module context flag `VALKEYMODULE_CTX_SCRIPT_EXECUTION` denotes
that the module API function is being called in the context of a
scripting engine execution.
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
clientReplyBlock stores the size of the actual allocation in it size
field (minus the header size). This can be used for more effective
deallocation with zfree_with_size.
Signed-off-by: Vadym Khoptynets <vadymkh@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>
In this commit we add a new context for the ACL log entries that is used
to log ACL failures that occur during scripts execution. To maintain
backward compatibility we still maintain the "lua" context for failures
that happen when running Lua scripts. For other scripting engines the
context description will be just "script".
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
The `README.md` file is currently missing a section to build Valkey with
`fast_float`, which was introduced in Valkey 8.1 as an optional
dependency (#1260)
Signed-off-by: hieu2102 <hieund2102@gmail.com>
Introduce a new config `hash-seed` which can be set only at startup and
controls the hash seed for the server. This includes all hash tables.
This change makes it so that both primaries and replicas will return the
same results for SCAN/HSCAN/ZSCAN/SSCAN cursors. This is useful in order
to make sure SCAN behaves correctly after a failover.
Resolves#4
---------
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>
In `entry.c`, the `entry` is a block of memory with variable contents.
The structure can be difficult to understand. A new header comment more
clearly documents the contents/layout of the `entry`.
Also, in `entry.h`, the `entry` was defined by `typedef void entry`.
This allows blind casting to the `entry` type. It defeats compiler type
checking.
Even though the `entry` has a variable definition, we can define entry
as a legitimate type which allows the compiler to perform type checking.
By performing `typedef struct _entry entry`, now the `entry` is
understood to be a pointer to some type of undefined structure. We can
pass a pointer and the compiler can typecheck the pointer. (Of course we
can't dereference it, because we haven't actually defined the struct.)
Signed-off-by: Jim Brunner <brunnerj@amazon.com>
### Summary
Addresses https://github.com/valkey-io/valkey/issues/2619.
This PR extends the `HSETEX` command to support optional key-level `NX`
and `XX` flags, allowing operations conditional on the existence of the
hash key.
### Changes
- Updated `hsetex.json` and regenerated `commands.def`.
- Extended argument parsing for NX/XX.
- Added key-level `NX`/`XX` support in `HSETEX`.
- Added tests covering all four NX/XX scenarios.
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.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>
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>
Note: these changes are part of the effort to run Lua engine as an
external scripting engine module.
The new function `ValkeyModule_ReplyWithCustomErrorFormat` is being
added to the module API to allow scripting engines to return errors that
originated from running commands within the script code, without
counting twice in the error stats counters.
More details on why this is needed by scripting engines can be read in
an older commit 4a0fd63b69 messsage.
This PR also adds a new test to ensure the correctness of the newly
added function.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.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>
Note: these changes are another step towards being able to run Lua
engine as an external scripting engine module.
In this commit we improve the `ValkeyModule_Call` API function code to
match the validations and behavior of the `scriptCall` function,
currently used by the Lua engine to run commands using `server.call` Lua
Valkey API.
The changes made are backward compatible. The new behavior/validations
are only enabled when calling `ValkeyModule_Call` while running a script
using `EVAL` or `FCALL`.
To test these changes, we improved the `HELLO` dummy scripting engine
module to support calling commands, and compare the behavior with
calling the same command from a Lua script.
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
This commit adds a new `INFO` section called "Scripting Engines" that
shows the information of the current scripting engines available in the
server.
Here's an output example:
```
> info scriptingengines
# Scripting Engines
engines_count:2
engines_total_used_memory:68608
engines_total_memory_overhead:56
engine_0:name=LUA,module=built-in,abi_version=4,used_memory=68608,memory_overhead=16
engine_1:name=HELLO,module=helloengine,abi_version=4,used_memory=0,memory_overhead=40
```
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Skip IPv6 tests automatically when IPv6 is not available.
This fixes the problem that tests fail when IPv6 is not available on the
system, which can worry users when they run `make test`.
IPv6 availibility is detected by opening a dummy server socket and
trying to connect to it using a client socket over IPv6.
Fixes#2643
---------
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>
Currently, monotonic clock initialization relies on the model name field
from /proc/cpuinfo to retrieve the clock speed. However, this is not
always present. In case it is not present, measure the clock tick and
use it instead.
Before fix:
```
monotonic: x86 linux, unable to determine clock rate
```
After fix:
```
21695:M 25 Oct 2025 20:16:23.168 * monotonic clock: X86 TSC @ 2649 ticks/us
```
Fixes#2774
---------
Signed-off-by: Ken Nam <otherscase@gmail.com>
Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
- Moved `server-cpulist`, `bio-cpulist`, `aof-rewrite-cpulist`,
`bgsave-cpulist` configurations to ADVANCED CONFIG.
- Moved `ignore-warnings` configuration to ADVANCED CONFIG.
- Moved `availability-zone` configuration to GENERAL.
These configs were incorrectly placed at the end of the file in the
ACTIVE DEFRAGMENTATION section.
Fixes#2736
---------
Signed-off-by: ritoban23 <ankudutt101@gmail.com>
A super tiny change to optimize the function
`sentinelAskPrimaryStateToOtherSentinels` to early return when the
sentinel does not observe the primary as subjectively down.
Signed-off-by: Zhijun <dszhijun@gmail.com>
This PR introduces the support for implementing remote debuggers in
scripting engines modules.
The module API is extended with scripting engines callbacks and new
functions that can be used by scripting engine modules to implement a
remote debugger.
Most of the code that was used to implement the Lua debugger, was
refactored and moved to the `scripting_engine.c` file, and only the code
specific to the Lua engine, remained in the `debug_lua.c` file.
The `SCRIPT DEBUG (YES|NO|SYNC)` command was extend with an optional
parameter that can be used to specify the engine name, where we want to
enable the debugger. If no engine name is specified, the Lua engine is
used to keep backwards compatibility.
In
[src/valkeymodule.h](https://github.com/valkey-io/valkey/pull/1701/files#diff-b91520205c29a3a5a940786e509b2f13a5e73a1ac2016be773e62ea64c7efb28)
we see the module API changes. And in the `helloscripting.c` file we can
see how to implement a simple debugger for the dummy HELLO scripting
engine.
---------
Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
This is fixing a minor annoyance when running single tests. With this
change, the runtest script doesn't start more runners than the total
number of tests to run. These are seen in the printouts like `[ready]:
68359`.
Screenshot before:
```
$ ./runtest --single tests/unit/limits.tcl
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 68359
Testing unit/limits
[ready]: 68355
[ready]: 68362
[ready]: 68356
[ready]: 68361
[ready]: 68358
[ready]: 68364
[ready]: 68366
[ready]: 68367
[ready]: 68368
[ready]: 68357
[ready]: 68360
[ready]: 68363
[ready]: 68365
[ready]: 68369
[ready]: 68370
[ok]: Check if maxclients works refusing connections (906 ms)
[1/1 done]: unit/limits (1 seconds)
The End
Execution time of different units:
1 seconds - unit/limits
\o/ All tests passed without errors!
Cleanup: may take some time... OK
```
Screenshot after:
```
$ ./runtest --single tests/unit/limits.tcl
Cleanup: may take some time... OK
Starting test server at port 21079
[ready]: 68439
Testing unit/limits
[ok]: Check if maxclients works refusing connections (906 ms)
[1/1 done]: unit/limits (1 seconds)
The End
Execution time of different units:
1 seconds - unit/limits
\o/ All tests passed without errors!
Cleanup: may take some time... OK
```
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
When benchmarking with the a target thoughput using the `--rps` flag,
display an RPS histogram in the benchmark report. This can help identify
if there is a bottleneck.
Related to #2219.
---------
Signed-off-by: Hanxi Zhang <hanxizh@amazon.com>
This change makes it so that the module API reference can be cleanly
generated from the module.c file. Most of this seems to be because of
the code formatting work we did. There are two parts:
1. Updated some of the odd corner cases in module.c so they can be
handled. For example, there was a method that had none of the method on
the first line, which was unhandled. None of these are functional and I
think format should be OK with them.
2. Better handle multi-line function prototypes in the ruby code. Before
we just relied on the function to be on a single line, now we handle it
on multiple lines. This feels pretty hacked in, but I don't really
understand the rest of the code but it does work.
Generated this PR:
https://github.com/valkey-io/valkey-doc/pull/374/files.
---------
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
This clusterLink->flags was added in #2310, during the review, we at
the end chose to add a new CLUSTER_LINK_XXX flag instead of sharing the
old CLUSTER_NODE_XXX flag.
Signed-off-by: Binbin <binloveplay1314@qq.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>
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>
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>
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>
Resolves#2696
The primary issue was that with sanitizer mode, the test needed more
time for primary’s replication buffers grow beyond `2 × backlog_size`.
Increasing the threshold of `repl-timeout` to 30s, ensures that the
inactive replica is not disconnected while the full sync is proceeding.
`rdb-key-save-delay` controls or throttles the data written to the
client output buffer, and in this case, we are deterministically able to
perform the fullsync within 10s (10000 keys * 0.001s).
Increasing the `wait_for_condition` gives it enough retries to verify
that `mem_total_replication_buffers` reaches the required `2 ×
backlog_size`.
Signed-off-by: Sarthak Aggarwal <sarthagg@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>
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>
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>
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>
Currently, zdiff() becomes a no-op in the case that the first key is
empty.
The existing comment of "Skip everything if the smallest input is empty"
is misleading, as qsort is bypassed for the case of ZDIFF. There's no
guarantee that the first key holds the smallest cardinality.
The real reason behind such bypass is the following. ZDIFF computes the
difference between the first and all successive input sorted sets.
Meaning, if the first key is empty, we cannot reduce further from an
already empty collection, and thus zdiff() becomes a no-op.
Signed-off-by: Kyle Kim <kimkyle@amazon.com>
When using `skip` inside a test to skip a test, when running with
--dump-logs it causes the logs to be dumped. Introduced in #2342.
The reason is the "skipped" exception is caught in the start_server proc
in tests/support/server.tcl. This is where the $::dump_logs variable is
checked.
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
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>
* 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>
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
Signed-off-by: Sarthak Aggarwal <sarthagg@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>
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>
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>
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>
# 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>
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>
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>
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>
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>
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>
There is a daily test failure in valgrind, which looks like an issue related to
slowness in valgrind mode.
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Co-authored-by: Binbin <binloveplay1314@qq.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>
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.
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>
2025-08-13 09:28:44 +03:00
1072 changed files with 55007 additions and 32128 deletions
You are an expert code reviewer for the KV project. Provide helpful, constructive feedback on code quality, safety, and adherence to project standards.
## 1. Review Tone & Focus
- **Tone:** Be professional, direct, constructive, and empathetic.
- **Focus:** Critique the *code*, never the *person*.
- **Constructive:** Suggest improvements, explain *why*, provide examples.
## 2. Critical Checks
- **DCO:** **Flag missing**`Signed-off-by: Name <email>` in commits. Every commit needs it.
- **Security:** If PR fixes a security vulnerability, flag it: "Security fixes should be reported privately to security@hanzo.ai, not via public PRs."
## 3. Major Decision Detection
Flag PRs that appear to be "Technical Major Decisions" requiring TSC consensus:
- Fundamental changes to core datastructures
- New data structures or APIs
- Backward compatibility breaks
- New user-visible fields requiring long-term maintenance
- New external libraries affecting runtime behavior
**Action:** Comment mentioning **@core-team** that this appears to require TSC review and ask if consensus was reached in a linked Issue.
## 4. Documentation Reminder
If PR changes user-facing behavior (new commands, changed semantics, new config):
- **Remind** author that docs at [kv-doc](https://github.com/hanzoai/kv-doc) may need updating.
- **Suggest** linking PR to related Issue with "Fixes #xyz" pattern if applicable.
## 5. Governance Changes
**ANY change to `GOVERNANCE.md`** requires special attention - comment mentioning **@core-team** for review.
or [Valkey's Discord](https://discord.gg/zbcPa5umUB)
or [Valkey's Matrix](https://matrix.to/#/#valkey:matrix.org)
* Found a bug? [Report it here](https://github.com/valkey-io/valkey/issues/new?template=bug_report.md&title=%5BBUG%5D)
* Valkey crashed? [Submit a crash report here](https://github.com/valkey-io/valkey/issues/new?template=crash_report.md&title=%5BCRASH%5D+%3Cshort+description%3E)
* Suggest a new feature? [Post your detailed feature request here](https://github.com/valkey-io/valkey/issues/new?template=feature_request.md&title=%5BNEW%5D)
*Want to help with documentation? [Move on to valkey-doc](https://github.com/valkey-io/valkey-doc)
or [KV's Matrix](https://matrix.to/#/#kv:matrix.org)
* Found a bug? [Report it here](https://github.com/hanzoai/kv/issues/new?template=bug_report.md&title=%5BBUG%5D)
*KV crashed? [Submit a crash report here](https://github.com/hanzoai/kv/issues/new?template=crash_report.md&title=%5BCRASH%5D+%3Cshort+description%3E)
* Suggest a new feature? [Post your detailed feature request here](https://github.com/hanzoai/kv/issues/new?template=feature_request.md&title=%5BNEW%5D)
*Report a test failure? [Report it here](https://github.com/hanzoai/kv/issues/new?template=test-failure.md)
* Want to help with documentation? [Move on to kv-doc](https://github.com/hanzoai/kv-doc)
* Report a vulnerability? See [SECURITY.md](SECURITY.md)
## Developer Certificate of Origin
@@ -57,7 +58,7 @@ By making a contribution to this project, I certify that:
involved.
```
We require that every contribution to Valkey to be signed with a DCO. We require the
We require that every contribution to KV to be signed with a DCO. We require the
usage of known identity (such as a real or preferred name). We do not accept anonymous
contributors nor those utilizing pseudonyms. A DCO signed commit will contain a line like:
@@ -71,11 +72,11 @@ user.name and user.email are set in your git configs, you can use `git commit` w
or `--signoff` to add the `Signed-off-by` line to the end of the commit message. We also
require revert commits to include a DCO.
If you're contributing code to the Valkey project in any other form, including
If you're contributing code to the KV project in any other form, including
sending a code fragment or patch via private email or public discussion groups,
you need to ensure that the contribution is in accordance with the DCO.
# How to provide a patch or a new feature
## How to provide a patch or a new feature
1. If it is a major feature or a semantical change, please don't start coding
straight away: if your feature is not a conceptual fit you'll lose a lot of
@@ -85,7 +86,7 @@ features to be accepted. Here you can see if there is consensus about your idea.
2. If in step 1 you get an acknowledgment from the project leaders, use the following
procedure to submit a patch:
1. Fork Valkey on GitHub ([HOWTO](https://docs.github.com/en/github/getting-started-with-github/fork-a-repo))
1. Fork KV on GitHub ([HOWTO](https://docs.github.com/en/github/getting-started-with-github/fork-a-repo))
1. Create a topic branch (`git checkout -b my_branch`)
1. Make the needed changes and commit with a DCO. (`git commit -s`)
1. Push to your branch (`git push origin my_branch`)
@@ -99,9 +100,32 @@ certain issues/PRs over others. If you think your issue/PR is very important
try to popularize it, have other users commenting and sharing their point of
view, and so forth. This helps.
4.For minor fixes, open a pull request on GitHub.
4.While developing code, make sure to refer to our [DEVELOPMENT_GUIDE.md](DEVELOPMENT_GUIDE.md),
which includes documentation about various best practices for writing KV code.
5. For minor fixes, open a pull request on GitHub.
To link a pull request to an existing issue, please write "Fixes #xyz" somewhere
in the pull request description, where xyz is the issue number.
## Running the daily workflow on demand for your branch
Use [`.github/workflows/daily.yml`](.github/workflows/daily.yml) with
`workflow_dispatch` to run daily tests manually on any branch in your fork.
1. Open your fork on GitHub and go to **Actions** -> **Daily**.
2. Click **Run workflow**.
3. In the **Branch** dropdown, select the branch that contains the workflow file you want to use.
4. In the input fields, set:
*`use_repo` to your fork (for example, `your-user/kv`)
*`use_git_ref` to your branch name (or a specific commit SHA)
5. Optionally set `skipjobs`, `skiptests`, `test_args`, and `cluster_test_args`.
6. Click **Run workflow**.
Notes:
* To run the full matrix, set `skipjobs` and `skiptests` to `none`.
Do not leave them empty, since the workflow input defaults may be applied.
* The scheduled part of this workflow is gated to `hanzoai/kv`, but manual
If you are making material changes to a file that has a different license at the top, also add the above license snippet.
There isn't a well defined test for what is considered a material change, but a good rule of thumb is that material changes are more than 100 lines of code.
## Test coverage
KV uses two types of tests: unit and integration tests.
All contributions should include a test of some form.
Unit tests are present in the `src/unit` directory, and are intended to test individual structures or files.
For example, most changes to data structures should include corresponding unit tests.
Integration tests are located in the `tests/` directory, and are intended to test end-to-end functionality.
Adding new commands should come with corresponding integration tests.
When writing cluster mode tests, do not use the legacy `tests/cluster` framework, which has been deprecated, and instead write tests in `unit/cluster`.
## Documentation
KV keeps most of the user documentation in the [kv-doc](https://github.com/hanzoai/kv-doc) repository in a few areas:
1. Major functionality is documented in the [topics](https://github.com/hanzoai/kv-doc/tree/main/topics) section.
1. Specific command behavior is documented in the [commands](https://github.com/hanzoai/kv-doc/tree/main/commands) section.
Command history is also documented in the [command json file](https://github.com/hanzoai/kv/tree/unstable/src/commands).
1. Server info fields are documented in the [INFO](https://github.com/hanzoai/kv-doc/blob/main/commands/info.md) command.
When a PR is opened that requires documentation to be updated, the `needs-doc-pr` should be added until the corresponding documentation PR is open.
The Valkey project is managed by a Technical Steering Committee (TSC) composed of the maintainers of the Valkey repository.
The Valkey project includes all of the current and future repositories under the Valkey-io organization.
The KV project is managed by a Technical Steering Committee (TSC) composed of the maintainers of the KV repository.
The KV project includes all of the current and future repositories under the hanzoai organization.
Committers are defined as individuals with write access to the code within a repository.
Maintainers are defined as individuals with full access to a repository and own its governance.
Both maintainers and committers should be clearly listed in the MAINTAINERS.md file in a given projects repository.
Maintainers of other repositories within the Valkey project are not members of the TSC unless explicitly added.
Both maintainers and committers shall be clearly listed in the MAINTAINERS.md file in a given project's repository.
Maintainers of other repositories within the KV project are not members of the TSC unless explicitly added.
## Technical Steering Committee
The TSC is responsible for oversight of all technical, project, approval, and policy matters for Valkey.
The TSC is responsible for oversight of all technical, project, approval, and policy matters for KV.
The TSC members are listed in the [MAINTAINERS.md](MAINTAINERS.md) file in the KV repository.
At any time, no more than one third (1/3) of the TSC members may be employees, contractors, or representatives of the same organization or affiliated organizations.
For the purposes of this document, “organization” includes companies, corporations, universities, research institutes, non-profits, governmental institutions, and any of their subsidiaries or affiliates.
If, at any time, the 1/3 organization limit is exceeded (for example, due to changes in employment, company acquisitions, or organizational affiliations), the TSC shall be notified as soon as possible.
The TSC must promptly take action to restore compliance, which may include removing or reassigning members in accordance with the procedures outlined in the [Termination of Membership](#termination-of-membership) section.
The TSC shall strive to resolve the situation within 30 days of notification, and document the steps taken to restore compliance.
The TSC members are listed in the [MAINTAINERS.md](MAINTAINERS.md) file in the Valkey repository.
Maintainers (and accordingly, TSC members) may be added or removed by no less than 2/3 affirmative vote of the current TSC.
The TSC shall appoint a Chair responsible for organizing TSC meetings.
If the TSC Chair is removed from the TSC (or the Chair steps down from that role), it is the responsibility of the TSC to appoint a new Chair.
The TSC can amend this governance document by no less than a 2/3 affirmative vote.
The TSC may, at its discretion, add or remove members who are not maintainers of the main Valkey repository.
The TSC may, at its discretion, add or remove maintainers from other repositories within the Valkey project.
The TSC may, at its discretion, add or remove members who are not maintainers of the main KV repository.
The TSC may, at its discretion, add or remove maintainers from other repositories within the KV project.
## Voting
@@ -28,17 +33,34 @@ Rather, the TSC shall determine consensus based on their good faith consideratio
The TSC shall document evidence of consensus in accordance with these requirements.
If consensus cannot be reached, the TSC shall make the decision by a vote.
A vote shall also be called when an issue or pull request is marked as a major decision, which are decisions that have a significant impact on the Valkey architecture or design.
Examples of major decisions:
* Fundamental changes to the Valkey core datastructures
* Adding a new data structure or API
* Changes that affect backward compatibility
* New user visible fields that need to be maintained
* Modifications to the TSC or other governance documents
* Adding members to other roles within the Valkey project
* Delegation of maintainership for projects to other groups or individuals
* Adding or removing a new external library such as a client
or module to the project.
A vote shall also be called when an issue or pull request is marked as a major decision, which are decisions that have a significant impact on the KV architecture or design.
### Technical Major Decisions
Technical major decisions include:
* Fundamental changes to the KV core datastructures
* Adding a new data structure or API
* Changes that affect backward compatibility
* New user visible fields that need to be maintained
* Adding or removing a new external library such as a client or module to the project when it affects runtime behavior
Technical major decisions shall be approved by a simple majority vote whenever one can be obtained.
If a simple majority cannot be reached within a two-week voting period, and no TSC member has voted against, the decision may instead be approved through explicit “+2” support from at least two TSC members, recorded on the relevant issue or pull request.
If the pull request author or issue proposer is a TSC member, their +1 counts toward the +2.
If any TSC member casts a negative vote, the decision must follow the simple majority voting process and cannot be approved through +2.
Once a technical major decision has been approved through the +2 mechanism, any subsequent concerns shall be raised through a new major decision process; +2 approvals are not retracted directly.
### Governance Major Decisions
Governance major decisions include:
* Adding TSC members or involuntary removal of TSC members
* Modifying this governance document
* Delegation of maintainership for projects or governance authority
* Creating, modifying, or removing roles within the KV project
* Any change that alters voting rules, TSC responsibilities, or project oversight
* Structural changes to the TSC, including composition limits
Governance major decisions shall require approval by a super-majority vote of at least two thirds (2/3) of the entire TSC.
Any member of the TSC can call a vote with reasonable notice to the TSC, setting out a discussion period and a separate voting period.
Any discussion may be conducted in person or electronically by text, voice, or video.
@@ -46,23 +68,24 @@ The discussion shall be open to the public, with the notable exception of discus
In any vote, each voting TSC member will have one vote.
The TSC shall give at least two weeks for all members to submit their vote.
Except as specifically noted elsewhere in this document, decisions by vote require a simple majority vote of all voting members.
It is the responsibility of the TSC chair to help facilitate the voting process as needed to make sure it completes within the voting period.
If a vote results in a tie, the status quo is preserved.
It is the responsibility of the TSC Chair to help facilitate the voting process as needed to make sure it completes within the voting period.
## Termination of Membership
A maintainer's access (and accordingly, their position on the TSC) will be removed if any of the following occur:
* Involuntary Removal: Removal via the [Governance Major Decision](#governance-major-decisions) voting process.
* Resignation: Written notice of resignation to the TSC.
* TSC Vote: 2/3 affirmative vote of the TSC to remove a member
* Unreachable Member: If a member is unresponsive for more than six months, the remaining active members of the TSC may vote to remove the unreachable member by a simple majority.
## Technical direction for other Valkey projects
## Technical direction for other KV projects
The TSC may delegate decision making for other projects within the Valkey organization to the maintainers responsible for those projects.
Delegation of decision making for a project is considered a major decision, and shall happen with an explicit vote.
Projects within the Valkey organization must indicate the individuals with commit permissions by updating the MAINTAINERS.md within their repositories.
The TSC may delegate decision making for other projects within the KV organization to the maintainers responsible for those projects.
Delegation of decision making for a project is considered a [Governance Major Decision](#governance-major-decisions).
Projects within the KV organization must indicate the individuals with commit permissions by updating the MAINTAINERS.md within their repositories.
The TSC may, at its discretion, overrule the decisions made by other projects within the Valkey organization, although they should show restraint in doing so.
The TSC may, at its discretion, overrule the decisions made by other projects within the KV organization, although they shall show restraint in doing so.
Note that the network card (192.168.122.100 of this example) should support
RDMA. To test a server supports RDMA or not:
% rdma res show (a new version iproute2 package)
Or:
% ibv_devices
# Playing with Valkey
You can use valkey-cli to play with Valkey. Start a valkey-server instance,
then in another terminal try the following:
% cd src
% ./valkey-cli
valkey> ping
PONG
valkey> set foo bar
OK
valkey> get foo
"bar"
valkey> incr mycounter
(integer) 1
valkey> incr mycounter
(integer) 2
valkey>
# Installing Valkey
In order to install Valkey binaries into /usr/local/bin, just use:
% make install
You can use `make PREFIX=/some/other/directory install` if you wish to use a
different destination.
_Note_: For compatibility with Redis, we create symlinks from the Redis names (`redis-server`, `redis-cli`, etc.) to the Valkey binaries installed by `make install`.
The symlinks are created in same directory as the Valkey binaries.
The symlinks are removed when using `make uninstall`.
The creation of the symlinks can be skipped by setting the makefile variable `USE_REDIS_SYMLINKS=no`.
`make install` will just install binaries in your system, but will not configure
init scripts and configuration files in the appropriate place. This is not
needed if you just want to play a bit with Valkey, but if you are installing
it the proper way for a production system, we have a script that does this
for Ubuntu and Debian systems:
% cd utils
% ./install_server.sh
_Note_: `install_server.sh` will not work on macOS; it is built for Linux only.
The script will ask you a few questions and will setup everything you need
to run Valkey properly as a background daemon that will start again on
system reboots.
You'll be able to stop and start Valkey using the script named
`/etc/init.d/valkey_<portnumber>`, for instance `/etc/init.d/valkey_6379`.
# Building using `CMake`
In addition to the traditional `Makefile` build, Valkey supports an alternative, **experimental**, build system using `CMake`.
To build and install `Valkey`, in `Release` mode (an optimized build), type this into your terminal:
<p align="center">
<strong>Hanzo KV</strong>
</p>
<p align="center">
High-performance key-value store for the Hanzo ecosystem.<br/>
In-memory data store used as database, cache, streaming engine, and message broker.
If you believe you've discovered a security vulnerability, please contact the Valkey team at security@lists.valkey.io.
If you believe you've discovered a security vulnerability, please contact the KV team at security@hanzo.ai.
Please *DO NOT* create an issue.
We follow a responsible disclosure procedure, so depending on the severity of the issue we may notify Valkey vendors about the issue before releasing it publicly.
If you would like to be added to our list of vendors, please reach out to the Valkey team at maintainers@lists.valkey.io.
We follow a responsible disclosure procedure, so depending on the severity of the issue we may notify KV vendors about the issue before releasing it publicly.
If you would like to be added to our list of vendors, please reach out to the KV team at maintainers@hanzo.ai.
This directory contains all Valkey dependencies, except for the libc that
This directory contains all KV dependencies, except for the libc that
should be provided by the operating system.
* **Jemalloc** is our memory allocator, used as replacement for libc malloc on Linux by default. It has good performances and excellent fragmentation behavior. This component is upgraded from time to time.
* **libvalkey** is the official C client library for Valkey. It is used by valkey-cli, valkey-benchmark and Valkey Sentinel. It is managed in a separate project and updated as needed.
* **linenoise** is a readline replacement. It is developed by the same authors of Valkey but is managed as a separated project and updated as needed.
* **libkv** is the official C client library for KV. It is used by kv-cli, kv-benchmark and KV Sentinel. It is managed in a separate project and updated as needed.
* **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.
@@ -14,10 +14,10 @@ How to upgrade the above dependencies
Jemalloc
---
Jemalloc is modified with changes that allow us to implement the Valkey
active defragmentation logic. However this feature of Valkey is not mandatory
and Valkey is able to understand if the Jemalloc version it is compiled
against supports such Valkey-specific modifications. So in theory, if you
Jemalloc is modified with changes that allow us to implement the KV
active defragmentation logic. However this feature of KV is not mandatory
and KV is able to understand if the Jemalloc version it is compiled
against supports such KV-specific modifications. So in theory, if you
are not interested in the active defragmentation, you can replace Jemalloc
just following these steps:
@@ -29,7 +29,7 @@ just following these steps:
Jemalloc configuration script is broken and will not work nested in another
git repository.
However note that we change Jemalloc settings via the `configure` script of Jemalloc using the `--with-lg-quantum` option, setting it to the value of 3 instead of 4. This provides us with more size classes that better suit the Valkey data structures, in order to gain memory efficiency.
However note that we change Jemalloc settings via the `configure` script of Jemalloc using the `--with-lg-quantum` option, setting it to the value of 3 instead of 4. This provides us with more size classes that better suit the KV data structures, in order to gain memory efficiency.
If you want to upgrade Jemalloc while also providing support for
active defragmentation, in addition to the above steps you need to perform
@@ -39,7 +39,7 @@ the following additional steps:
to add `#define JEMALLOC_FRAG_HINT`.
6. Implement the function `je_get_defrag_hint()` inside `src/jemalloc.c`. You
can see how it is implemented in the current Jemalloc source tree shipped
with Valkey, and rewrite it according to the new Jemalloc internals, if they
with KV, and rewrite it according to the new Jemalloc internals, if they
changed, otherwise you could just copy the old implementation if you are
upgrading just to a similar version of Jemalloc.
@@ -59,13 +59,13 @@ cd deps/jemalloc
4. Update jemalloc's version in `deps/Makefile`: search for "`--with-version=<old-version-tag>-0-g0`" and update it accordingly.
5. Commit the changes (VERSION,configure,Makefile).
Libvalkey
Libkv
---
Libvalkey is used by Sentinel, `valkey-cli` and `valkey-benchmark`.
The library is built without its own version of the sds and dict type and uses the Valkey provided variant instead.
Libkv is used by Sentinel, `kv-cli` and `kv-benchmark`.
The library is built without its own version of the sds and dict type and uses the KV provided variant instead.
Libvalkey is the official C client for the [Valkey](https://valkey.io) database. It also supports any server that uses the `RESP` protocol (version 2 or 3). This project supports both standalone and cluster modes.
Libkv is the official C client for the [KV](https://hanzo.ai) database. It also supports any server that uses the `RESP` protocol (version 2 or 3). This project supports both standalone and cluster modes.
## Table of Contents
@@ -31,7 +31,7 @@ This library supports and is tested against `Linux`, `FreeBSD`, `macOS`, and `Wi
## Building
Libvalkey is written in C targeting C99. Unfortunately we have no plans on supporting C89 or earlier. The project does use a few widely supported compiler extensions, specifically for the bundled `sds` string library, although we have plans to remove this from the library.
Libkv is written in C targeting C99. Unfortunately we have no plans on supporting C89 or earlier. The project does use a few widely supported compiler extensions, specifically for the bundled `sds` string library, although we have plans to remove this from the library.
We support plain GNU make and CMake. Following is information on how to build the library.
@@ -66,4 +66,4 @@ sudo make install
## Contributing
Please see [`CONTRIBUTING.md`](https://github.com/valkey-io/libvalkey/blob/main/CONTRIBUTING.md).
Please see [`CONTRIBUTING.md`](https://github.com/hanzoai/kv/blob/main/CONTRIBUTING.md).
When connecting to a cluster, `NULL` is returned when the context can't be allocated, or `err` and `errstr` are set in the returned allocated context when there are issues.
So when connecting it's simple to handle error states.
There are also several flags you can specify in `valkeyClusterOptions.options`. It's a bitwise OR of the following flags:
There are also several flags you can specify in `kvClusterOptions.options`. It's a bitwise OR of the following flags:
| Flag | Description |
| --- | --- |
| `VALKEY_OPT_USE_CLUSTER_NODES` | Tells libvalkey to use the command `CLUSTER NODES` when updating its slot map (cluster topology).<br>Libvalkey uses `CLUSTER SLOTS` by default. |
| `VALKEY_OPT_USE_REPLICAS` | Tells libvalkey to keep parsed information of replica nodes. |
| `VALKEY_OPT_BLOCKING_INITIAL_UPDATE` | **ASYNC**: Tells libvalkey to perform the initial slot map update in a blocking fashion. The function call will wait for a slot map update before returning so that the returned context is immediately ready to accept commands. |
| `VALKEY_OPT_REUSEADDR` | Tells libvalkey to set the [SO_REUSEADDR](https://man7.org/linux/man-pages/man7/socket.7.html) socket option |
| `VALKEY_OPT_PREFER_IPV4`<br>`VALKEY_OPT_PREFER_IPV6`<br>`VALKEY_OPT_PREFER_IP_UNSPEC` | Informs libvalkey to either prefer IPv4 or IPv6 when invoking [getaddrinfo](https://man7.org/linux/man-pages/man3/gai_strerror.3.html). `VALKEY_OPT_PREFER_IP_UNSPEC` will cause libvalkey to specify `AF_UNSPEC` in the getaddrinfo call, which means both IPv4 and IPv6 addresses will be searched simultaneously.<br>Libvalkey prefers IPv4 by default. |
| `VALKEY_OPT_MPTCP` | Tells libvalkey 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. |
| `KV_OPT_USE_CLUSTER_NODES` | Tells libkv to use the command `CLUSTER NODES` when updating its slot map (cluster topology).<br>Libkv uses `CLUSTER SLOTS` by default. |
| `KV_OPT_USE_REPLICAS` | Tells libkv to keep parsed information of replica nodes. |
| `KV_OPT_BLOCKING_INITIAL_UPDATE` | **ASYNC**: Tells libkv to perform the initial slot map update in a blocking fashion. The function call will wait for a slot map update before returning so that the returned context is immediately ready to accept commands. |
| `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_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
@@ -91,11 +91,11 @@ The primary command interface is a `printf`-like function that takes a format st
This will construct a `RESP` command and deliver it to the correct node in the cluster.
fprintf(stderr,"Error response from node: %s\n",reply->str);
}else{
// Handle reply..
@@ -104,11 +104,11 @@ freeReplyObject(reply);
```
Commands will be sent to the cluster node that the client perceives handling the given key.
If the cluster topology has changed the Valkey node might respond with a redirection error which the client will handle, update its slot map and resend the command to correct node.
If the cluster topology has changed the KV node might respond with a redirection error which the client will handle, update its slot map and resend the command to correct node.
The reply will in this case arrive from the correct node.
If a node is unreachable, for example if the command times out or if the connect times out, it can indicate that there has been a failover and the node is no longer part of the cluster.
In this case, `valkeyClusterCommand` returns NULL and sets `err` and `errstr` on the cluster context, but additionally, libvalkey schedules a slot map update to be performed when the next command is sent.
In this case, `kvClusterCommand` returns NULL and sets `err` and `errstr` on the cluster context, but additionally, libkv schedules a slot map update to be performed when the next command is sent.
That means that if you try the same command again, there is a good chance the command will be sent to another node and the command may succeed.
### Executing commands on a specific node
@@ -116,20 +116,20 @@ That means that if you try the same command again, there is a good chance the co
When there is a need to send commands to a specific node, the following low-level API can be used.
This function handles `printf`-like arguments similar to `valkeyClusterCommand`, but will only attempt to send the command to the given node and will not perform redirects or retries.
This function handles `printf`-like arguments similar to `kvClusterCommand`, but will only attempt to send the command to the given node and will not perform redirects or retries.
If the command times out or the connection to the node fails, a slot map update is scheduled to be performed when the next command is sent.
`valkeyClusterCommandToNode` also performs a slot map update if it has previously been scheduled.
`kvClusterCommandToNode` also performs a slot map update if it has previously been scheduled.
### Disconnecting/cleanup
To disconnect and free the context the following function can be used:
```c
valkeyClusterFree(cc);
kvClusterFree(cc);
```
This function closes the sockets and deallocates the context.
@@ -138,31 +138,31 @@ This function closes the sockets and deallocates the context.
For pipelined commands, every command is simply appended to the output buffer but not delivered to the server, until you attempt to read the first response, at which point the entire buffer will be delivered.
The `valkeyClusterAppendCommand` function can be used to append a command, which is identical to the `valkeyClusterCommand` family, apart from not returning a reply.
After calling an append function `valkeyClusterGetReply` can be used to receive the subsequent replies.
The `kvClusterAppendCommand` function can be used to append a command, which is identical to the `kvClusterCommand` family, apart from not returning a reply.
After calling an append function `kvClusterGetReply` can be used to receive the subsequent replies.
The following example shows a simple cluster pipeline.
Since an initial slot map update is performed asynchronously any command sent directly after `valkeyClusterAsyncConnectWithOptions` may fail
Since an initial slot map update is performed asynchronously any command sent directly after `kvClusterAsyncConnectWithOptions` may fail
because the initial slot map has not yet been retrieved and the client doesn't know which cluster node to send the command to.
You may use the [`eventCallback`](#events-per-cluster-context-1) to be notified when the slot map is updated and the client is ready to accept commands.
A crude example of using the `eventCallback` can be found in [this test case](../tests/ct_async.c).
Another option is to enable blocking initial slot map updates using the option `VALKEY_OPT_BLOCKING_INITIAL_UPDATE`.
When enabled `valkeyClusterAsyncConnectWithOptions` will initially connect to the cluster in a blocking fashion and wait for the slot map before returning.
Another option is to enable blocking initial slot map updates using the option `KV_OPT_BLOCKING_INITIAL_UPDATE`.
When enabled `kvClusterAsyncConnectWithOptions` will initially connect to the cluster in a blocking fashion and wait for the slot map before returning.
Any command sent by the user thereafter will create a new non-blocking connection, unless a non-blocking connection already exists to the destination.
The function returns a pointer to a newly created `valkeyClusterAsyncContext` struct and its `err` field should be checked to make sure the initial slot map update was successful.
The function returns a pointer to a newly created `kvClusterAsyncContext` struct and its `err` field should be checked to make sure the initial slot map update was successful.
### Connection options
There is a variety of options you can specify using the `valkeyClusterOptions` struct when connecting to a cluster.
There is a variety of options you can specify using the `kvClusterOptions` struct when connecting to a cluster.
One asynchronous API specific option is `VALKEY_OPT_BLOCKING_INITIAL_UPDATE` which enables the initial slot map update to be performed in a blocking fashion.
One asynchronous API specific option is `KV_OPT_BLOCKING_INITIAL_UPDATE` which enables the initial slot map update to be performed in a blocking fashion.
The connect function will wait for a slot map update before returning so that the returned context is immediately ready to accept commands.
See previous [Connection options](#connection-options) section for common options.
@@ -273,21 +273,21 @@ See previous [Connection options](#connection-options) section for common option
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.
The return value is `VALKEY_OK` when the command was successfully added to the output buffer and `VALKEY_ERR` otherwise.
When the connection is being disconnected per user-request, no new commands may be added to the output buffer and `VALKEY_ERR` is returned.
The return value is `KV_OK` when the command was successfully added to the output buffer and `KV_ERR` otherwise.
When the connection is being disconnected per user-request, no new commands may be added to the output buffer and `KV_ERR` is returned.
Commands will be sent to the cluster node that the client perceives handling the given key.
If the cluster topology has changed the Valkey node might respond with a redirection error which the client will handle, update its slot map and resend the command to correct node.
If the cluster topology has changed the KV node might respond with a redirection error which the client will handle, update its slot map and resend the command to correct node.
The reply will in this case arrive from the correct node.
The reply callback, that is called when the reply is received, should have the following prototype:
This functions will only attempt to send the command to a specific node and will not perform redirects or retries, but communication errors will trigger a slot map update just like the commonly used API.
@@ -310,25 +310,25 @@ This functions will only attempt to send the command to a specific node and will
Asynchronous cluster connections can be terminated using:
```c
valkeyClusterAsyncDisconnect(acc);
kvClusterAsyncDisconnect(acc);
```
When this function is called, connections are **not** immediately terminated.
Instead, new commands are no longer accepted and connections are only terminated when all pending commands have been written to a socket, their respective replies have been read and their respective callbacks have been executed.
After this, the disconnection callback is executed with the `VALKEY_OK` status and the context object is freed.
After this, the disconnection callback is executed with the `KV_OK` status and the context object is freed.
### Events
#### Events per cluster context
Use [`event_callback` in `valkeyClusterOptions`](#events-per-cluster-context) to get notified when certain events occur.
Use [`event_callback` in `kvClusterOptions`](#events-per-cluster-context) to get notified when certain events occur.
When the callback function requires the current `valkeyClusterAsyncContext`, it can typecast the given `valkeyClusterContext` to a `valkeyClusterAsyncContext`.
The `valkeyClusterAsyncContext` struct is an extension of the `valkeyClusterContext` struct.
When the callback function requires the current `kvClusterAsyncContext`, it can typecast the given `kvClusterContext` to a `kvClusterAsyncContext`.
The `kvClusterAsyncContext` struct is an extension of the `kvClusterContext` struct.
Because the connections that will be created are non-blocking, the kernel is not able to instantly return if the specified host and port is able to accept a connection.
Instead, use a connect callback to be notified when a connection is established or failed.
Similarly, a disconnect callback can be used to be notified about a disconnected connection (either because of an error or per user request).
The callbacks can be enabled using the following options when calling `valkeyClusterAsyncConnectWithOptions`:
The callbacks can be enabled using the following options when calling `kvClusterAsyncConnectWithOptions`:
```c
valkeyClusterOptionsopt={
kvClusterOptionsopt={
.async_connect_callback=connect_cb;
.async_disconnect_callback=disconnect_cb;
}
```
The connect callback function should have the following prototype, aliased to `valkeyConnectCallback`:
The connect callback function should have the following prototype, aliased to `kvConnectCallback`:
```c
void(valkeyAsyncContext*ac,intstatus);
void(kvAsyncContext*ac,intstatus);
```
On a connection attempt, the `status` argument is set to `VALKEY_OK` when the connection was successful.
The file description of the connection socket can be retrieved from a `valkeyAsyncContext` as `ac->c->fd`.
On a connection attempt, the `status` argument is set to `KV_OK` when the connection was successful.
The file description of the connection socket can be retrieved from a `kvAsyncContext` as `ac->c->fd`.
The disconnect callback function should have the following prototype, aliased to `valkeyDisconnectCallback`:
The disconnect callback function should have the following prototype, aliased to `kvDisconnectCallback`:
```c
void(constvalkeyAsyncContext*ac,intstatus);
void(constkvAsyncContext*ac,intstatus);
```
On a disconnection the `status` argument is set to `VALKEY_OK` if it was initiated by the user, or to `VALKEY_ERR` when it was caused by an error.
On a disconnection the `status` argument is set to `KV_OK` if it was initiated by the user, or to `KV_ERR` when it was caused by an error.
When caused by an error the `err` field in the context can be accessed to get the error cause.
You don't need to reconnect in the disconnect callback since libvalkey will reconnect by itself when the next command is handled.
You don't need to reconnect in the disconnect callback since libkv will reconnect by itself when the next command is handled.
## Miscellaneous
@@ -369,22 +369,22 @@ You don't need to reconnect in the disconnect callback since libvalkey will reco
TLS support is not enabled by default and requires an explicit build flag as described in [`README.md`](../README.md#building).
When support is enabled, TLS can be enabled on a cluster context using a prepared `valkeyTLSContext` and the options `valkeyClusterOptions.tls` and `valkeyClusterOptions.tls_init_fn`.
When support is enabled, TLS can be enabled on a cluster context using a prepared `kvTLSContext` and the options `kvClusterOptions.tls` and `kvClusterOptions.tls_init_fn`.
```c
// Initialize the OpenSSL library.
valkeyInitOpenSSL();
kvInitOpenSSL();
// Initialize a valkeyTLSContext, which holds an OpenSSL context.
A `valkeyClusterNodeIterator` can be used to iterate on all known master nodes in a cluster context.
First it needs to be initiated using `valkeyClusterInitNodeIterator` and then you can repeatedly call `valkeyClusterNodeNext` to get the next node from the iterator.
A `kvClusterNodeIterator` can be used to iterate on all known master nodes in a cluster context.
First it needs to be initiated using `kvClusterInitNodeIterator` and then you can repeatedly call `kvClusterNodeNext` to get the next node from the iterator.
The iterator will handle changes due to slot map updates by restarting the iteration, but on the new set of master nodes.
There is no bookkeeping for already iterated nodes when a restart is triggered, which means that a node can be iterated over more than once depending on when the slot map update happened and the change of cluster nodes.
Note that when `valkeyClusterCommandToNode` is called, a slot map update can happen if it has been scheduled by the previous command, for example if the previous call to `valkeyClusterCommandToNode` timed out or the node wasn't reachable.
Note that when `kvClusterCommandToNode` is called, a slot map update can happen if it has been scheduled by the previous command, for example if the previous call to `kvClusterCommandToNode` timed out or the node wasn't reachable.
To detect when the slot map has been updated, you can check if the slot map version (`iter.route_version`) is equal to the current cluster context's slot map version (`cc->route_version`).
If it isn't, it means that the slot map has been updated and the iterator will restart itself at the next call to `valkeyClusterNodeNext`.
If it isn't, it means that the slot map has been updated and the iterator will restart itself at the next call to `kvClusterNodeNext`.
Another way to detect that the slot map has been updated is to [register an event callback](#events-per-cluster-context) and look for the event `VALKEYCLUSTER_EVENT_SLOTMAP_UPDATED`.
Another way to detect that the slot map has been updated is to [register an event callback](#events-per-cluster-context) and look for the event `KVCLUSTER_EVENT_SLOTMAP_UPDATED`.
### Extend the list of supported commands
The list of commands and the position of the first key in the command line is defined in [`src/cmddef.h`](../src/cmddef.h) which is included in this repository.
It has been generated using the `JSON` files describing the syntax of each command in the Valkey repository, which makes sure we support all commands in Valkey, at least in terms of cluster routing.
It has been generated using the `JSON` files describing the syntax of each command in the KV repository, which makes sure we support all commands in KV, at least in terms of cluster routing.
To add support for custom commands defined in modules, you can regenerate `cmddef.h` using the script [`gencommands.py`](../script/gencommands.py).
Use the `JSON` files from Valkey and any additional files on the same format as arguments to the script.
Use the `JSON` files from KV and any additional files on the same format as arguments to the script.
For details, see the comments inside `gencommands.py`.
Libvalkey can replace both libraries `hiredis` and `hiredis-cluster`.
This guide highlights which APIs that have changed and what you need to do when migrating to libvalkey.
Libkv can replace both libraries `hiredis` and `hiredis-cluster`.
This guide highlights which APIs that have changed and what you need to do when migrating to libkv.
The general actions needed are:
* Replace the prefix `redis` with `valkey` in API usages.
* Replace the prefix `redis` with `kv` in API usages.
* Replace the term `SSL` with `TLS` in API usages for secure communication.
* Update include paths depending on your previous installation.
All `libvalkey` headers are now found under `include/valkey/`.
All `libkv` headers are now found under `include/kv/`.
* Update used build options, e.g. `USE_TLS` replaces `USE_SSL`.
## Migrating from `hiredis` v1.2.0
@@ -17,19 +17,19 @@ The type `sds` is removed from the public API.
### Renamed API functions
*`redisAsyncSetConnectCallbackNC` is renamed to `valkeyAsyncSetConnectCallback`.
*`redisAsyncSetConnectCallbackNC` is renamed to `kvAsyncSetConnectCallback`.
### Removed API functions
*`redisFormatSdsCommandArgv` removed from API. Can be replaced with `valkeyFormatCommandArgv`.
*`redisFormatSdsCommandArgv` removed from API. Can be replaced with `kvFormatCommandArgv`.
*`redisFreeSdsCommand` removed since the `sds` type is for internal use only.
*`redisAsyncSetConnectCallback` is removed, but can be replaced with `valkeyAsyncSetConnectCallback` which accepts the non-const callback function prototype.
*`redisAsyncSetConnectCallback` is removed, but can be replaced with `kvAsyncSetConnectCallback` which accepts the non-const callback function prototype.
### Renamed API defines
*`HIREDIS_MAJOR` is renamed to `LIBVALKEY_VERSION_MAJOR`.
*`HIREDIS_MINOR` is renamed to `LIBVALKEY_VERSION_MINOR`.
*`HIREDIS_PATCH` is renamed to `LIBVALKEY_VERSION_PATCH`.
*`HIREDIS_MAJOR` is renamed to `LIBKV_VERSION_MAJOR`.
*`HIREDIS_MINOR` is renamed to `LIBKV_VERSION_MINOR`.
*`HIREDIS_PATCH` is renamed to `LIBKV_VERSION_PATCH`.
### Removed API defines
@@ -37,7 +37,7 @@ The type `sds` is removed from the public API.
## Migrating from `hiredis-cluster` 0.14.0
* The cluster client initiation procedure is changed and `valkeyClusterOptions`
* The cluster client initiation procedure is changed and `kvClusterOptions`
should be used to specify options when creating a context.
See documentation for configuration examples when using the
[Synchronous API](cluster.md#synchronous-api) or the
@@ -45,51 +45,51 @@ The type `sds` is removed from the public API.
The [examples](../examples/) directory also contains some common client
initiation examples that might be helpful.
* The default command to update the internal slot map is changed to `CLUSTER SLOTS`.
`CLUSTER NODES` can be re-enabled through options using `VALKEY_OPT_USE_CLUSTER_NODES`.
* A `valkeyClusterAsyncContext` now embeds a `valkeyClusterContext` instead of
`CLUSTER NODES` can be re-enabled through options using `KV_OPT_USE_CLUSTER_NODES`.
* A `kvClusterAsyncContext` now embeds a `kvClusterContext` instead of
holding a pointer to it. Replace any use of `acc->cc` with `&acc->cc` or similar.
### Renamed API functions
*`ctx_get_by_node` is renamed to `valkeyClusterGetValkeyContext`.
*`actx_get_by_node` is renamed to `valkeyClusterGetValkeyAsyncContext`.
*`ctx_get_by_node` is renamed to `kvClusterGetKVContext`.
*`actx_get_by_node` is renamed to `kvClusterGetKVAsyncContext`.
### Renamed API defines
*`REDIS_ROLE_NULL` is renamed to `VALKEY_ROLE_UNKNOWN`.
*`REDIS_ROLE_MASTER` is renamed to `VALKEY_ROLE_PRIMARY`.
*`REDIS_ROLE_SLAVE` is renamed to `VALKEY_ROLE_REPLICA`.
*`REDIS_ROLE_NULL` is renamed to `KV_ROLE_UNKNOWN`.
*`REDIS_ROLE_MASTER` is renamed to `KV_ROLE_PRIMARY`.
*`REDIS_ROLE_SLAVE` is renamed to `KV_ROLE_REPLICA`.
### Removed API functions
*`redisClusterConnect2` removed, use `valkeyClusterConnectWithOptions`.
*`redisClusterContextInit` removed, use `valkeyClusterConnectWithOptions`.
*`redisClusterSetConnectCallback` removed, use `valkeyClusterOptions.connect_callback`.
*`redisClusterSetEventCallback` removed, use `valkeyClusterOptions.event_callback`.
*`redisClusterSetMaxRedirect` removed, use `valkeyClusterOptions.max_retry`.
*`redisClusterSetOptionAddNode` removed, use `valkeyClusterOptions.initial_nodes`.
*`redisClusterSetOptionAddNodes` removed, use `valkeyClusterOptions.initial_nodes`.
*`redisClusterConnect2` removed, use `kvClusterConnectWithOptions`.
*`redisClusterContextInit` removed, use `kvClusterConnectWithOptions`.
*`redisClusterSetConnectCallback` removed, use `kvClusterOptions.connect_callback`.
*`redisClusterSetEventCallback` removed, use `kvClusterOptions.event_callback`.
*`redisClusterSetMaxRedirect` removed, use `kvClusterOptions.max_retry`.
*`redisClusterSetOptionAddNode` removed, use `kvClusterOptions.initial_nodes`.
*`redisClusterSetOptionAddNodes` removed, use `kvClusterOptions.initial_nodes`.
*`redisClusterSetOptionConnectBlock` removed since it was deprecated.
*`redisClusterSetOptionConnectNonBlock` removed since it was deprecated.
*`redisClusterSetOptionConnectTimeout` removed, use `valkeyClusterOptions.connect_timeout`.
*`redisClusterSetOptionMaxRetry` removed, use `valkeyClusterOptions.max_retry`.
*`redisClusterSetOptionParseSlaves` removed, use `valkeyClusterOptions.options` and `VALKEY_OPT_USE_REPLICAS`.
*`redisClusterSetOptionPassword` removed, use `valkeyClusterOptions.password`.
*`redisClusterSetOptionConnectTimeout` removed, use `kvClusterOptions.connect_timeout`.
*`redisClusterSetOptionMaxRetry` removed, use `kvClusterOptions.max_retry`.
*`redisClusterSetOptionParseSlaves` removed, use `kvClusterOptions.options` and `KV_OPT_USE_REPLICAS`.
*`redisClusterSetOptionPassword` removed, use `kvClusterOptions.password`.
*`redisClusterSetOptionRouteUseSlots` removed, `CLUSTER SLOTS` is used by default.
*`redisClusterSetOptionUsername` removed, use `valkeyClusterOptions.username`.
*`redisClusterAsyncConnect` removed, use `valkeyClusterAsyncConnectWithOptions` with options flag `VALKEY_OPT_BLOCKING_INITIAL_UPDATE`.
*`redisClusterAsyncConnect2` removed, use `valkeyClusterAsyncConnectWithOptions`.
*`redisClusterAsyncContextInit` removed, `valkeyClusterAsyncConnectWithOptions` will initiate the context.
*`redisClusterAsyncSetConnectCallback` removed, but `valkeyClusterOptions.async_connect_callback` can be used which accepts a non-const callback function prototype.
*`redisClusterAsyncSetConnectCallbackNC` removed, use `valkeyClusterOptions.async_connect_callback`.
*`redisClusterAsyncSetDisconnectCallback` removed, use `valkeyClusterOptions.async_disconnect_callback`.
*`redisClusterSetOptionUsername` removed, use `kvClusterOptions.username`.
*`redisClusterAsyncConnect` removed, use `kvClusterAsyncConnectWithOptions` with options flag `KV_OPT_BLOCKING_INITIAL_UPDATE`.
*`redisClusterAsyncConnect2` removed, use `kvClusterAsyncConnectWithOptions`.
*`redisClusterAsyncContextInit` removed, `kvClusterAsyncConnectWithOptions` will initiate the context.
*`redisClusterAsyncSetConnectCallback` removed, but `kvClusterOptions.async_connect_callback` can be used which accepts a non-const callback function prototype.
*`redisClusterAsyncSetConnectCallbackNC` removed, use `kvClusterOptions.async_connect_callback`.
*`redisClusterAsyncSetDisconnectCallback` removed, use `kvClusterOptions.async_disconnect_callback`.
*`parse_cluster_nodes` removed from API, for internal use only.
*`parse_cluster_slots` removed from API, for internal use only.
### Removed API defines
*`HIRCLUSTER_FLAG_NULL` removed.
*`HIRCLUSTER_FLAG_ADD_SLAVE` removed, flag can be replaced with an option, see `VALKEY_OPT_USE_REPLICAS`.
*`HIRCLUSTER_FLAG_ADD_SLAVE` removed, flag can be replaced with an option, see `KV_OPT_USE_REPLICAS`.
*`HIRCLUSTER_FLAG_ROUTE_USE_SLOTS` removed, the use of `CLUSTER SLOTS` is enabled by default.
### Removed support for splitting multi-key commands per slot
@@ -101,5 +101,5 @@ Commands affected are `DEL`, `EXISTS`, `MGET` and `MSET`.
_Proposed action:_
Partition the keys by slot using `valkeyClusterGetSlotByKey` before sending affected commands.
Construct new commands when needed and send them using multiple calls to `valkeyClusterCommand` or equivalent.
Partition the keys by slot using `kvClusterGetSlotByKey` before sending affected commands.
Construct new commands when needed and send them using multiple calls to `kvClusterCommand` or equivalent.
This document describes using `libvalkey` 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.
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.
## Table of Contents
@@ -31,20 +31,20 @@ The synchronous API has a pretty small surface area, with only a few commands to
### Connecting
There are several convenience functions to connect in various ways (e.g. host and port, Unix socket, etc). See [include/valkey/valkey.h](../include/valkey/valkey.h) for more details.
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.
When connecting to a server, libvalkey 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.
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 `valkeyOptions` helper struct. This includes information to connect to the server as well as other flags.
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
valkeyOptionsopt={0};
kvOptionsopt={0};
// You can set primary connection info
if(tcp){
VALKEY_OPTIONS_SET_TCP(&opt,"localhost",6379);
KV_OPTIONS_SET_TCP(&opt,"localhost",6379);
}else{
VALKEY_OPTIONS_SET_UNIX(&opt,"/tmp/valkey.sock");
KV_OPTIONS_SET_UNIX(&opt,"/tmp/kv.sock");
}
// You may attach any arbitrary data to the context
VALKEY_OPTIONS_SET_PRIVDATA(&opt,my_data);
KV_OPTIONS_SET_PRIVDATA(&opt,my_data);
```
There are also several flags you can specify when using the `valkeyOptions` helper struct.
There are also several flags you can specify when using the `kvOptions` helper struct.
| Flag | Description |
| --- | --- |
| `VALKEY_OPT_NONBLOCK` | Tells libvalkey to make a non-blocking connection. |
| `VALKEY_OPT_REUSEADDR` | Tells libvalkey to set the [SO_REUSEADDR](https://man7.org/linux/man-pages/man7/socket.7.html) socket option |
| `VALKEY_OPT_PREFER_IPV4`<br>`VALKEY_OPT_PREFER_IPV6`<br>`VALKEY_OPT_PREFER_IP_UNSPEC` | Informs libvalkey to either prefer IPv4 or IPv6 when invoking [getaddrinfo](https://man7.org/linux/man-pages/man3/gai_strerror.3.html). `VALKEY_OPT_PREFER_IP_UNSPEC` will cause libvalkey to specify `AF_UNSPEC` in the getaddrinfo call, which means both IPv4 and IPv6 addresses will be searched simultaneously.<br>Libvalkey prefers IPv4 by default. |
| `VALKEY_OPT_NO_PUSH_AUTOFREE` | Tells libvalkey 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. |
| `VALKEY_OPT_NOAUTOFREEREPLIES` | **ASYNC**: tells libvalkey not to automatically invoke `freeReplyObject` after executing the reply callback. |
| `VALKEY_OPT_NOAUTOFREE` | **ASYNC**: Tells libvalkey not to automatically free the `valkeyAsyncContext` on connection/communication failure, but only if the user makes an explicit call to `valkeyAsyncDisconnect` or `valkeyAsyncFree` |
| `VALKEY_OPT_MPTCP` | Tells libvalkey 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. |
| `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, libvalkey will execute `strlen` on each argument.
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 `valkeyCommand`
kvReply*reply=kvCommandArgv(ctx,3,argv,argvlens);
// Handle error conditions similarly to `kvCommand`
```
### Using replies
The `valkeyCommand` and `valkeyCommandArgv` functions return a `valkeyReply` on success and `NULL` in the event of a severe error (e.g. a communication failure with the server, out of memory condition, etc).
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 `valkeyContext->err` for the error code and `valkeyContext->errstr` for a human readable error string.
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 `valkeyReply` is returned, you should test the `valkeyReply->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 `VALKEY_REPLY_ERROR` and the specific error string will be in the `reply->str` member.
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
-`VALKEY_REPLY_ERROR` - An error reply. The error string is in `reply->str`.
-`VALKEY_REPLY_STATUS` - A status reply which will be in `reply->str`.
-`VALKEY_REPLY_INTEGER` - An integer reply, which will be in `reply->integer`.
-`VALKEY_REPLY_DOUBLE` - A double reply which will be in `reply->dval` as well as `reply->str`.
-`VALKEY_REPLY_NIL` - a nil reply.
-`VALKEY_REPLY_BOOL` - A boolean reply which will be in `reply->integer`.
-`VALKEY_REPLY_BIGNUM` - As of yet unused, but the string would be in `reply->str`.
-`VALKEY_REPLY_STRING` - A string reply which will be in `reply->str`.
-`VALKEY_REPLY_VERB` - A verbatim string reply which will be in `reply->str` and who's type will be in `reply->vtype`.
-`VALKEY_REPLY_ARRAY` - An array reply where each element is in `reply->element` with the number of elements in `reply->elements`.
-`VALKEY_REPLY_MAP` - A map reply, which structurally looks just like `VALKEY_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`.
-`VALKEY_REPLY_SET` - Another array-like reply representing a set (e.g. a reply from `SMEMBERS`). Access via `reply->element` and `reply->elements`.
-`VALKEY_REPLY_ATTR` - An attribute reply. As of yet unused by valkey-server.
-`VALKEY_REPLY_PUSH` - An out of band push reply. This is also array-like in nature.
-`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 libvalkey returns non-null `valkeyReply` struts you are responsible for freeing them with `freeReplyObject`. In order to disconnect and free the context simply call `valkeyFree`.
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`.
`valkeyCommand` and `valkeyCommandArgv` 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 `valkeyAppendCommand` and `valkeyAppendCommandArgv` functions.
`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 `valkeyAppendCommand` the command is simply appended to the output buffer of `valkeyContext` but not delivered to the server, until you attempt to read the first response, at which point the entire buffer will be delivered.
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.
fprintf(stderr,"Error: Non-integer reply to INCRBY?\n");
exit(1);
}
@@ -185,13 +185,13 @@ for (size_t i = 0; i < 100000; i++) {
}
```
`valkeyGetReply` 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.
`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.
@@ -199,26 +199,26 @@ while (valkeyGetReply(c, (void**)&reply) == VALKEY_OK) {
### Errors
As previously mentioned, when there is a communication error libvalkey will return `NULL` and set the `err` and `errstr` members with the nature of the problem. The specific error types are as follows.
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.
-`VALKEY_ERR_IO` - A problem with the connection.
-`VALKEY_ERR_EOF` - The server closed the connection.
-`VALKEY_ERR_PROTOCOL` - There was an error parsing the reply.
-`VALKEY_ERR_TIMEOUT` - A connect, read, or write timeout.
-`VALKEY_ERR_OOM` - Out of memory.
-`VALKEY_ERR_OTHER` - Some other error (check `c->errstr` for details).
-`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
Libvalkey context structs are **not** thread safe. You should not attempt to share them between threads, unless you really know what you're doing.
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
Libvalkey contexts have a few more mechanisms you can customize to your needs.
Libkv contexts have a few more mechanisms you can customize to your needs.
#### Maximum input buffer size
Libvalkey 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".
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;
@@ -226,7 +226,7 @@ context->reader->maxbuf = 0;
#### Maximum array elements
By default, libvalkey 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".
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".
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, libvalkey will simply process these messages and discard them.
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 `valkeyOptions` struct and set it on initialization .
**NOTE**: You may also specify a push handler in the `kvOptions` struct and set it on initialization .
Internally libvalkey 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.
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.
// libkv will return the previously set allocators.
kvAllocFuncsold=kvSetAllocators(&my_allocators);
```
They can also be reset to the glibc or musl defaults
```c
valkeyResetAllocators();
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
Libvalkey 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.
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
Libvalkey provides an `valkeyAsyncContext` to manage asynchronous connections which works similarly to the synchronous context.
Libkv provides an `kvAsyncContext` to manage asynchronous connections which works similarly to the synchronous context.
For a graceful disconnect use `valkeyAsyncDisconnect` which will block new commands from being issued.
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 `valkeyAsyncFree` which also will block new commands from being issued.
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 `VALKEY_OK` status, and the context object is freed.
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).
Libvalkey implements TLS on top of its `valkeyContext` and `valkeyAsyncContext`, so you will need to establish a connection first and then initiate a TLS handshake.
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.