mirrors/scylladb
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

querier_cache: unregister querier from reader_concurrency_semaphore during eviction

Browse Source 
In insert_querier(), we may evict older queriers to make room for the new one.
However, we forgot to unregister the evicted queriers from
reader_concurrency_semaphore. As a result, when reader_concurrency_semaphore
eventually wanted to evict something, it saw an inactive_read_handle that was
not connected to a querier_cache::entry, and crashed on use-after-free.

Fix by evicting through the inactive_read_handle associated with the querier
to be evicted. This removes traces of the querier from both
reader_concurrency_semaphore and querier_cache. We also have to massage the
statistics since querier_inactive_read::evict() updates different counters.

Fixes #4018.

Tests: unit(release)
Reviewed-by: Botond Denes <bdenes@scylladb.com>
Message-Id: <20190102175023.26093-1-avi@scylladb.com>
This commit is contained in:
Avi Kivity
2019-01-02 19:50:23 +02:00
committed by Pekka Enberg
parent 2717bdd301
commit 918d255168
1 changed files with 8 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
querier.cc
View File

@@ -273,13 +273,15 @@ static void insert_querier(
memory_usage += q.memory_usage();
if (memory_usage >= max_queriers_memory_usage) {
auto it = entries.begin();
const auto end = entries.end();
while (it != end && memory_usage >= max_queriers_memory_usage) {
++stats.memory_based_evictions;
while (!entries.empty() && memory_usage >= max_queriers_memory_usage) {
auto it = entries.begin();
memory_usage -= it->memory_usage();
--stats.population;
it = entries.erase(it);
auto ir = sem.unregister_inactive_read(it->get_inactive_handle());
ir->evict();
// querier_inactive_read::evict() updates resource_based_evictions,
// (and population) while we want memory_based_evictions:
--stats.resource_based_evictions;
++stats.memory_based_evictions;
}
}