Use DB field to cache user assigned open issues count [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
Due to the query retrieving the counts of assigned issues for a user being a bit expensive (~787ms) and this query being part of the layout (appearing on every page, cached for 20 minutes), this is an experiment in persisting the value for an easy-read-expensive-write strategy.
The strategy here is to leverage the already-existing cache values and slowly convert them over to a cache column on the DB. A background migration methodically converts them, while in the foreground new values can be saved and retrieved from the DB.
feature flag
Using feature flag assigned_open_issues_database_cache
and the idea is that it can be switched on and off relatively seamlessly - if it's off the cache is still used, but if it's on we expensive-write to the DB field. I want to avoid paying a heavy cost if it's switched on or off. So we use both the cache and the DB field when it's on, but only the cache when it's off.
Strategy:
- Create a new column in
users
(or some other table?) calledassigned_open_issues_count
- Begin background migration that converts cached values of
assigned_open_issues_count
to populate the above row of that user'sassigned_open_issues_count
column - The background migration then recalculates counts that do NOT have a DB value, very slowly since it is an expensive query.
Meanwhile...
- If we ask for
assigned_open_issues_count
on a user, we return the DB row value if it exists, then we return the cache value if it exists, then if neither of those values have been set, we recalculate. - The
IssuableBaseService
method https://gitlab.com/gitlab-org/gitlab/-/blob/f2d88f4f342b0035479d9092e458ccd524e2e380/app/services/issuable_base_service.rb#L457 necessitates an override that doesn't quite do that - but it does force a recalculation since something has changed.
DB migration output
up
$ brails db:migrate:up VERSION=20210322110033
== 20210322110033 AddAssignedOpenIssuesCountToUser: migrating =================
-- add_column(:users, :assigned_open_issues_count, :integer)
-> 0.0136s
== 20210322110033 AddAssignedOpenIssuesCountToUser: migrated (0.0136s) ========
down
$ brails db:migrate:down VERSION=20210322110033
== 20210322110033 AddAssignedOpenIssuesCountToUser: reverting =================
-- remove_column(:users, :assigned_open_issues_count)
-> 0.0025s
== 20210322110033 AddAssignedOpenIssuesCountToUser: reverted (0.0026s) ========
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #325470 (closed)