Unverified Commit 9b564b52 authored by Oran Agra's avatar Oran Agra Committed by GitHub
Browse files

Fix race in client side tracking (#9116)

The `Tracking gets notification of expired keys` test in tracking.tcl
used to hung in valgrind CI quite a lot.

It turns out the reason is that with valgrind and a busy machine, the
server cron active expire cycle could easily run in the same event loop
as the command that created `mykey`, so that when they key got expired,
there were two change events to broadcast, one that set the key and one
that expired it, but since we used raxTryInsert, the client that was
associated with the "last" change was the one that created the key, so
the NOLOOP filtered that event.

This commit adds a test that reproduces the problem by using lazy expire
in a multi-exec which makes sure the key expires in the same event loop
as the one that added it.
parent f004073b
...@@ -326,7 +326,7 @@ void trackingRememberKeyToBroadcast(client *c, char *keyname, size_t keylen) { ...@@ -326,7 +326,7 @@ void trackingRememberKeyToBroadcast(client *c, char *keyname, size_t keylen) {
* tree. This way we know who was the client that did the last * tree. This way we know who was the client that did the last
* change to the key, and can avoid sending the notification in the * change to the key, and can avoid sending the notification in the
* case the client is in NOLOOP mode. */ * case the client is in NOLOOP mode. */
raxTryInsert(bs->keys,(unsigned char*)keyname,keylen,c,NULL); raxInsert(bs->keys,(unsigned char*)keyname,keylen,c,NULL);
} }
raxStop(&ri); raxStop(&ri);
} }
......
...@@ -132,6 +132,22 @@ start_server {tags {"tracking network"}} { ...@@ -132,6 +132,22 @@ start_server {tags {"tracking network"}} {
assert {$keys eq {mykey}} assert {$keys eq {mykey}}
} }
test {Tracking gets notification of lazy expired keys} {
r CLIENT TRACKING off
r CLIENT TRACKING on BCAST REDIRECT $redir_id NOLOOP
# Use multi-exec to expose a race where the key gets an two invalidations
# in the same event loop, once by the client so filtered by NOLOOP, and
# the second one by the lazy expire
r MULTI
r SET mykey{t} myval px 1
r SET mykeyotherkey{t} myval ; # We should not get it
r DEBUG SLEEP 0.1
r GET mykey{t}
r EXEC
set keys [lsort [lindex [$rd_redirection read] 2]]
assert {$keys eq {mykey{t}}}
} {} {needs:debug}
test {HELLO 3 reply is correct} { test {HELLO 3 reply is correct} {
set reply [r HELLO 3] set reply [r HELLO 3]
assert_equal [dict get $reply proto] 3 assert_equal [dict get $reply proto] 3
......
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