Skip to content

Commit

Permalink
Fix timing issue in the new tot-net-out replica test (#1060)
Browse files Browse the repository at this point in the history
Apparently there is a timing issue when using wait_for_ofs_sync:
```
[exception]: Executing test client: can't read "out_before": no such variable.
can't read "out_before": no such variable
```

The reason is that if the connection between the primary
and the replica is not established yet, the master_repl_offset
of the primary and replica in wait_for_ofs_sync is 0, and
the check fails, resulting in no replica client in the
client list below.

In this case, we need to make sure the replica is online
before proceeding.

Signed-off-by: Binbin <binloveplay1314@qq.com>
  • Loading branch information
enjoy-binbin authored Sep 20, 2024
1 parent 7fab157 commit d9c41e9
Showing 1 changed file with 28 additions and 32 deletions.
60 changes: 28 additions & 32 deletions tests/unit/introspection.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ start_server {tags {"introspection"}} {
set kv [split $item "="]
set k [lindex $kv 0]
if {[string match $field $k]} {
return [lindex $kv 1]
return [lindex $kv 1]
}
}
return ""
Expand Down Expand Up @@ -332,7 +332,7 @@ start_server {tags {"introspection"}} {
r migrate [srv 0 host] [srv 0 port] key 9 5000 AUTH2 user password
catch {r auth not-real} _
catch {r auth not-real not-a-password} _

assert_match {*"key"*"9"*"5000"*} [$rd read]
assert_match {*"key"*"9"*"5000"*"(redacted)"*} [$rd read]
assert_match {*"key"*"9"*"5000"*"(redacted)"*"(redacted)"*} [$rd read]
Expand Down Expand Up @@ -375,46 +375,45 @@ start_server {tags {"introspection"}} {

$rd close
}

test {MONITOR log blocked command only once} {

# need to reconnect in order to reset the clients state
reconnect

set rd [valkey_deferring_client]
set bc [valkey_deferring_client]
r del mylist

$rd monitor
$rd read ; # Discard the OK

$bc blpop mylist 0
wait_for_blocked_clients_count 1
r lpush mylist 1
wait_for_blocked_clients_count 0
r lpush mylist 2

# we expect to see the blpop on the monitor first
assert_match {*"blpop"*"mylist"*"0"*} [$rd read]

# we scan out all the info commands on the monitor
set monitor_output [$rd read]
while { [string match {*"info"*} $monitor_output] } {
set monitor_output [$rd read]
}

# we expect to locate the lpush right when the client was unblocked
assert_match {*"lpush"*"mylist"*"1"*} $monitor_output

# we scan out all the info commands
set monitor_output [$rd read]
while { [string match {*"info"*} $monitor_output] } {
set monitor_output [$rd read]
}

# we expect to see the next lpush and not duplicate blpop command
assert_match {*"lpush"*"mylist"*"2"*} $monitor_output

$rd close
$bc close
}
Expand Down Expand Up @@ -655,7 +654,7 @@ start_server {tags {"introspection"}} {
assert_equal [r config get save] {save {}}
}
} {} {external:skip}

test {CONFIG SET with multiple args} {
set some_configs {maxmemory 10000001 repl-backlog-size 10000002 save {3000 5}}

Expand All @@ -677,7 +676,7 @@ start_server {tags {"introspection"}} {

test {CONFIG SET rollback on set error} {
# This test passes an invalid percent value to maxmemory-clients which should cause an
# input verification failure during the "set" phase before trying to apply the
# input verification failure during the "set" phase before trying to apply the
# configuration. We want to make sure the correct failure happens and everything
# is rolled back.
# backup maxmemory config
Expand All @@ -700,7 +699,7 @@ start_server {tags {"introspection"}} {

test {CONFIG SET rollback on apply error} {
# This test tries to configure a used port number in the server. This is expected
# to pass the `CONFIG SET` validity checking implementation but fail on
# to pass the `CONFIG SET` validity checking implementation but fail on
# actual "apply" of the setting. This will validate that after an "apply"
# failure we rollback to the previous values.
proc dummy_accept {chan addr port} {}
Expand Down Expand Up @@ -733,7 +732,7 @@ start_server {tags {"introspection"}} {
dict set some_configs port $used_port

# Run a dummy server on used_port so we know we can't configure the server to
# use it. It's ok for this to fail because that means used_port is invalid
# use it. It's ok for this to fail because that means used_port is invalid
# anyway
catch {socket -server dummy_accept -myaddr 127.0.0.1 $used_port} e
if {$::verbose} { puts "dummy_accept: $e" }
Expand Down Expand Up @@ -779,18 +778,18 @@ start_server {tags {"introspection"}} {

test {CONFIG GET multiple args} {
set res [r config get maxmemory maxmemory* bind *of]

# Verify there are no duplicates in the result
assert_equal [expr [llength [dict keys $res]]*2] [llength $res]

# Verify we got both name and alias in result
assert {[dict exists $res slaveof] && [dict exists $res replicaof]}
assert {[dict exists $res slaveof] && [dict exists $res replicaof]}

# Verify pattern found multiple maxmemory* configs
assert {[dict exists $res maxmemory] && [dict exists $res maxmemory-samples] && [dict exists $res maxmemory-clients]}
assert {[dict exists $res maxmemory] && [dict exists $res maxmemory-samples] && [dict exists $res maxmemory-clients]}

# Verify we also got the explicit config
assert {[dict exists $res bind]}
assert {[dict exists $res bind]}
}

test {valkey-server command line arguments - error cases} {
Expand Down Expand Up @@ -845,22 +844,19 @@ start_server {tags {"introspection"}} {
set primary_pid [srv -1 pid]
set replica [srv 0 client]
set replica_pid [srv 0 pid]

$replica replicaof $primary_host $primary_port

# Wait for replica to be connected before proceeding.
wait_for_ofs_sync $primary $replica
wait_replica_online $primary

# Avoid PINGs to make sure tot-net-out is stable.
$primary config set repl-ping-replica-period 3600

# Increase repl timeout to avoid replica disconnecting
$primary config set repl-timeout 3600
$replica config set repl-timeout 3600

# Wait for the replica to receive the command.
wait_for_ofs_sync $primary $replica


# Get the tot-net-out of the replica before sending the command.
set info_list [$primary client list]
foreach info [split $info_list "\r\n"] {
Expand All @@ -869,11 +865,11 @@ start_server {tags {"introspection"}} {
break
}
}

# Send a command to the primary.
set value_size 10000
$primary set foo [string repeat "a" $value_size]

# Get the tot-net-out of the replica after sending the command.
set info_list [$primary client list]
foreach info [split $info_list "\r\n"] {
Expand Down

0 comments on commit d9c41e9

Please sign in to comment.