Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

querier_cache: population is mis-accounted #5123

Closed
denesb opened this issue Oct 1, 2019 · 3 comments
Closed

querier_cache: population is mis-accounted #5123

denesb opened this issue Oct 1, 2019 · 3 comments

Comments

@denesb
Copy link
Contributor

@denesb denesb commented Oct 1, 2019

Versions: 3.1, master (c9aae13).

(gdb) p $db->_querier_cache._entries._M_impl._M_node._M_size 
$108 = 83
(gdb) p $db->_querier_cache._stats
$109 = {lookups = 0, misses = 0, drops = 0, time_based_evictions = 8157, resource_based_evictions = 101295, memory_based_evictions = 0, population = 38}

The two should be equal. Also seen population counts that under-flowed.

@avikivity

This comment has been minimized.

Copy link
Contributor

@avikivity avikivity commented Oct 2, 2019

Maybe an exceptional path that caused miss-accounting? Accounting should take place in the same place we add/remove from the cache.

@denesb

This comment has been minimized.

Copy link
Contributor Author

@denesb denesb commented Oct 2, 2019

The problem was found to be entries that are evicted immediately on insert. These were not added to the population but were subtracted upon eviction nevertheless. Patch with the fix is already on the list.

avikivity added a commit that referenced this issue Oct 2, 2019
…population

Currently, the population stat is not increased for entries that are
evicted immediately on insert, however the code that does the eviction
still decreases the population stat, leading to an imbalance and in some
cases the underflow of the population stat. To fix, unconditionally
increase the population stat upon inserting an entry, regardless of
whether it is immediately evicted or not.

Fixes: #5123

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20191001153215.82997-1-bdenes@scylladb.com>
avikivity added a commit that referenced this issue Oct 5, 2019
…population

Currently, the population stat is not increased for entries that are
evicted immediately on insert, however the code that does the eviction
still decreases the population stat, leading to an imbalance and in some
cases the underflow of the population stat. To fix, unconditionally
increase the population stat upon inserting an entry, regardless of
whether it is immediately evicted or not.

Fixes: #5123

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20191001153215.82997-1-bdenes@scylladb.com>
(cherry picked from commit 00b432b)
avikivity added a commit that referenced this issue Oct 5, 2019
…population

Currently, the population stat is not increased for entries that are
evicted immediately on insert, however the code that does the eviction
still decreases the population stat, leading to an imbalance and in some
cases the underflow of the population stat. To fix, unconditionally
increase the population stat upon inserting an entry, regardless of
whether it is immediately evicted or not.

Fixes: #5123

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20191001153215.82997-1-bdenes@scylladb.com>
(cherry picked from commit 00b432b)
@avikivity

This comment has been minimized.

Copy link
Contributor

@avikivity avikivity commented Nov 17, 2019

Already backported, removing backport candidate label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.