Commit d80c8711 authored by Oran Agra's avatar Oran Agra
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.

(cherry picked from commit 9b564b52)
parent 981953a6
...@@ -275,7 +275,7 @@ void trackingRememberKeyToBroadcast(client *c, char *keyname, size_t keylen) { ...@@ -275,7 +275,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);
} }
......
...@@ -108,6 +108,22 @@ start_server {tags {"tracking"}} { ...@@ -108,6 +108,22 @@ start_server {tags {"tracking"}} {
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 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 [$rd1 read] 2]]
assert {$keys eq {mykey{t}}}
} {}
test {Tracking gets notification on tracking table key eviction} { test {Tracking gets notification on tracking table key eviction} {
r CLIENT TRACKING off r CLIENT TRACKING off
r CLIENT TRACKING on REDIRECT $redir NOLOOP r CLIENT TRACKING on REDIRECT $redir NOLOOP
......
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