[backport] Backport sweep for 9.0 - #3879
Merged
Merged
Conversation
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
valkeyrie-ops
Bot
force-pushed
the
agent/backport/sweep/9.0
branch
from
June 5, 2026 10:43
7c33dca to
ae5c070
Compare
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
![valkeyrie-bot[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
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>
…onfigured (#2846) When dual-channel-replication is enabled, and replica-announce-ip is set, the RDB/AOF channel does not announce itself at this endpoint. This defaults to the IP address behind the NAT, or the Kubernetes Pod IP in our case. This means that if Sentinel is polling the primary for connected replicas, it will first see the ephemeral pod IP, then revert to the announce-ip - leaving behind the pod IP as a down replica. This PR configures the RDB/AOF channel to also announce itself at the announce-ip to prevent the stale replica. ## Testing I evaluated writing unit tests for this, but I am not sure of a way we can test an IP address different to localhost (127.0.0.1) that would fail without the fix. I did test on Kubernetes against 9.0 tag and verified the fix there too. ### Status quo On 9.0 image tag: ``` $ kubectl get pods -n valkey-baseline -o custom-columns=NAME:.metadata.name,POD-IP:.status.podIP NAME POD-IP valkey-primary-5bd78c8566-llb6k 10.244.0.25 valkey-replica-0 10.244.0.17 valkey-replica-1 10.244.0.13 $ kubectl get services -n valkey-baseline -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.spec.clusterIP NAME CLUSTER-IP valkey-primary 10.96.147.28 valkey-replica-0 10.96.66.233 valkey-replica-1 10.96.57.230 ``` Logs below show that pod IP for valkey-primary-5bd78c8566-llb6k `10.244.0.25:6379` is being used for dual-channel replication. This should be its cluster IP `10.96.147.28` as this is what is set in replica-announce-ip. ``` 1:M 14 Nov 2025 17:57:51.750 * Replica 10.96.147.28:6379 asks for synchronization 1:M 14 Nov 2025 17:57:51.751 * Replica 10.244.0.25:6379 asks for synchronization 1:M 14 Nov 2025 17:57:56.135 * Dual channel replication: Sending to replica 10.244.0.25:6379 RDB end offset 1763269 and client-id 35 1:M 14 Nov 2025 17:57:56.140 * Replica 10.96.147.28:6379 asks for synchronization ``` ### This fix ``` $ kubectl get pods -n valkey-test -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.status.podIP NAME POD-IP valkey-primary-594c9597b5-qqvdk 10.244.0.26 valkey-replica-0 10.244.0.10 valkey-replica-1 10.244.0.18 $ kubectl get services -n valkey-test -o custom-columns=NAME:.metadata.name,CLUSTER-IP:.spec.clusterIP NAME CLUSTER-IP valkey-primary 10.96.125.142 valkey-replica None valkey-replica-0 10.96.155.74 valkey-replica-1 10.96.64.111 valkey-sentinel None ``` Logs show that the Cluster IP is now being used for dual-channel replication. ``` 1:M 14 Nov 2025 17:57:49.923 * Replica 10.96.125.142:6379 asks for synchronization 1:M 14 Nov 2025 17:57:49.924 * Replica 10.96.125.142:6379 asks for synchronization 1:M 14 Nov 2025 17:57:54.913 * Dual channel replication: Sending to replica 10.96.125.142:6379 RDB end offset 1771247 and client-id 36 1:M 14 Nov 2025 17:57:54.916 * Replica 10.96.125.142:6379 asks for synchronization ``` Fixes #2338 Signed-off-by: Joseph Heyburn <jdheyburn@gmail.com>
In `evalMode()`, `argv2` is allocated with `zmalloc()` but freed with `free()`. On builds using jemalloc/tcmalloc, this can cause `free(): invalid pointer` Closes #3273 Signed-off-by: Su Ko <rhtn1128@gmail.com>
Noticed a use-after-free/double-free bug while load-testing manual slot migrations. Was able to reliably trigger a process crash. Original trace: ``` Backtrace: 0 libsystem_platform.dylib 0x00000001917496a4 _sigtramp + 56 1 libsystem_pthread.dylib 0x000000019170f848 pthread_kill + 296 2 libsystem_c.dylib 0x00000001916189e4 abort + 124 3 libsystem_malloc.dylib 0x000000019151c174 malloc_vreport + 892 4 libsystem_malloc.dylib 0x000000019151fc90 malloc_report + 64 5 libsystem_malloc.dylib 0x000000019152421c ___BUG_IN_CLIENT_OF_LIBMALLOC_POINTER_BEING_FREED_WAS_NOT_ALLOCATED + 32 6 valkey-server 0x0000000104fc2174 _sdsMakeRoomFor + 780 7 valkey-server 0x0000000104fc30a0 sdscatfmt + 168 8 valkey-server 0x00000001050318b8 generateSyncSlotsEstablishCommand + 176 9 valkey-server 0x000000010503156c createSlotImportJob + 324 10 valkey-server 0x0000000105034fd4 clusterCommandSyncSlots + 1572 11 valkey-server 0x000000010502f370 clusterCommandSpecial + 2432 12 valkey-server 0x0000000105021e80 clusterCommand + 2992 13 valkey-server 0x0000000104fb59a8 call + 320 14 valkey-server 0x0000000104fcc708 processCommandAndResetClient + 3088 15 valkey-server 0x0000000104fc9c3c processInputBuffer + 440 16 valkey-server 0x0000000104fc9198 readQueryFromClient + 108 17 valkey-server 0x000000010508073c connSocketEventHandler + 136 18 valkey-server 0x0000000104fa49b4 aeProcessEvents + 316 19 valkey-server 0x0000000104fc081c main + 18408 20 dyld 0x000000019136eb98 start + 6076 ``` The bug seems to be that the result of `sdscatfmt` is being ignored. Since `sdscatfmt` potentially reallocs a new buffer when it needs to, and deallocs the old one, this can cause a use-after-free/double-free if the loop triggers a buffer-size increase. This PR fixes the bug. Signed-off-by: Tony Wooster <twooster@gmail.coM>
Made few changes to support TSAN build in valkey-search repository. Related: valkey-io/valkey-search#703 Signed-off-by: Baswanth Vegunta <baswanth@amazon.com> Co-authored-by: Baswanth Vegunta <baswanth@amazon.com>
`zdiffAlgorithm2()` can break out early once the destination cardinality reaches zero. In that path, a temporary SDS created by `zuiSdsFromValue()` was left dirty and never released, because that cleanup normally happens on the next iterator step. This patch explicitly discards the dirty value before the early `break`. ``` ==11724== 11 bytes in 1 blocks are definitely lost in loss record 36 of 1,442 ==11724== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==11724== by 0x2FA974: ztrymalloc_usable_internal (zmalloc.c:156) ==11724== by 0x2FAAEA: zmalloc_usable (zmalloc.c:200) ==11724== by 0x282D25: _sdsnewlen (sds.c:102) ==11724== by 0x2830B2: sdsnewlen (sds.c:169) ==11724== by 0x2DB537: zuiSdsFromValue (t_zset.c:2290) ==11724== by 0x2DBE00: zdiffAlgorithm2 (t_zset.c:2502) ==11724== by 0x2DC085: zdiff (t_zset.c:2568) ==11724== by 0x2DD01F: zunionInterDiffGenericCommand (t_zset.c:2817) ==11724== by 0x2DD574: zdiffCommand (t_zset.c:2898) ==11724== by 0x29F5AE: call (server.c:3883) ==11724== by 0x2A1598: processCommand (server.c:4569) ``` Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> Signed-off-by: Sarthak Aggarwal <sarthakaggarwal97@gmail.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
The multiStateMemOverhead() function was incorrectly calculating the memory overhead for watched keys. It used sizeof(c->mstate->watched_keys) which is the size of the list structure itself, instead of sizeof(watchedKey) which is the actual per-key overhead. This was introduced in #1405. Signed-off-by: Binbin <binloveplay1314@qq.com>
The `valkey-cli --cluster del-node` command fails when attempting to delete unreachable or failed nodes, reporting `No such node ID` even though the node exists in the cluster topology. The root cause is the command only loads information about reachable nodes, causing the lookup to fail. This PR added a new function for loading all nodes information to solve this. 1. Loading all nodes from gossip: - Added `clusterManagerLoadAllInfoFromNode()` that loads both reachable and unreachable nodes from cluster gossip - Extracts common logic into `clusterManagerLoadInfoCommon()` with an `include_unreachable` flag - Keeps the original `clusterManagerLoadInfoFromNode()` unchanged to avoid affecting existing callers 2. Added success message to be consistent with other cluster commands: `[OK] Node <id> removed from the cluster.` 3. Added test coverage for `del-node` which previously had none. 4. Load slot information from gossip for unreachable nodes in `clusterManagerNodeLoadInfo()` 5. Skip unreachable primaries in `clusterManagerNodeWithLeastReplicas()` ``` ./runtest --single unit/cluster/cli [ok]: del-node: Cannot delete node with slots (9 ms) [ok]: del-node: Delete reachable node without slots (23 ms) [ok]: del-node: Delete unreachable node without slots (1333 ms) [ok]: del-node: Cannot delete unreachable primary with slots (3368 ms) ``` ``` valkey-cli --cluster del-node 127.0.0.1:7000 eb837ea7c48908e5304eafd8b1b3ced57147c448 Could not connect to Valkey at 127.0.0.1:7002: Connection refused >>> Removing node eb837ea7c48908e5304eafd8b1b3ced57147c448 from cluster 127.0.0.1:7000 >>> Sending CLUSTER FORGET messages to the cluster... >>> WARNING: Could not connect to node 127.0.0.1:7002, unable to send CLUSTER RESET. [OK] Node eb837ea7c48908e5304eafd8b1b3ced57147c448 removed from the cluster. ``` Before ``` $ valkey-cli --cluster del-node <entry-node-ip>:<entry-node-port> <failed-node-id> Could not connect to Valkey at <target-node-ip>:<target-node-port>: Connection refused >>> Removing node <id> from cluster <entry-node-ip>:<entry-node-port> [ERR] No such node ID <id> ``` After ``` $ valkey-cli --cluster del-node <entry-node-ip>:<entry-node-port> <failed-node-id> Could not connect to Valkey at <target-node-ip>:<target-node-port>: Connection refused >>> Removing node <id> from cluster <entry-node-ip>:<entry-node-port> >>> Sending CLUSTER FORGET messages to the cluster... >>> WARNING: Could not connect to node <target-node-ip>:<target-node-port>, unable to send CLUSTER RESET. [OK] Node <id> removed from the cluster. ``` Fixes #3208 --------- Signed-off-by: Yang Zhao <zymy701@gmail.com>
…ty primary (#2811) ## Summary This PR handles a network race condition that would cause a replica to read stale cluster packet and then incorrectly promote itself to an empty primary within an existing shard. ## Issue "Migrated replica reports zero repl offset and rank, and fails to win election - sigstop" the test case in `replica-migration.tcl` is a known example where the network race condition can occur. Here's the timeline: - T1: Slot migration — R7 and R3 both try to replicate from R0. Only R3 succeeds, triggering a BGSAVE on R0, while R7 blocks in `receiveSynchronousResponse()`. - T2: While R7 is blocked, R0 is SIGSTOP'd by the test. R4 wins the election and becomes the new primary. R4 sends PINGs to R7 - These PINGs land in R7's kernel TCP receive buffer but are not read by R7 yet. - T3 (5s after T1, due to receiveSynchronousResponse): R7 wakes up: - It reads from inbound links and finds out the remote nodes have already closed their end (they detected R7 as FAIL), getting "I/O error: connection closed" on each. R7 calls `accept()` for the new connections that were established during the block. These connections carry data that was sent seconds ago while R7 was dead. - R7's outbound links are still valid, so it sends PING to R4. - T4: R7 receives and processes fresh PONG packet from R4 on outbound link, reconfiguring itself to follow R4. - T5: R7 reads stale PING packet of R4 via inbound link. This packet was generated R4 was following R0, so R7 incorrectly believes R4 is still following R0 now. And Reconfiguring itself as a replica of R0 from R4. - T6: R7 finds R0 is FAIL, so it starts an election and wins, and becomes an empty-primary. ## Analyze So in T4, R4 is the new primary, and R7 is reconfiguring itself as a replica. So in R7's view, R4 is the primary, and myself (R7) is a replica. And in T5, there is a stale packet from sender (R4), the stale packet is saying: sender (R4) is a replica and R0 is the primary. We originally had a logic for stale packet, meaning we would try to ignore stale packet that would cause exceptions. So in T5: - sender_claims_to_be_primary is false since R4 is saying it is a replica. - sender_last_reported_as_primary is true since in R7's view, R4 is a primary. - sender_claimed_primary is R0, and sender (R4) and sender_claimed_primary (R0) is in the same shard. - nodeEpoch(sender_claimed_primary) is R0's epoch. R0 is an old and dead (not yet) primary. - sender_claimed_config_epoch is R0's epoch since R4 is a replica, and R0 is R4's primary. - nodeEpoch(sender_claimed_primary) == sender_claimed_config_epoch, so the logic fail and we process a stale packet. So in this point, the packet is not a stale packet in R4 and R0's view. - But it is a stale packet in myself (R7) view. In R7's local view, R4 is the new primary and it should have a bigger epoch, that is nodeEpoch(sender) should > sender_claimed_config_epoch. ## Fix The PR fixes the issue by enhancing the existing guardrail logic against stale packet. Previously that logic only detects `nodeEpoch(sender_claimed_primary) > sender_claimed_config_epoch` as stale packet, now it also checks `nodeEpoch(sender) > sender_claimed_config_epoch` to make sure we have up-to-date primary-replica chain. Signed-off-by: Zhijun <dszhijun@gmail.com> Co-authored-by: Binbin <binloveplay1314@qq.com>
Some test cases write thoughsands of commands in a pipeline and afterwards read the replies. This can lead to TCP ACK being dropped and the connection broken. CLIENT REPLY OFF prevents this. "Main db not affected when fail to diskless load" in cluster/diskless-load-swapdb has been observed to be flaky. The others are just defensive but they follow the same pattern. Similar fixes in the past: #3430, #2483. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
In valkey.conf, slot-migration-max-failover-repl-bytes allows setting to -1 to disable the limit. ``` Setting this to -1 will disable this limit ``` But slot-migration-max-failover-repl-bytes is defined as MEMORY_CONFIG and memtoull() rejects negative inputs, making it impossible to set the value to -1 via config file or CONFIG SET. ``` >>> 'slot-migration-max-failover-repl-bytes "-1"' argument must be a memory value ``` Introduce SIGNED_MEMORY_CONFIG flag for memory configs that also accept plain negative number. When memtoull() fails and this flag is set, fall back to string2ll() for parsing. Use ll2string() for CONFIG GET and rewriteConfigNumericalOption() for CONFIG REWRITE when the value is negative. Add a serverAssert in initConfigValues() to enforce that PERCENT_CONFIG and SIGNED_MEMORY_CONFIG are never combined on the same config, since both use negative values with different semantics. This means we have had this issue since it was introduced in #1949. Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Increase time to wait for bgsave to start. This wait has been seen
failing in these test cases.
The test case loops with 'no', 'slow', 'fast', 'all' and 'timeout'
replicas, in tests/integration/replication.tcl
Example:
*** [err]: diskless fast replicas drop during rdb pipe in
tests/integration/replication.tcl
rdb child didn't terminate
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
…igs (#3440) rewriteConfigFormatMemory() and rewriteConfigBytesOption() used long long parameters to represent memory bytes, but configs like maxmemory are stored as unsigned long long and allow values up to ULLONG_MAX. When the value exceeds LLONG_MAX (e.g. 9223372036854775808), it overflows to negative when passed as long long, causing config rewrite to produce incorrect output like "maxmemory -8589934592gb". Change both functions to use unsigned long long, matching the actual semantics of memory byte values. This is consistent with how numericConfigGet() already handles MEMORY_CONFIG using ull2string(). There are some MEMORY_CONFIG are signed and can be negative: 1. maxmemory-clients is a PERCENT_CONFIG 2. slot-migration-max-failover-repl-bytes is a SIGNED_MEMORY_CONFIG (after #3443) None of them were affected: ``` static void numericConfigRewrite(standardConfig *config, const char *name, struct rewriteConfigState *state) { ... if (config->data.numeric.flags & PERCENT_CONFIG && value < 0) { rewriteConfigPercentOption(state, name, -value, config->data.numeric.default_value); } else if (config->data.numeric.flags & SIGNED_MEMORY_CONFIG && value < 0) { rewriteConfigNumericalOption(state, name, value, config->data.numeric.default_value); } else if (config->data.numeric.flags & MEMORY_CONFIG) { rewriteConfigBytesOption(state, name, value, config->data.numeric.default_value); ... ``` Signed-off-by: Binbin <binloveplay1314@qq.com>
`hpersistCommand` calls `addReplyArrayLen` before `lookupKeyWrite` + `checkType`. When HPERSIST targets a non-hash key, the server writes a RESP array header followed by a WRONGTYPE error — a malformed response that permanently desynchronizes the client connection. This moves `lookupKeyWrite` + `checkType` before `addReplyArrayLen`, matching the pattern used by every other HFE command (e.g. `hgetdelCommand`, `hexpireGenericCommand`). Added a test for HPERSIST on a wrong-type key. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
This deflakes all variants of `diskless replicas drop during rdb pipe`. The main issue turned out to be that the test was too sensitive to timing and log ordering under TLS, not that the core behavior was wrong. This keeps the same five subcases (no, slow, fast, all, timeout) but makes them much less CI-fragile. CI passes 200 times: https://github.com/sarthakaggarwal97/valkey/actions/runs/24547258515 --------- Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> Signed-off-by: Sarthak Aggarwal <25262500+sarthakaggarwal97@users.noreply.github.com> Co-authored-by: Sarthak Aggarwal <25262500+sarthakaggarwal97@users.noreply.github.com>
…led (#3458) When close_asap flag is set, set bytes read to 0 In the readToQueryBuf, the c->nread represents the number of bytes read. When close_asap flag is set, there is a bug where the c->nread isn't reset to 0 and this breaks the invariant. IOThreads then incorrectly think there is data to read and results in a crash. This change fixes this bug. To elaborate on the race possible: 1. Let's say that a IO thread job for reading query from a client got enqueued as part of a epoll - https://github.com/valkey-io/valkey/blob/unstable/src/io_threads.c#L417. 2. Later the client gets freed async and is marked as close_asap - https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L2175 3. While processing the io_thread job for the client, it invokes iothreadReadQueryFromClient. Here, [`readToQueryBuf`](https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L6497) returns as a no-op since the client is marked close-asap. Also, the c->nread is not reset to 0 and count contain the value from a previous read. 4. Later parseInputBuffer [gets invoked](https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L6514). 5. The parseInputBuffer then [accesses the query_buf](https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L3864). The query_buf here would be null in resetSharedQueryBuf as part of beforeNextClient. Signed-off-by: Deepak Nandihalli <deepak.nandihalli@gmail.com>
Since there is some mismatch between the already installed `ar` tool on a macOS runner and Clang 22, installed by brew; lets use the brew installed `llvm-ar`. Expected to fix the issue in CI job `build-macos-latest`. --------- Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>
…3498) There is a double free issue in the code. The error handling path called both decrRefCount(o) and streamFreeNACK(nack), but the nack was obtained from cgroup->pel via raxFind and is still referenced there. decrRefCount(o) frees it through freeStream -> streamFreeCG -> raxFreeWithCallback(cg->pel, zfree), so the explicit streamFreeNACK(nack) causes a double free. Remove the redundant streamFreeNACK(nack) call and add a regression test with a crafted corrupt payload that triggers the duplicate consumer PEL entry path. This was introduced in 492d8d0. Signed-off-by: Binbin <binloveplay1314@qq.com>
valkeyrie-ops
Bot
force-pushed
the
agent/backport/sweep/9.0
branch
from
June 10, 2026 10:37
0ac0c8a to
740404f
Compare
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
## Summary Fix a file descriptor leak in `connSocketBlockingConnect()` when `aeWait()` times out. ## Bug When `anetTcpNonBlockConnect()` succeeds but `aeWait()` times out (e.g., MIGRATE to an unreachable host), the fd is leaked because it was never assigned to `conn->fd`. The caller's `connClose()` checks `conn->fd != -1` and skips cleanup. ## Fix Assign `conn->fd = fd` immediately after `anetTcpNonBlockConnect()` succeeds, before `aeWait()`. This way the caller's normal `connClose()` cleanup path handles the fd on any error, which is consistent with how the rest of the connection lifecycle works. TLS connections also benefit since `connTLSBlockingConnect` delegates to this function for the TCP layer. ## Reproducer ``` valkey-cli SET key hello # Repeat against unreachable host: for i in $(seq 1 30); do valkey-cli MIGRATE 192.0.2.1 6379 key 0 500; done # Check: /proc/<pid>/fd shows 30 leaked socket fds ``` *This issue was generated by AI but verified, with love, by a human.* Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
If not cleared, the job may no longer be valid by the time the client goes to cleanup. This dangling reference could cause a crash if you set slot-migration-log-max-len to 0 and are very unlucky. Signed-off-by: Jacob Murphy <jkmurphy@google.com>
❌ Provenance Check AlertPotential code similarities detected with upstream repository.
This check was performed automatically by the Provenance Guard Action. |
Author
|
I did not push a fix: targeted macOS verification did not pass (failure) Looked at the failure from this run. |
Contributor
test_networking.c:641-682 declare arrays sized by `const int iovmax =
16`, which in C is not a compile-time constant, so clang treats them as
VLAs and fails under -Werror,-Wgnu-folding-constant ('variable length
array folded to constant array as an extension').
Author
|
Fixed compile of src/unit/test_networking.c (test_addBufferToReplyIOV) and pushed Fixing the failure from this run. Root cause: test_networking.c:641-682 declare arrays sized by Verified by: targeted verification on a macOS runner (run) Review: The enum constant is a true compile-time integer constant expression in C, eliminating the VLA folding warning at its source; no assertions were weakened and no test behavior changed since the value (16) and array dimensions are identical. The fix passed targeted verification of the failing check; this PR's full CI will confirm. I do not merge. |
Contributor
In src/unit/test_vset.c, arrays are sized by const local variables (e.g. `const unsigned int total_entries = 1000; mock_entry *entries[total_entries];` at lines 137/181/229/284); clang treats a const-qualified variable as a non-constant expression, folding these into VLAs, which -Werror -Wgnu-folding-constant rejects (4 errors -> make[1] Error 1), while GCC on Linux accepts them.
Author
|
Fixed build-macos-latest / make (all-with-unit-tests) and pushed Fixing the failure from this run. Root cause: In src/unit/test_vset.c, arrays are sized by const local variables (e.g. Verified by: targeted verification on a macOS runner (run) Review: The change replaces const-sized VLAs with zmalloc/zfree heap arrays, directly eliminating the -Wgnu-folding-constant clang errors without altering any test assertions or product behavior; added TEST_ASSERTs and matching zfree calls are correct and complete. The fix passed targeted verification of the failing check; this PR's full CI will confirm. I do not merge. |
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>
…ations (#3261) The "New Master down consecutively" test was sometimes failing under Valgrind by timing out. The new overrides match those used for the cluster in the first part of the file - see #2672 Under Valgrind's 10-20x slowdown, a single failover requiring ~15 seconds of server time can exceed the test's 100-second wall-clock wait. Error text: ``` *** [err]: New Master down consecutively in tests/unit/cluster/slave-selection.tcl No failover detected when master 12 fails ``` Daily run failure: https://github.com/valkey-io/valkey/actions/runs/22421982161/job/64921545936#logs --------- Signed-off-by: Rain Valentine <rsg000@gmail.com> Signed-off-by: Rain Valentine <rainval@amazon.com>
Contributor
Author
|
I did not push a fix: check still failing after fix Looked at the failure from this run. Evidence |
Contributor
Author
|
I did not push a fix: check still failing after fix Looked at the failure from this run. Evidence |
Contributor
Author
|
I did not push a fix: the failure did not reproduce on a clean checkout; it is likely flaky or environment-specific, so refusing rather than pushing a fix Looked at the failure from this run. Evidence |
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>
Carries on from where #3161 left off. The test-sanitizer-address-large-memory jobs were being OOM-killed on GitHub-hosted runners (15.6GB RAM) due to ASAN's 2-3x memory overhead. Changes: - Skip 4GB quicklist compression test under ASAN (requires ~16-24GB with dual buffers + ASAN overhead) - Reduce integration test sizes from 5GB to 4.1GB (preserves >4GB 32-bit boundary coverage) - Reduce XADD iterations from 10 to 3 - Add memory monitoring to track minimum free memory during CI runs Signed-off-by: Rain Valentine <rsg000@gmail.com>
valkeyrie-ops
Bot
added
the
ai-resolved-conflicts
Author
AI conflict resolution: source PR #3161Separate jobs for large memory tests with sanitizers 1 conflicted file Claude Summary
AI-resolved conflicted files
Full backport commit diff: commit 5929614d1567.
|
Author
AI conflict resolution: source PR #3263Fix OOM aborts in large-memory ASAN tests on GitHub runners 3 conflicted files Claude Summary
AI-resolved conflicted files
Full backport commit diff: commit c4b72ac6392d.
|
Contributor
src/unit/test_files.h is a generated file that was not regenerated after test_vset_remove_expired_leaves_ht_bucket_size_one was added to src/unit/test_vset.c; the CI step regenerates it and git diff shows the missing prototype/registration entry (log lines 9522-9539).
Author
|
Fixed validate commands.def up to date and pushed Fixing the failure from this run. Root cause: src/unit/test_files.h is a generated file that was not regenerated after test_vset_remove_expired_leaves_ht_bucket_size_one was added to src/unit/test_vset.c; the CI step regenerates it and git diff shows the missing prototype/registration entry (log lines 9522-9539). Full verification outputVerified by: targeted verification on a Linux runner Review: The fix regenerates test_files.h to register test_vset_remove_expired_leaves_ht_bucket_size_one, which genuinely exists in test_vset.c; the change adds only the prototype and registration entry, directly addressing the stated root cause without weakening any test or touching unrelated code. The fix passed targeted verification of the failing check; this PR's full CI will confirm. I do not merge. |
The test "Blocking keyspace notification with pipelining hset after
hget" was recently failing intermittently with two different errors:
1. `Expected [expr {114 * 10 < 1114}]` - timing assertion failed under Valgrind
2. `Timeout waiting for blocked clients` - race condition on normal runs
The test used wall-clock timing to verify that hget (non-blocking)
completed faster than hset (blocking). This is unreliable because:
- Valgrind slows execution 10-50x, making timing ratios meaningless
- Fast systems may complete both operations in <10ms, causing ratio failures
This fix replaces timing assertions with blocked client count checks,
which directly verify the blocking mechanism rather than inferring it
from timing. The test now confirms hget's response is available before
hset blocks, then waits for the blocked client count to transition
through the expected states.
Signed-off-by: Rain Valentine <rsg000@gmail.com>
Fixes #4103 Issue: In dual-channel replication the RDB is generated and streamed on a dedicated RDB-channel client. That connection's handshake (dualChannelReplHandleHandshake) sent REPLCONF capa … listening-port … but not REPLCONF version. So the primary's RDB-channel client had replica_version = 0, and replicaRdbVersion() fell back to RDB version 11. RDB 11 can't serialize newer encodings like a hash with volatile fields requires RDB_TYPE_HASH_2 (RDB version >= 80). so the save aborted and the full sync failed. Fix: Append version VALKEY_VERSION to the existing REPLCONF on RDB-channel connection. --------- Signed-off-by: Harkrishn Patro <h_patro@apple.com>
Author
AI conflict resolution: source PR #4105Send replica version on dual-channel RDB connection 1 conflicted file Claude Summary
AI-resolved conflicted files
Full backport commit diff: commit 89b06438267d.
|
sarthakaggarwal97
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport sweep for 9.0
Automated cherry-picks from PRs marked "To be backported".
Applied
valkey-cli --cluster del-nodefor unreachable nodesbuild-macos-latestAI resolution details are posted as comments on this PR when available.
Skipped
These candidates were evaluated but contribute no change to this branch, e.g. the fix targets code that does not exist here. No backport commit was created for them.
Needs attention
These candidates could not be applied automatically and need a maintainer to follow up.
3 candidate(s)
Generated by valkey-ci-agent using Claude Code.