Conversation
After the master kills the replication link the replica does not report the outcome until it has parsed through the RDB bytes already buffered in its socket receive buffer and reached the EOF. That is 6-25ms on an idle machine but has been measured in seconds on oversubscribed sanitizer and TLS runners, and the ~5s budget had no headroom for it. Raise the try count from 500 to 6000 (~60s). wait_for_log_messages returns on the first match so the poll delay stays at 10ms and healthy iterations are unaffected. Reproduced locally by slowing only the post-kill parse: set key-load-delay on the replica immediately before the kill so the receive buffer is already full at full read speed, with debug populate 100000 to put thousands of opcodes in that window. At key-load-delay 2000 the assertion fails in 4 of 5 loops before this change and passes in 5 of 5 after, with observed kill-to-outcome windows up to 20.4s. 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
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.
diskless loading short readgives the replica ~5s to log the outcome after the master kills the replication link, but the replica cannot report anything until it has parsed through the RDB bytes already sitting in its socket receive buffer and reached the EOF. That takes 6-25ms on an idle machine and seconds on an oversubscribed sanitizer or TLS runner, so the budget has no headroom and the test flakes. This raises the try count from 500 to 6000 (~60s) at the same 10ms poll delay. Tracked upstream as valkey-io/valkey#4325.Details
Problem
tests/integration/replication.tcl:826onupstream/unstable(d6415e766):tests/support/util.tcl:285-307makes that 500 tries at 10ms, so ~5s.The wait is not waiting for a network event.
rioConnEnsureBufferedturnsconnRead() == 0into immediate failure (src/rio.c:261-268,src/connection.h:350-352) andrdbReportErrorlogsInternal error in RDB(src/rdb.c:122-160), so the only thing standing between the kill and the log line is replica-side RDB parse throughput over the already-buffered bytes. The budget is a throughput bet on the runner.The 5s came verbatim from Redis
03406fcb6(redis#9763, 2021), before sanitizer jobs existed and before this job started passing--accurate(500 iterations instead of 100,tests/integration/replication.tcl:945-947).Reproduction
The knob is
key-load-delay(src/config.c:3582, applied per top-level RDB opcode atsrc/rdb.c:3949). Two things have to be right or it does nothing:key-load-delayof 20000, 100000 and 200000 from test start all still passed (14s / 37s / 177s runtime, assertion never tripped). The CI shape is a replica that read at full speed, filled a large autotuned rcvbuf, and then lost its CPU.debug populate 100000puts thousands of opcodes in that window.Also set
loading-process-events-interval-bytes 16384on the replica (src/config.c:3657, default 2MB) so the mid-loadCONFIG SETlands promptly; during loading the replica only processes events at those boundaries (src/rdb.c:3374).Copy the test to
tests/integration/repro-issue-44.tclwithattempts5 and:Measured kill-to-outcome window:
key-load-delay(us, set at kill)Testing
Before, budget
500 10:After, budget
6000 10:20.4s observed under the artificial slowdown, against 60s of budget and the 6.6s the UBSan runner needed.
--only "diskless loading short read"cannot isolate this test: laterwait_for_synccalls in the file sit outside anytestbody and depend on areplicaofthat a skipped test performs, so the run aborts withreplica didn't sync in timeattests/integration/replication.tcl:250. The 5-loop evidence above therefore comes from the extracted copy, which carries the identical assertion. The full file was run separately with the fix.Not verified against a real UBSan build; the slowdown above stands in for the runner.
Decisions
--accurateiterations even when nothing is wrong. Raising the count costs nothing on a healthy run becausewait_for_log_messagesreturns on first match.connReadreturns 0.66f9618f4(#3853) added two abort paths that emit none of the three grepped-for strings,Failed to initialize RDB stream reader from primary(src/replication.c:2782-2785) andCompressed RDB stream from primary was truncated; will resync(src/replication.c:2812). The test does not enablerepl-compression, so this is latent rather than the observed cause, and it wants its own change.Fixes #44
This was generated by AI but verified, with love, by a human.