Commit bddd15bf authored by Stan Hu's avatar Stan Hu Committed by Luke Duncalfe

Flush statistics cache anytime it is updated

`Projects::UpdateStatisticsService` attempts to refresh project
statistics, but it relies the caller to flush the existing statistics
cache for the repository. If the flush doesn't happen, the refresh won't
actually do anything. For example, this might be happening in
`ProjectCacheWorker`.

We now expire the stats inside the service to ensure consistent results.

This came out of
https://gitlab.com/gitlab-org/gitlab/-/issues/255001.

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52938
parent ace19a7e
......@@ -2,18 +2,49 @@
module Projects
class UpdateStatisticsService < BaseService
include ::Gitlab::Utils::StrongMemoize
STAT_TO_CACHED_METHOD = {
repository_size: :size,
commit_count: :commit_count
}.freeze
def execute
return unless project
Gitlab::AppLogger.info("Updating statistics for project #{project.id}")
project.statistics.refresh!(only: statistics.map(&:to_sym))
expire_repository_caches
expire_wiki_caches
project.statistics.refresh!(only: statistics)
end
private
def expire_repository_caches
if statistics.empty?
project.repository.expire_statistics_caches
elsif method_caches_to_expire.present?
project.repository.expire_method_caches(method_caches_to_expire)
end
end
def expire_wiki_caches
return unless project.wiki_enabled? && statistics.include?(:wiki_size)
project.wiki.repository.expire_method_caches([:size])
end
def method_caches_to_expire
strong_memoize(:method_caches_to_expire) do
statistics.map { |stat| STAT_TO_CACHED_METHOD[stat] }.compact
end
end
def statistics
params[:statistics]
strong_memoize(:statistics) do
params[:statistics]&.map(&:to_sym)
end
end
end
end
---
title: Flush statistics cache anytime it is updated
merge_request: 52938
author:
type: fixed
......@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe Projects::UpdateStatisticsService do
using RSpec::Parameterized::TableSyntax
let(:service) { described_class.new(project, nil, statistics: statistics)}
let(:statistics) { %w(repository_size) }
......@@ -18,12 +20,46 @@ RSpec.describe Projects::UpdateStatisticsService do
end
context 'with an existing project' do
let(:project) { create(:project) }
let_it_be(:project) { create(:project) }
where(:statistics, :method_caches) do
[] | %i(size commit_count)
['repository_size'] | [:size]
[:repository_size] | [:size]
[:lfs_objects_size] | nil
[:commit_count] | [:commit_count] # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands
[:repository_size, :commit_count] | %i(size commit_count)
[:repository_size, :commit_count, :lfs_objects_size] | %i(size commit_count)
end
with_them do
it 'refreshes the project statistics' do
expect(project.statistics).to receive(:refresh!).with(only: statistics.map(&:to_sym)).and_call_original
service.execute
end
it 'invalidates the method caches after a refresh' do
expect(project.wiki.repository).not_to receive(:expire_method_caches)
if method_caches.present?
expect(project.repository).to receive(:expire_method_caches).with(method_caches).and_call_original
else
expect(project.repository).not_to receive(:expire_method_caches)
end
service.execute
end
end
end
context 'with an existing project with a Wiki' do
let(:project) { create(:project, :repository, :wiki_enabled) }
let(:statistics) { [:wiki_size] }
it 'refreshes the project statistics' do
expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
.with(only: statistics.map(&:to_sym))
.and_call_original
it 'invalidates and refreshes Wiki size' do
expect(project.statistics).to receive(:refresh!).with(only: statistics).and_call_original
expect(project.wiki.repository).to receive(:expire_method_caches).with(%i(size)).and_call_original
service.execute
end
......
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