Unverified Commit 62c8be28 authored by Binbin's avatar Binbin Committed by GitHub
Browse files

Regression test for sync psync crash (#10288)

Added regression tests for #10020 / #10081 / #10243.
The above PRs fixed some crashes due to an asserting,
see function `clientHasPendingReplies` (introduced in #9166).

This commit added some tests to cover the above scenario.
These tests will all fail in #9166, althought fixed not,
there is value in adding these tests to cover and verify
the changes. And it also can cover #8868 (verify the logs).

Other changes: 
1. Reduces the wait time in `waitForBgsave` and `waitForBgrewriteaof`
from 1s to 50ms, which should reduce the time for some tests.
2. Improve the test infra to print context when `assert_match` fails.
3. Improve the test infra to print `$error` when `assert_error` fails.
```
Expected an error matching 'ERR*' but got 'OK' (context: type eval line 4 cmd {assert_error "ERR*" {r set a b}} proc ::test)
```
parent 2eb9b196
...@@ -1233,3 +1233,89 @@ test {replica can handle EINTR if use diskless load} { ...@@ -1233,3 +1233,89 @@ test {replica can handle EINTR if use diskless load} {
} }
} }
} {} {external:skip} } {} {external:skip}
start_server {tags {"repl" "external:skip"}} {
test "replica do not write the reply to the replication link - SYNC (_addReplyToBufferOrList)" {
set rd [redis_deferring_client]
set lines [count_log_lines 0]
$rd sync
$rd ping
catch {$rd read} e
if {$::verbose} { puts "SYNC _addReplyToBufferOrList: $e" }
assert_equal "PONG" [r ping]
# Check we got the warning logs about the PING command.
verify_log_message 0 "*Replica generated a reply to command 'ping', disconnecting it: *" $lines
$rd close
catch {exec kill -9 [get_child_pid 0]}
waitForBgsave r
}
test "replica do not write the reply to the replication link - SYNC (addReplyDeferredLen)" {
set rd [redis_deferring_client]
set lines [count_log_lines 0]
$rd sync
$rd xinfo help
catch {$rd read} e
if {$::verbose} { puts "SYNC addReplyDeferredLen: $e" }
assert_equal "PONG" [r ping]
# Check we got the warning logs about the XINFO HELP command.
verify_log_message 0 "*Replica generated a reply to command 'xinfo|help', disconnecting it: *" $lines
$rd close
catch {exec kill -9 [get_child_pid 0]}
waitForBgsave r
}
test "replica do not write the reply to the replication link - PSYNC (_addReplyToBufferOrList)" {
set rd [redis_deferring_client]
set lines [count_log_lines 0]
$rd psync replicationid -1
assert_match {FULLRESYNC * 0} [$rd read]
$rd get foo
catch {$rd read} e
if {$::verbose} { puts "PSYNC _addReplyToBufferOrList: $e" }
assert_equal "PONG" [r ping]
# Check we got the warning logs about the GET command.
verify_log_message 0 "*Replica generated a reply to command 'get', disconnecting it: *" $lines
verify_log_message 0 "*== CRITICAL == This master is sending an error to its replica: *" $lines
verify_log_message 0 "*Replica can't interact with the keyspace*" $lines
$rd close
catch {exec kill -9 [get_child_pid 0]}
waitForBgsave r
}
test "replica do not write the reply to the replication link - PSYNC (addReplyDeferredLen)" {
set rd [redis_deferring_client]
set lines [count_log_lines 0]
$rd psync replicationid -1
assert_match {FULLRESYNC * 0} [$rd read]
$rd slowlog get
catch {$rd read} e
if {$::verbose} { puts "PSYNC addReplyDeferredLen: $e" }
assert_equal "PONG" [r ping]
# Check we got the warning logs about the SLOWLOG GET command.
verify_log_message 0 "*Replica generated a reply to command 'slowlog|get', disconnecting it: *" $lines
$rd close
catch {exec kill -9 [get_child_pid 0]}
waitForBgsave r
}
test "PSYNC with wrong offset should throw error" {
# It used to accept the FULL SYNC, but also replied with an error.
assert_error {ERR value is not an integer or out of range} {r psync replicationid offset_str}
set logs [exec tail -n 100 < [srv 0 stdout]]
assert_match {*Replica * asks for synchronization but with a wrong offset} $logs
assert_equal "PONG" [r ping]
}
}
...@@ -24,10 +24,10 @@ proc assert_no_match {pattern value} { ...@@ -24,10 +24,10 @@ proc assert_no_match {pattern value} {
} }
} }
proc assert_match {pattern value} { proc assert_match {pattern value {detail ""}} {
if {![string match $pattern $value]} { if {![string match $pattern $value]} {
set context "(context: [info frame -1])" set context "(context: [info frame -1])"
error "assertion:Expected '$value' to match '$pattern' $context" error "assertion:Expected '$value' to match '$pattern' $context $detail"
} }
} }
...@@ -84,9 +84,9 @@ proc assert_range {value min max {detail ""}} { ...@@ -84,9 +84,9 @@ proc assert_range {value min max {detail ""}} {
proc assert_error {pattern code {detail ""}} { proc assert_error {pattern code {detail ""}} {
if {[catch {uplevel 1 $code} error]} { if {[catch {uplevel 1 $code} error]} {
assert_match $pattern $error assert_match $pattern $error $detail
} else { } else {
assert_failed "assertion:Expected an error but nothing was caught" $detail assert_failed "Expected an error matching '$pattern' but got '$error'" $detail
} }
} }
......
...@@ -89,7 +89,7 @@ proc waitForBgsave r { ...@@ -89,7 +89,7 @@ proc waitForBgsave r {
puts -nonewline "\nWaiting for background save to finish... " puts -nonewline "\nWaiting for background save to finish... "
flush stdout flush stdout
} }
after 1000 after 50
} else { } else {
break break
} }
...@@ -103,7 +103,7 @@ proc waitForBgrewriteaof r { ...@@ -103,7 +103,7 @@ proc waitForBgrewriteaof r {
puts -nonewline "\nWaiting for background AOF rewrite to finish... " puts -nonewline "\nWaiting for background AOF rewrite to finish... "
flush stdout flush stdout
} }
after 1000 after 50
} else { } else {
break break
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment