Cache group issues count for state
What does this MR do?
Related to #325656 (closed)
- Backend changes
For issuable lists we execute an expensive COUNT
query to populate the counts in state tabs, using the module Gitlab::IssuablesCountForState
. The query is being called from multiple locations on this page but it’s cached using RequestStore
which results in only one query execution per page load.
Regardless, this can still be quite expensive because this query gets significantly slower for large number of issues. In this MR we introduce an option to cache these counts using Rails.cache
as well.
The conditions for this caching are the following:
- The state counts are all over the threshold value, currently set as 1000.
As the query calculates the total number of issuables and then groups them by state to return a hash with the states and values, we wouldn’t get any benefits from caching a single value.
For example, if we have
{ opened: 1000, closed: 100, all: 1100 }
, we would still need to execute the query to calculateclosed
state if we only cached the single value for the:opened
state. For this reason, we will cache the full hash, but only when every value is> 1000
. - The parent
group
is defined. - . The feature flag
:cached_issuables_state_count
is enabled for the group. - There are no filters or search terms present. Filters or search terms would change the number of issuables displayed, so for these cases, we can’t use cached values.
- The cache expires every 10 minutes
- Frontend changes
Given that these values would no longer be precise if they are cached, we will display them rounded to the thousands. Similarly to the counts in the sidebar at the group level. Even if the counts are not cached, we will still round all values over 1000 to keep consistency.
In later iterations, and if we consider this change to be an improvement in performance, we would like to be able to use a tooltip (or hovering) to fetch the actual value.
What does it apply to?
These changes will initially apply to issues at the group level only.
Although it can be extended to MRs and Epics, we want to test the performance changes in small increments and we will be using the FF cached_issues_state_count
enabled only for the group gitlab-org
to begin with.
The case for issuables at the project level is much more complex because the expensive COUNT
query is first being called from the page pagination, resulting in no change in performance if we cache the counts for the tabs.
How to reproduce?
- In a group with at least 1000 open issues and 1000 closed issues enable the FF using
Feature.enable(:cached_issues_state_count
- Navigate to the issues list. For example
http://localhost:3000/groups/gitlab-org/-/issues
- The values in the state tabs should be rounded.
- The following query should be cached:
SELECT COUNT(*) AS count_all, "issues"."state_id" AS issues_state_id FROM "issues" INNER JOIN "projects" ON "projects"."id" = "issues"."project_id" LEFT JOIN project_features ON projects.id = project_features.project_id WHERE "projects"."namespace_id" IN (WITH RECURSIVE "base_and_descendants" AS ((SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" = 22)
UNION
(SELECT "namespaces".* FROM "namespaces", "base_and_descendants" WHERE "namespaces"."type" = 'Group' AND "namespaces"."parent_id" = "base_and_descendants"."id")) SELECT "namespaces"."id" FROM "base_and_descendants" AS "namespaces") AND (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 1 AND (project_authorizations.project_id = projects.id)) OR projects.visibility_level IN (0,10,20)) AND ("project_features"."issues_access_level" > 0 OR "project_features"."issues_access_level" IS NULL) AND "projects"."archived" = FALSE AND "issues"."issue_type" IN (0, 1) GROUP BY "issues"."state_id"
Alternatively, to test only the changes in the tabs template with our having to create the issues, we can modify the counts value in the helper. For example in the line 196 in app/helpers/issuables_helper.rb
replace
count = issuables_count_for_state(issuable_type, state)
for
count = { opened: 1000, closed: 1000, all: 2000 }
Screenshots (strongly suggested)
Issuables count in tabs | COUNT query |
---|---|
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.