Conversation
The test freezes the socket-target BGSAVE with rdb-key-save-delay 2000000 so the replica stays in REPL_STATE_TRANSFER while 50MB is pushed through the main channel. rioConnset buffers the child's output, so the replica's RDB socket receives nothing and server.repl_transfer_lastio never advances (src/replication.c:3248). The default 60s repl-timeout then aborts the sync (src/replication.c:5374-5379), cancelReplicationHandshake frees the pending buffer being measured (src/replication.c:3070), and the reconnect is parked behind the still-running BGSAVE (src/replication.c:1286), so mem_total_replication_buffers can never reach the 40MB threshold. That gives the test a hard repl-timeout-second wall clock budget for the 50MB write loop. On the TLS jobs a Tcl client pushing 1MB values manages about 180KB/s, so the loop needs minutes and the test fails. Set repl-timeout to 3600 on both servers. The stall is injected by the test, so the timeout should not fire at all. Fixes valkey-io/valkey#3923 Signed-off-by: Madelyn Olson <matolson@amazon.com>
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
1 participant
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.
Dual channel replication buffer memory fieldsfreezes the primary's socket-target BGSAVE withrdb-key-save-delay 2000000so the replica stays inREPL_STATE_TRANSFERwhile the test pushes 50MB down the main channel, butrioConnsetbuffers the child's output so the replica's RDB socket reads nothing andrepl_transfer_lastionever advances. The default 60srepl-timeouttherefore aborts the sync,cancelReplicationHandshakefrees the pending buffer the test is measuring, and the reconnect is parked behind the still-running BGSAVE, somem_total_replication_bufferscan never reach the 40MB the wait loop needs. That gives the test a hard 60s wall-clock budget for the 50MB write loop, and on the TLS jobs a Tcl client pushing 1MB values only manages about 180KB/s. This setsrepl-timeout 3600on both servers, since the stall is injected by the test and the timeout should not fire at all.Details
Problem
Upstream record: valkey-io/valkey#3923 (open, 5 occurrences, all
*-tls-modulejobs). Latest:test-almalinux9-tls-modulerun 34727724387.Chain, at
agents/unstable:tests/integration/dual-channel-replication.tcl:1542setsrdb-key-save-delay 2000000over 1000 keys, so the BGSAVE child emits one 20 byte key every 2s.rioConnsetbuffers, so the replica's RDB socket gets nothing after the$EOF:preamble.server.repl_transfer_lastioonly advances on a real RDB-channel read (src/replication.c:3248), so it stays pinned at sync start.src/replication.c:5374-5379trips the bulk-transfer timeout.repl-timeoutdefaults to 60 (src/config.c:3500) and this test never overrides it, unlike 20 otherrepl-timeoutcalls in the same file.cancelReplicationHandshakecallsfreePendingReplDataBuf(src/replication.c:3070), which zeroesserver.pending_repl_data.mem(src/replication.c:3022). That field is exactly what backsmem_total_replication_buffersandmem_replicas_repl_buffer(src/server.c:6418-6419).src/replication.c:1286"Current BGSAVE has socket target. Waiting for next BGSAVE for SYNC", so no new stream arrives and the buffer cannot refill. Unrecoverable once the timeout fires.The failing CI run's main-channel client shows
tot-net-out=11264508andomem=20504at disconnect: 11MB of the intended 51MB in 61s, with no write backlog on the primary. The bottleneck is the Tcl TLS client, about 180KB/s.Reproduction
The knob is
repl-timeout. The test's budget isrepl-timeoutseconds; squeeze it below the write loop's duration and the CI failure appears verbatim. No test edit needed,--configsupplies it, and the target test is extracted to its own file because--onlycannot skip the out-of-testwait_process_pausedscaffolding at line 832.Before, 5/5 fail:
Server logs match the CI timeline exactly. Primary:
Replica:
After, with
--config repl-timeout 1still applied so the test's ownconfig setis what wins, 5/5 pass:TLS could not be exercised locally,
package require tlsis unavailable on this host, so the throughput half of the diagnosis rests on the CItot-net-outfigure rather than a local measurement.Why not the alternative fixes
valkey-io/valkey#4309 replaces the fixed 50 writes with a loop that sends until the replica buffer reaches the threshold. It does not fix this failure. The constraint is wall clock, not write count: at 180KB/s the 40MB target is unreachable inside 60s regardless of loop shape, and once the timeout fires the buffer is zeroed with no path to refill.
Scaling the 40MB and 10MB thresholds down would also work, but it weakens what the test checks and still leaves a wall-clock budget that a slow enough runner can blow.
Testing
The test suite has no
repl-timeoutstate to leak here, bothconfig setcalls are scoped to servers this test starts.This was generated by AI but verified, with love, by a human.