Rework issuable state caching
@smcgivern and I had a long talk today about caching the issuable state counters at the top of the issue/MR index pages, and the caches for the sidebar. We also talked about reusing these counters for the Kaminari pagination system. After discussing many different solutions (all equally complex) we concluded the following:
The queries used to count to number of issues/MRs per state happens to be fast enough in most cases nowadays, most likely due to improvements made recently (e.g. merging "reopened" into "opened"). For example, when filtering by assignee or author most of these queries only take a few milliseconds to run.
The exception here is when you search for a term (e.g. "error"), in which case it can easily take half a second to count the number of rows. This however is true right now as well as the first time you search for a term we need to run these queries anyway.
Based on this we decided to take the following steps:
1. Remove Caching
First we'll remove the cache used for the opened/closed/merged state tabs at the top. This greatly simplifies the logic and allows us to re-use the numbers for the pagination system (as they are now always accurate). This will change our response timings, but not necessarily by that much. For example, currently in the best case we do this:
- Load the state counts from the cache
- Run a separate
COUNT(*)
for the pagination, taking up N milliseconds
By removing the cache we instead do the following:
- Run a
COUNT(*)
to count the rows per state, taking up N milliseconds + a little bit extra - Re-use this result for the pagination system
This means that in the best case our response timings won't even change, in the worst case they will go up a little bit.
When searching for a term (e.g. "error" in CE) the COUNT per state will take around 500 milliseconds, with an additional 300 milliseconds for the COUNT used by the pagination system; for a total of 800 or so milliseconds. By reusing the counts we will run the 500 milliseconds query, but we don't need the 300 milliseconds query. This means we only slow down the page by 500 milliseconds in all cases, instead of 800 milliseconds. This means the first request for a term is actually faster than when using caching.
Improve Cache Invalidation In The Sidebar
Right now the cache for issues/MRs in the sidebar is updated for every issue/MR update. Instead we should change this (by re-using the invalidation logic used for the caches at the top)
so this is only done when the state or confidential
flag changes.
Reuse Counts In Kaminari
Kaminari's paginate
method can take an optional total_pages:
argument. When given it won't run a COUNT query, instead re-using the value. A WIP MR for this can be found in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13805. By doing the above we can safely reuse this count while still providing an accurate number of pages/rows.
Impact
The impact is the following:
- Request timings will be more consistent since we're not pulling stuff from a cache and invalidating it every 2 minutes
- Request timings will be slightly slower in some cases. How visible this is depends on the cache hit ratio of our current approach
- The first request when searching for a term will be faster, but subsequent requests for the same term will stay the same instead of using a cache
Fortunately with the sidebar cache we shouldn't have much of a problem staying under 200 milliseconds of SQL time in most cases, especially once we start fixing some of the other problems on MR/issue index pages.