prevent client tracking from causing feedback loop in performEvictions (#8100)

When client tracking is enabled signalModifiedKey can increase memory usage,
this can cause the loop in performEvictions to keep running since it was measuring
the memory usage impact of signalModifiedKey.

The section that measures the memory impact of the eviction should be just on dbDelete,
excluding keyspace notification, client tracking, and propagation to AOF and replicas.

This resolves part of the problem described in #8069
p.s. fix took 1 minute, test took about 3 hours to write.
This commit is contained in:
Oran Agra 2020-12-06 14:51:22 +02:00 committed by GitHub
parent 1df5bb5687
commit c4fdf09c05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 55 additions and 1 deletions

View File

@ -629,6 +629,8 @@ int performEvictions(void) {
* we are freeing removing the key, but we can't account for
* that otherwise we would never exit the loop.
*
* Same for CSC invalidation messages generated by signalModifiedKey.
*
* AOF and Output buffer memory will be freed eventually so
* we only care about memory used by the key space. */
delta = (long long) zmalloc_used_memory();
@ -637,12 +639,12 @@ int performEvictions(void) {
dbAsyncDelete(db,keyobj);
else
dbSyncDelete(db,keyobj);
signalModifiedKey(NULL,db,keyobj);
latencyEndMonitor(eviction_latency);
latencyAddSampleIfNeeded("eviction-del",eviction_latency);
delta -= (long long) zmalloc_used_memory();
mem_freed += delta;
server.stat_evictedkeys++;
signalModifiedKey(NULL,db,keyobj);
notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted",
keyobj, db->id);
decrRefCount(keyobj);

View File

@ -260,3 +260,55 @@ start_server {tags {"maxmemory"}} {
r dbsize
} {4098}
}
start_server {tags {"maxmemory"}} {
test {client tracking don't cause eviction feedback loop} {
r config set maxmemory 0
r config set maxmemory-policy allkeys-lru
r config set maxmemory-eviction-tenacity 100
# 10 clients listening on tracking messages
set clients {}
for {set j 0} {$j < 10} {incr j} {
lappend clients [redis_deferring_client]
}
foreach rd $clients {
$rd HELLO 3
$rd read ; # Consume the HELLO reply
$rd CLIENT TRACKING on
$rd read ; # Consume the CLIENT reply
}
# populate 300 keys, with long key name and short value
for {set j 0} {$j < 300} {incr j} {
set key $j[string repeat x 1000]
r set $key x
# for each key, enable caching for this key
foreach rd $clients {
$rd get $key
$rd read
}
}
# set the memory limit which will cause a few keys to be evicted
# we need to make sure to evict keynames of a total size of more than
# 16kb since the (PROTO_REPLY_CHUNK_BYTES), only after that the
# invalidation messages have a chance to trigger further eviction.
set used [s used_memory]
set limit [expr {$used - 40000}]
r config set maxmemory $limit
# make sure some eviction happened
set evicted [s evicted_keys]
if {$::verbose} { puts "evicted: $evicted" }
assert_range $evicted 10 50
foreach rd $clients {
$rd read ;# make sure we have some invalidation message waiting
$rd close
}
# make sure we didn't drain the database
assert_range [r dbsize] 200 300
}
}