Commit 0adedbb4 authored by Hiroyuki Sato's avatar Hiroyuki Sato

Fix the bug that the project statistics is not updated

parent c7f918aa
...@@ -143,6 +143,7 @@ ...@@ -143,6 +143,7 @@
- repository_remove_remote - repository_remove_remote
- system_hook_push - system_hook_push
- update_merge_requests - update_merge_requests
- update_project_statistics
- upload_checksum - upload_checksum
- web_hook - web_hook
- repository_update_remote_mirror - repository_update_remote_mirror
......
...@@ -28,18 +28,20 @@ class ProjectCacheWorker ...@@ -28,18 +28,20 @@ class ProjectCacheWorker
def update_statistics(project, statistics = []) def update_statistics(project, statistics = [])
return if Gitlab::Database.read_only? return if Gitlab::Database.read_only?
return unless try_obtain_lease_for(project.id, :update_statistics) return unless try_obtain_lease_for(project.id, statistics)
Rails.logger.info("Updating statistics for project #{project.id}") UpdateProjectStatisticsWorker.perform_in(LEASE_TIMEOUT, project.id, statistics)
project.statistics.refresh!(only: statistics)
end end
private private
def try_obtain_lease_for(project_id, section) def try_obtain_lease_for(project_id, statistics)
Gitlab::ExclusiveLease Gitlab::ExclusiveLease
.new("project_cache_worker:#{project_id}:#{section}", timeout: LEASE_TIMEOUT) .new(project_cache_worker_key(project_id, statistics), timeout: LEASE_TIMEOUT)
.try_obtain .try_obtain
end end
def project_cache_worker_key(project_id, statistics)
(["project_cache_worker", project_id] + statistics.sort).join(":")
end
end end
# frozen_string_literal: true
# Worker for updating project statistics.
class UpdateProjectStatisticsWorker
include ApplicationWorker
# project_id - The ID of the project for which to flush the cache.
# statistics - An Array containing columns from ProjectStatistics to
# refresh, if empty all columns will be refreshed
# rubocop: disable CodeReuse/ActiveRecord
def perform(project_id, statistics = [])
project = Project.find_by(id: project_id)
return unless project && project.repository.exists?
Rails.logger.info("Updating statistics for project #{project.id}")
project.statistics.refresh!(only: statistics.map(&:to_sym))
end
# rubocop: enable CodeReuse/ActiveRecord
end
---
title: Fix the bug that the project statistics is not updated
merge_request: 26854
author: Hiroyuki Sato
type: fixed
...@@ -90,3 +90,4 @@ ...@@ -90,3 +90,4 @@
- [import_issues_csv, 2] - [import_issues_csv, 2]
- [chat_notification, 2] - [chat_notification, 2]
- [migrate_external_diffs, 1] - [migrate_external_diffs, 1]
- [update_project_statistics, 1]
...@@ -7,9 +7,9 @@ describe ProjectCacheWorker do ...@@ -7,9 +7,9 @@ describe ProjectCacheWorker do
let(:worker) { described_class.new } let(:worker) { described_class.new }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:statistics) { project.statistics } let(:lease_key) { (["project_cache_worker", project.id] + statistics.sort).join(":") }
let(:lease_key) { "project_cache_worker:#{project.id}:update_statistics" }
let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT } let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT }
let(:statistics) { [] }
describe '#perform' do describe '#perform' do
before do before do
...@@ -35,14 +35,6 @@ describe ProjectCacheWorker do ...@@ -35,14 +35,6 @@ describe ProjectCacheWorker do
end end
context 'with an existing project' do context 'with an existing project' do
it 'updates the project statistics' do
expect(worker).to receive(:update_statistics)
.with(kind_of(Project), %i(repository_size))
.and_call_original
worker.perform(project.id, [], %w(repository_size))
end
it 'refreshes the method caches' do it 'refreshes the method caches' do
expect_any_instance_of(Repository).to receive(:refresh_method_caches) expect_any_instance_of(Repository).to receive(:refresh_method_caches)
.with(%i(readme)) .with(%i(readme))
...@@ -51,6 +43,18 @@ describe ProjectCacheWorker do ...@@ -51,6 +43,18 @@ describe ProjectCacheWorker do
worker.perform(project.id, %w(readme)) worker.perform(project.id, %w(readme))
end end
context 'with statistics' do
let(:statistics) { %w(repository_size) }
it 'updates the project statistics' do
expect(worker).to receive(:update_statistics)
.with(kind_of(Project), %i(repository_size))
.and_call_original
worker.perform(project.id, [], statistics)
end
end
context 'with plain readme' do context 'with plain readme' do
it 'refreshes the method caches' do it 'refreshes the method caches' do
allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false)
...@@ -66,13 +70,15 @@ describe ProjectCacheWorker do ...@@ -66,13 +70,15 @@ describe ProjectCacheWorker do
end end
describe '#update_statistics' do describe '#update_statistics' do
let(:statistics) { %w(repository_size) }
context 'when a lease could not be obtained' do context 'when a lease could not be obtained' do
it 'does not update the repository size' do it 'does not update the repository size' do
stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
expect(statistics).not_to receive(:refresh!) expect(UpdateProjectStatisticsWorker).not_to receive(:perform_in)
worker.update_statistics(project) worker.update_statistics(project, statistics.map(&:to_sym))
end end
end end
...@@ -80,11 +86,11 @@ describe ProjectCacheWorker do ...@@ -80,11 +86,11 @@ describe ProjectCacheWorker do
it 'updates the project statistics' do it 'updates the project statistics' do
stub_exclusive_lease(lease_key, timeout: lease_timeout) stub_exclusive_lease(lease_key, timeout: lease_timeout)
expect(statistics).to receive(:refresh!) expect(UpdateProjectStatisticsWorker).to receive(:perform_in)
.with(only: %i(repository_size)) .with(lease_timeout, project.id, statistics.map(&:to_sym))
.and_call_original .and_call_original
worker.update_statistics(project, %i(repository_size)) worker.update_statistics(project, statistics.map(&:to_sym))
end end
end end
end end
......
require 'spec_helper'
describe UpdateProjectStatisticsWorker do
let(:worker) { described_class.new }
let(:project) { create(:project, :repository) }
describe '#perform' do
context 'with a non-existing project' do
it 'does nothing' do
expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
worker.perform(-1)
end
end
context 'with an existing project without a repository' do
it 'does nothing' do
allow_any_instance_of(Repository).to receive(:exists?).and_return(false)
expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
worker.perform(project.id)
end
end
context 'with an existing project' do
it 'refreshes the project statistics' do
expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
.with(only: [])
.and_call_original
worker.perform(project.id)
end
context 'with a specific statistics target' do
it 'refreshes the project repository size' do
statistics_target = %w(repository_size)
expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
.with(only: statistics_target.map(&:to_sym))
.and_call_original
worker.perform(project.id, statistics_target)
end
end
end
end
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