Commit 80586cb7 authored by Patrick Bajao's avatar Patrick Bajao

Do not cache attention requests count

In https://gitlab.com/gitlab-org/gitlab/-/issues/357385, it was
reported that the cached count is inaccurate with the actual results.

In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84140, the
count query has been improved (from ~400ms down to ~14ms).

Since the query is performing better, we don't need to cache the
count.

This is behind `uncached_mr_attention_requests_count` feature flag.
The invalidation calls are not behind the feature flag to keep it
simple. When we decide to remove the feature flag, we can also
remove the calls for `User#invalidate_attention_requested_count`.
parent 7c488bfa
...@@ -1712,8 +1712,12 @@ class User < ApplicationRecord ...@@ -1712,8 +1712,12 @@ class User < ApplicationRecord
end end
def attention_requested_open_merge_requests_count(force: false) def attention_requested_open_merge_requests_count(force: false)
Rails.cache.fetch(attention_request_cache_key, force: force, expires_in: COUNT_CACHE_VALIDITY_PERIOD) do if Feature.enabled?(:uncached_mr_attention_requests_count, self, default_enabled: :yaml)
MergeRequestsFinder.new(self, attention: self.username, state: 'opened', non_archived: true).execute.count MergeRequestsFinder.new(self, attention: self.username, state: 'opened', non_archived: true).execute.count
else
Rails.cache.fetch(attention_request_cache_key, force: force, expires_in: COUNT_CACHE_VALIDITY_PERIOD) do
MergeRequestsFinder.new(self, attention: self.username, state: 'opened', non_archived: true).execute.count
end
end end
end end
......
---
name: uncached_mr_attention_requests_count
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84145
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/357480
milestone: '14.10'
type: development
group: group::code review
default_enabled: false
...@@ -4900,17 +4900,36 @@ RSpec.describe User do ...@@ -4900,17 +4900,36 @@ RSpec.describe User do
end end
describe '#attention_requested_open_merge_requests_count' do describe '#attention_requested_open_merge_requests_count' do
it 'returns number of open merge requests from non-archived projects' do let(:user) { create(:user) }
user = create(:user) let(:project) { create(:project, :public) }
project = create(:project, :public) let(:archived_project) { create(:project, :public, :archived) }
archived_project = create(:project, :public, :archived)
before do
create(:merge_request, source_project: project, author: user, reviewers: [user]) create(:merge_request, source_project: project, author: user, reviewers: [user])
create(:merge_request, :closed, source_project: project, author: user, reviewers: [user]) create(:merge_request, :closed, source_project: project, author: user, reviewers: [user])
create(:merge_request, source_project: archived_project, author: user, reviewers: [user]) create(:merge_request, source_project: archived_project, author: user, reviewers: [user])
end
it 'returns number of open merge requests from non-archived projects' do
expect(Rails.cache).not_to receive(:fetch)
expect(user.attention_requested_open_merge_requests_count(force: true)).to eq 1 expect(user.attention_requested_open_merge_requests_count(force: true)).to eq 1
end end
context 'when uncached_mr_attention_requests_count is disabled' do
before do
stub_feature_flags(uncached_mr_attention_requests_count: false)
end
it 'fetches from cache' do
expect(Rails.cache).to receive(:fetch).with(
user.attention_request_cache_key,
force: false,
expires_in: described_class::COUNT_CACHE_VALIDITY_PERIOD
).and_call_original
expect(user.attention_requested_open_merge_requests_count).to eq 1
end
end
end end
describe '#assigned_open_issues_count' do describe '#assigned_open_issues_count' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment