Some test cases write thoughsands of commands in a pipeline and
afterwards read the replies. This can lead to TCP ACK being dropped and
the connection broken. CLIENT REPLY OFF prevents this.
"Main db not affected when fail to diskless load" in
cluster/diskless-load-swapdb has been observed to be flaky.
The others are just defensive but they follow the same pattern.
Similar fixes in the past: #3430, #2483.
Signed-off-by: Viktor Söderqvist <[email protected]>
These failures seem to be attributed to a race condition in the Aborted
test case.
`rdb-key-save-delay` 10000 was being set after `$master exec` triggered
the full resync. Since repl-diskless-sync-delay 0 was set, the master
would immediately start streaming the RDB to the replica once it
reconnected. On the ARM runner, when it's fast enough, the entire RDB
generation and transfer could complete in ~78ms, before the delay was
ever applied. This meant the replica would complete the swap and have
1010 keys instead of the expected 200 and there would be no
async_loading window to observe or abort which led to the failures.
We saw this in the daily test failure logs
```
92948:S * RDB memory usage when created 110.85 Mb
92948:S * Done loading RDB, keys loaded: 1010, keys expired: 0
```
The fix moves `rdb-key-save-delay 10000` to before `$master exec` to
guarantee the delay is in effect on the master before the RDB generation
begins.
Closes#3394, closes#3395.
Signed-off-by: Nikhil Manglore <[email protected]>
- Supports `VALKEYMODULE_CONFIG_UNSIGNED` and `UNSIGNED_CONFIG` for a
wider range of configurations.
- Supported the API for `ValkeyModule_RegisterUnsignedNumericConfig`
- `string2ull` method supports the length parameter, making it more
secure
---------
Signed-off-by: artikell <[email protected]>
Signed-off-by: skyfirelee <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Co-authored-by: Madelyn Olson <[email protected]>
The test "Blocking keyspace notification with pipelining hset after
hget" was recently failing intermittently with two different errors:
1. `Expected [expr {114 * 10 < 1114}]` - timing assertion failed under Valgrind
2. `Timeout waiting for blocked clients` - race condition on normal runs
The test used wall-clock timing to verify that hget (non-blocking)
completed faster than hset (blocking). This is unreliable because:
- Valgrind slows execution 10-50x, making timing ratios meaningless
- Fast systems may complete both operations in <10ms, causing ratio failures
This fix replaces timing assertions with blocked client count checks,
which directly verify the blocking mechanism rather than inferring it
from timing. The test now confirms hget's response is available before
hset blocks, then waits for the blocked client count to transition
through the expected states.
Signed-off-by: Rain Valentine <[email protected]>
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 <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
**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 <[email protected]>
Co-authored-by: Mangat Toor <[email protected]>
COMMAND command is expensive to process (too much string operations)
but result is rarely changes, it makes sense to add caching
---------
Signed-off-by: Evgeny Barskiy <[email protected]>
Signed-off-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Cursor <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
Reverts "Fix the memory leak in the VM_GetCommandKeysWithFlags function
(#3088)"
This reverts commit 2e2eb418f9.
Signed-off-by: Viktor Söderqvist <[email protected]>
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 <[email protected]>
Signed-off-by: Arshid <[email protected]>
Signed-off-by: Binbin <[email protected]>
Co-authored-by: Binbin <[email protected]>
## 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 <[email protected]>
## 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 <[email protected]>
Signed-off-by: Yair Gottdenker <[email protected]>
Signed-off-by: Yair Gottdenker <[email protected]>
Co-authored-by: Yair Gottdenker <[email protected]>
Co-authored-by: Ran Shidlansik <[email protected]>
Co-authored-by: Jim Brunner <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Binbin <[email protected]>
Signed-off-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Su Ko <[email protected]>
Co-authored-by: KarthikSubbarao <[email protected]>
Co-authored-by: Binbin <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
## 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 <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Signed-off-by: Ran Shidlansik <[email protected]>
Co-authored-by: Madelyn Olson <[email protected]>
Co-authored-by: Ran Shidlansik <[email protected]>
Co-authored-by: Binbin <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 aa856b39f2 messsage.
This PR also adds a new test to ensure the correctness of the newly
added function.
---------
Signed-off-by: Ricardo Dias <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
Co-authored-by: Ricardo Dias <[email protected]>
# 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
* 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 <[email protected]>
## 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 <[email protected]>
Co-authored-by: Harkrishn Patro <[email protected]>
## **Introduction**
This PR introduces a new feature that enables replicas to perform
disk-based synchronization on a dedicated background thread (Bio
thread). Benchmarking results demonstrate significant improvements in
synchronization duration. In extreme cases, this optimization allows
syncs that would have previously failed to succeed.
## **Problem Statement**
Some administrators prefer the disk-based full synchronization mode for
replicas. This mode allows replicas to continue serving clients with
data while downloading the RDB file.
Valkey's predominantly single-threaded nature creates a challenge:
serving client read requests and saving data from the socket to disk are
not truly concurrent operations. In practice, the replica alternates
between processing client requests and replication data, leading to
inefficient behavior and prolonged sync durations, especially under high
load.
## **Proposed Solution**
To address this, the solution offloads the task of downloading the RDB
file from the socket to a background thread. This allows the main thread
to focus exclusively on handling client read requests while the
background thread handles communication with the primary.
## **Benchmarking Results**
### **Potential for Improvement**
In theory, this optimization can lead to unbounded improvement in sync
duration. By eliminating competition between client read events and
socket communication (i.e., events related to handling RDB download with
the primary), sync times become independent on load - the main thread
handles only client reads, while the background thread focuses on
primary RDB download events, allowing the system to perform consistently
even under high load.
The full valkey-benchmark commands can be found in the appendix below.
### Sync Duration with Feature Disabled (times in seconds)
16 threads, 64 clients: 172 seconds
32 threads, 128 clients: 436 seconds
48 threads, 192 clients: 710 seconds
### Sync Duration with Feature Enabled (times in seconds)
16 threads, 64 clients: 33 seconds (80.8% improvement)
32 threads, 128 clients: 33 seconds (92.4% improvement)
48 threads, 192 clients: 33 seconds (95.3% improvement)
## **Alternative Solutions Considered**
**IO Threads**
IO threads to not have an advantage over Bio in this case: The
save-to-disk job is rare (most likely no more than several executions in
a replica's lifetime), and there is never more than one simultaneous
execution. Bio threads make more sense for a single, slow long running
operation.
**io_uring**
For a single connection, io_uring doesn't provide as much of a
performance boost because the primary advantage comes from batching many
I/O operations together to reduce syscall overhead. With just one
connection, we won't have enough operations to benefit significantly
from these optimizations.
**Prioritizing primary's socket in the event loop**
This approach would help, but less effectively than using a Bio thread.
We would still need to allocate attention to handling read requests,
which could limit its benefit. It could be more useful on smaller
instance types with limited CPU cores. Edit: In practice, this feature
actually does this naturally when there is a single core - when both
threads are running on the same core, the Bio thread can get up to 50%
of CPU time now to handle RDB download, whereas before RDB download
events were queued after a bunch of read events (and didn't get even
close to 50%).
## **Code Design**
This PR introduces both the new BIO-based RDB sync flow described above
and a significant (and much-needed) refactoring of the
readSyncBulkPayload function. Previously, this single monolithic
function handled the entire replication flow, covering both disk-based
and socket-based syncs, and was responsible for everything from sync
preparation and RDB reception to loading and finalization. The
refactoring splits these responsibilities into clearer, more modular
components, improving readability, reusability, and maintainability.
### Disk-Based RDB Sync (Bio thread)
The old disk-based RDB save logic has been removed from the main thread.
It is now exclusively handled by a dedicated Bio thread, following the
team’s decision to remove the need for a config.
New Flow Overview:
After the replication handshake completes in `syncWithPrimary()` (or at
the end of the dual-channel handshake), the replica determines whether
to use disk-based sync via `useDisklessLoad()` (unchanged).
If disk sync is chosen, a read handler is set on the primary’s
connection:
`connSetReadHandler(conn, receiveRDBinBioThread);`
When the primary begins sending the RDB payload, this read handler is
triggered. It creates a dedicated Bio thread to perform the sync.
The Bio thread executes `replicaReceiveRDBFromPrimaryToDisk`, which is
now the main handler for receiving and saving RDB to disk.
This function replaces the disk logic previously found in
`readSyncBulkPayload`, but executes in a busy loop (as opposed to being
event-driven like `readSyncBulkPayload`) since the Bio thread is
single-purpose and not event-based.
The logic inside `replicaReceiveRDBFromPrimaryToDisk` is a direct port
of the previous flow, with some refactoring for clarity and reusability
across sync modes.
Upon completion or failure, the Bio thread signals the main thread using
a shared variable (`replica_bio_disk_save_state`).
The main thread detects Bio completion in a new function called
`handleBioThreadFinishedRDBDownload()` (triggered via replication cron).
This function then initiates the RDB load and resets relevant stats.
Note: The actual RDB loading remains on the main thread, as it already
halts I/O during load and doesn’t benefit from thread offloading,
simplifying thread-safety concerns.
### Diskless Sync (Socket)
For socket-based sync, the logic remains the same: asynchronous and
event-driven, but has been refactored for modularity and readability.
The original implementation in `readSyncBulkPayload` mixed socket and
disk sync logic with heavy branching.
Now, memory-based sync is isolated in a new function:
`replicaReceiveRDBFromPrimaryToMemory`
This function retains the same execution pattern as
`readSyncBulkPayload` (i.e., reacting to available data on the socket),
but is now cleaner and easier to follow.
Common logic that was previously duplicated across both sync modes is
now factored out into shared helpers:
`replicaBeforeLoadPrimaryRDB()`
`replicaAfterLoadPrimaryRDB()`
The result is a more maintainable and modular flow. Reviewers are
encouraged to compare this function side-by-side with the previous
`readSyncBulkPayload` in order to understand the change easily.
### Metrics
Previously, replication metrics such as `repl_transfer_size` and
`repl_transfer_read` were tracked on the main thread inside
`readSyncBulkPayload`. With the move to Bio threads, those variables
could not be safely reused due to thread safety concerns.
To address this:
New Bio-specific metrics are introduced:
`server.bio_repl_transfer_size`
`server.bio_repl_transfer_read`
These are updated by the Bio thread during disk-based syncs.
At the end of the sync, the values are merged back into the standard
metrics (server.repl_transfer_*) for consistency.
If INFO is queried during an ongoing Bio-based sync, the reported values
reflect either the Bio-specific metrics or a combined view when needed,
ensuring accurate observability.
### Appendix:
### **Benchmarking Setup**
- **Client machine:** AWS c5a.16xlarge
- **Server machines:** AWS c5a.2xlarge
```bash
# Step 1: Fill the primary and replica DBs with 6GB of data:
./valkey-benchmark -h <host> -p <port> -l -d 128 -t set -r 30000000 --threads 16 -c 64
# Step 2: Initiate heavy read load on the replica:
./valkey-benchmark -h <host> -p <port> -t get -r 30000000 --threads <t> -c <t> -n 1000000000 -P <P>
# Step 3: Enable/disable the config controlling the new feature:
./valkey-cli -h <host> -p <port> config set replica-save-to-disk-in-bio-thread <yes/no>
# Step 4: Initiate sync:
./valkey-cli -h <replica host> -p <replica port> replicaof <primary host> <primary port>
---------
Signed-off-by: Nitai Caro <[email protected]>
Co-authored-by: Nitai Caro <[email protected]>
When requirepass is enabled, we want command calls to return NOAUTH
instead of ERR with the error message.
Previously, these checks were all before the auth check:
- command existence exist
- command arity check
- command protected check
This may expose information such as whether the server supports the
command, whether the configuration item is enabled, etc. This is more
of a consistency issue as the same error message is returned when
requirepass is enabled, not a security issue.
This is a behavior change, though perhaps not a breaking one.
We put the auth check into !client_reprocessing_command block, that
means the reprocessing command would skip auth check, it's also a behavior
change but align with the old behavior before the refactor of reprocessing,
so it should be fine.
Before:
```
127.0.0.1:6379> foo bar
(error) ERR unknown command 'foo', with args beginning with: 'bar'
127.0.0.1:6379> set foo
(error) ERR wrong number of arguments for 'set' command
127.0.0.1:6379> module load foo
(error) ERR MODULE command not allowed...
```
After:
```
127.0.0.1:6379> foo bar
(error) NOAUTH Authentication required.
127.0.0.1:6379> set foo
(error) NOAUTH Authentication required.
127.0.0.1:6379> module load foo
(error) NOAUTH Authentication required.
```
Signed-off-by: Binbin <[email protected]>
Previously, the `next_yield_time` for a yielding module command was only
correctly calculated for the initial yield, taking into account whether
the server was loading an RDB. Subsequent yields incorrectly defaulted
to using `server.hz` for the calculation, regardless of the loading
state.
This bug had two consequences:
1. Incorrect Yield Timing: If not loading, the module command would
yield based on `server.hz` instead of the configured
`busy_reply_threshold`. This could lead to the main thread being blocked
for significantly longer or shorter periods than intended between
yields.
2. Test Flakiness: In the `blockedclient.tcl` test, this incorrect
timing meant that new commands from other clients were delayed until the
current, potentially overly long, yield cycle completed. With
`busy_time_limit` set to 50s, this significantly increased test duration
and risked timeouts, causing flakiness.
This commit refactors the `next_yield_time` calculation into a new
static function `computeNextYieldTime` and ensures it's called both
during the initial context creation and within `VM_Yield` after each
yield. This guarantees that `next_yield_time` is always predicated on
the current `server.loading` status.
The `busy_time_limit` in `blockedclient.tcl` has also been reduced from
50s to 5s. With the primary bug addressed, this shorter timeout is more
appropriate and helps the test complete faster, reducing the risk of
timeouts.
---------
Signed-off-by: Ping Xie <[email protected]>
I try to running moduleapi test and this command may test failed:
./runtest-moduleapi --single "unit/moduleapi/scan" --only "Module scan hash dict"
I found that the test `Module scan hash dict` and `Module scan hash
listpack` use the same key `hh`. They affected each other.
This PR adjust these tests to make them independent of each other.
---------
Signed-off-by: vitah <[email protected]>
Signed-off-by: Vitah Lin <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
Some Tcl test suites set the global variable `::singledb`. If they don't
restore the value afterwards, they can affect other test suites, which
leads to problems that are hard to track down.
This change introduces a test tag 'singledb' that has the same effect,
but it is local to the scope of the tag (a tags block, start_server or a
single test case). The tests should use the tag instead of setting the
global.
Additional change: Use ECHO instead of PING in `valkey_deferring_client`
and `valkey_client` procs when singledb is set, to avoid failing when
the server is loading. (SELECT and ECHO are allowed while loading, but
not PING.)
---------
Signed-off-by: Viktor Söderqvist <[email protected]>
This change enhances user experience and consistency by allowing a
module to block a client on keyspace event notifications. Consistency is
improved by allowing that reads after writes on the same connection
yield expected results. For example, in ValkeySearch, mutations
processed earlier on the same connection will be available for search.
The implementation extends `VM_BlockClient` to support blocking clients
on keyspace event notifications. Internal clients, LUA clients, clients
issueing multi exec and those with the `deny_blocking` flag set are not
blocked. Once blocked, a client’s reply is withheld until it is
explicitly unblocked.
---------
Signed-off-by: yairgott <[email protected]>
This change re-introduce large-memory tests to be run as solo tests on
daily runs.
the change includes:
1. separate the large-memory tests into separate test block (mainly
helpful in workflow manual dispatch cases)
2. place all current large memory tests in run solo blocks in order to
prevent potential runner OOM failures.
Signed-off-by: Ran Shidlansik <[email protected]>
There is a timing issue in the test:
```
*** [err]: module RM_Call of expired key propagation in tests/unit/moduleapi/propagate.tcl
Expected '1' to be equal to '2' (context: type eval line 27 cmd {assert_equal [$replica propagate-test.obeyed] 2} proc ::test)
```
We should wait for sync, otherwise the replica may not be fully
synchronized before checking. The test was introduced in #1582.
Signed-off-by: Binbin <[email protected]>
**Problem**
The cluster module test "Cluster module message DING/DONG
acknowledgment" is failing despite successful message exchanges between
nodes. While these messages are being properly exchanged between the
nodes, running multiple tests on the same cluster instance is causing
issues with message statistics tracking. `CONFIG RESETSTAT` doesn't seem
to reset the stats under certain cases which causes the error as the
stats are adding onto each other from previous tests.
**Solution**
Remove some overlap with the previous test case so we don't need `CONFIG
RESETSTAT` between the test cases.
Resolves#1764
---------
Signed-off-by: Nikhil Manglore <[email protected]>
* Pass `sender_id` as `NULL` terminated string as part of
`ValkeyModuleClusterMessageReceiver` for ease of usage by the module(s).
* Implement test coverage described in #1656 and ensures that nodes in
the cluster module properly acknowledge a "DING" message by sending a
"DONG" response.
Closes#1656.
---------
Signed-off-by: Nikhil Manglore <[email protected]>
CI report this failure:
```
ValueError: invalid literal for int() with base 10: ':100\r\n'
```
We were writing out a second response on the error.
Signed-off-by: Madelyn Olson <[email protected]>
This PR adds support for MustObeyClient Module API.
The purpose of this API is for Modules to handle commands / callbacks
knowing whether commands are arriving from the primary client or AOF
client and should never be rejected.
A use case is that Modules can have validation logic in command handlers
which only should be executed on primary nodes. Replica nodes must obey
commands replicated.
---------
Signed-off-by: KarthikSubbarao <[email protected]>
Signed-off-by: Karthik Subbarao <[email protected]>
In this PR we re-implement the `EVAL` commands (`EVAL`, `EVALSHA`,
`SCRIPT LOAD`, etc...) to use the scripting engine infrastructure
introduced in 6adef8e2f9. This allows
`EVAL` to run scripts using different scripting engines.
The Lua scripting engine implementation code was moved into its own
subdirectory `src/lua`.
This new implementation generalizes the module API for implementing
scripting engines to work with both `FUNCTION` and `EVAL` commands.
Module API changes include:
* Rename of callback
`ValkeyModuleScriptingEngineCreateFunctionsLibraryFunc` to
`ValkeyModuleScriptingEngineCompileCodeFunc`.
* Addition of a new enum `enum ValkeyModuleScriptingEngineSubsystemType`
to specify the scripting engine subsystem (EVAL, or FUNCTION, or both).
* In most callbacks was added a new parameter with the type
`ValkeyModuleScriptingEngineSubsystemType`.
* New callback specific for EVAL
`ValkeyModuleScriptingEngineResetEvalEnvFunc`.
* New API function `ValkeyModuleScriptingEngineExecutionState
(*ValkeyModule_GetFunctionExecutionState)(ValkeyModuleScriptingEngineServerRuntimeCtx
*server_ctx)` that is used by scripting engines to query the server
about the execution state of the script that is running.
Fixes#1261Fixes#1468
Follow-up of #1277
---------
Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
Co-authored-by: Ping Xie <[email protected]>
The `serverCron()` function contains a variety of maintenance functions
and is set up as a timer job, configured to run at a certain rate (hz).
The default rate is 10hz (every 100ms).
One of the things that `serverCron()` does is to perform maintenance
functions on connected clients. Since the number of clients is variable,
and can be very large, this could cause latency spikes when the 100ms
`serverCron()` task gets invoked. To combat those latency spikes, a
feature called "dynamic-hz" was introduced. This feature will run
`serverCron()` more often, if there are more clients. The clients get
processed up to 200 at a time. The delay for `serverCron()` is shortened
with the goal of processing all of the clients every second.
The result of this is that some of the other (non-client) maintenance
functions also get (unnecessarily) run more often. Like
`cronUpdateMemoryStats()` and `databasesCron()`. Logically, it doesn't
make sense to run these functions more often, just because we happen to
have more clients attached.
This PR separates client activities onto a separate, variable, timer.
The "dynamic-hz" feature is eliminated. Now, `serverCron` will run at a
standard configured rate. The separate clients cron will automatically
adjust based on the number of clients. This has the added benefit that
often, the 2 crons will fire during separate event loop invocations and
will usually avoid the combined latency impact of doing both maintenance
activities together.
The new timer follows the same rules which were established with the
dynamic HZ feature.
* The goal is to process all of the clients once per second
* We never want to process more than 200 clients in a single invocation
(`MAX_CLIENTS_PER_CLOCK_TICK`)
* We always process at least 5 clients at a time
(`CLIENTS_CRON_MIN_ITERATIONS`)
* The minimum rate is determined by HZ
The delay (ms) for the new timer is also more precise, computing the
number of milliseconds needed to achieve the goal of reaching all of the
clients every second. The old dynamic-hz feature just performs a
doubling of the HZ until the clients processing rate is achieved (i.e.
delays of 100ms, 50ms, 25ms, 12ms...)
---------
Signed-off-by: Jim Brunner <[email protected]>
Co-authored-by: Binbin <[email protected]>
This change uses the light message header for cluster module message
type to be sent to a target node or broadcast across the cluster. The
light message header was introduced in Valkey 8 to reduce network in/out
over clusterbus and have been already used for pub/sub message transfer.
It's only used if the target node supports the light message header
(~30B) otherwise it falls back to using the regular header (~2KB). The
module API `VM_SendClusterMessage` remains as is.
---------
Signed-off-by: Harkrishn Patro <[email protected]>
Signed-off-by: Harkrishn Patro <[email protected]>
Co-authored-by: Viktor Söderqvist <[email protected]>
This commit creates a new compilation unit for the scripting engine code
by extracting the existing code from the functions unit.
We're doing this refactor to prepare the code for running the `EVAL`
command using different scripting engines.
This PR has a module API change: we changed the type of error messages
returned by the callback
`ValkeyModuleScriptingEngineCreateFunctionsLibraryFunc` to be a
`ValkeyModuleString` (aka `robj`);
This PR also fixes#1470.
---------
Signed-off-by: Ricardo Dias <[email protected]>
When latency-monitor-threshold is set to 0, it means the latency monitor
is disabled, and in VM_LatencyAddSample, we wrote the condition
incorrectly, causing us to record latency when latency was turned off.
This bug was introduced in the very first day, see e3b1d6d, it was merged
in 2019.
Signed-off-by: Binbin <[email protected]>
