Unverified Commit 9dc6f93e authored by Binbin's avatar Binbin Committed by GitHub
Browse files

Add command being unblocked cause another command to get unblocked execution order test (#12324)

* Add command being unblocked cause another command to get unblocked execution order test

In #12301, we observed that if the
`while(listLength(server.ready_keys) != 0)`
in handleClientsBlockedOnKeys is changed to
`if(listLength(server.ready_keys) != 0)`,
the order of command execution will change.

It is wrong to change that. It means that if a command
being unblocked causes another command to get unblocked
(like a BLMOVE would do), then the new unblocked command
will wait for later to get processed rather than right away.

It'll not have any real implication if we change that since
we do call handleClientsBlockedOnKeys in beforeSleep again,
and redis will still behave correctly, but we don't change that.

An example:
1. $rd1 blmove src{t} dst{t} left right 0
2. $rd2 blmove dst{t} src{t} right left 0
3. $rd3 set key1{t}, $rd3 lpush src{t}, $rd3 set key2{t} in a pipeline

The correct order would be:
1. set key1{t}
2. lpush src{t}
3. lmove src{t} dst{t} left right
4. lmove dst{t} src{t} right left
5. set key2{t}

The wrong order would be:
1. set key1{t}
2. lpush src{t}
3. lmove src{t} dst{t} left right
4. set key2{t}
5. lmove dst{t} src{t} right left

This PR adds corresponding test to cover it.

* Add comment near while(listLength(server.ready_keys) != 0)
parent cefe4566
......@@ -325,6 +325,9 @@ void handleClientsBlockedOnKeys(void) {
* (i.e. not from call(), module context, etc.) */
serverAssert(server.also_propagate.numops == 0);
/* If a command being unblocked causes another command to get unblocked,
* like a BLMOVE would do, then the new unblocked command will get processed
* right away rather than wait for later. */
while(listLength(server.ready_keys) != 0) {
list *l;
......
......@@ -2312,4 +2312,52 @@ foreach {pop} {BLPOP BLMPOP_RIGHT} {
$rd close
}
test {Command being unblocked cause another command to get unblocked execution order test} {
r del src{t} dst{t} key1{t} key2{t} key3{t}
set repl [attach_to_replication_stream]
set rd1 [redis_deferring_client]
set rd2 [redis_deferring_client]
set rd3 [redis_deferring_client]
$rd1 blmove src{t} dst{t} left right 0
wait_for_blocked_clients_count 1
$rd2 blmove dst{t} src{t} right left 0
wait_for_blocked_clients_count 2
# Create a pipeline of commands that will be processed in one socket read.
# Insert two set commands before and after lpush to observe the execution order.
set buf ""
append buf "set key1{t} value1\r\n"
append buf "lpush src{t} dummy\r\n"
append buf "set key2{t} value2\r\n"
$rd3 write $buf
$rd3 flush
wait_for_blocked_clients_count 0
r set key3{t} value3
# If a command being unblocked causes another command to get unblocked, like a BLMOVE would do,
# then the new unblocked command will get processed right away rather than wait for later.
# If the set command occurs between two lmove commands, the results are not as expected.
assert_replication_stream $repl {
{select *}
{set key1{t} value1}
{lpush src{t} dummy}
{lmove src{t} dst{t} left right}
{lmove dst{t} src{t} right left}
{set key2{t} value2}
{set key3{t} value3}
}
$rd1 close
$rd2 close
$rd3 close
close_replication_stream $repl
} {} {needs:repl}
} ;# stop servers
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