diff options
| -rw-r--r-- | app/workers/project_cache_worker.rb | 33 | ||||
| -rw-r--r-- | changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml | 5 | ||||
| -rw-r--r-- | spec/workers/project_cache_worker_spec.rb | 73 |
3 files changed, 64 insertions, 47 deletions
diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index b0e1d8837d9..abe86066fb4 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -3,6 +3,7 @@ # Worker for updating any project specific caches. class ProjectCacheWorker include ApplicationWorker + include ExclusiveLeaseGuard LEASE_TIMEOUT = 15.minutes.to_i @@ -13,30 +14,30 @@ class ProjectCacheWorker # statistics - An Array containing columns from ProjectStatistics to # refresh, if empty all columns will be refreshed def perform(project_id, files = [], statistics = []) - project = Project.find_by(id: project_id) + @project = Project.find_by(id: project_id) + return unless @project&.repository&.exists? - return unless project && project.repository.exists? + update_statistics(statistics) - update_statistics(project, statistics.map(&:to_sym)) + @project.repository.refresh_method_caches(files.map(&:to_sym)) - project.repository.refresh_method_caches(files.map(&:to_sym)) - - project.cleanup + @project.cleanup end - def update_statistics(project, statistics = []) - return unless try_obtain_lease_for(project.id, :update_statistics) - - Rails.logger.info("Updating statistics for project #{project.id}") + private - project.statistics.refresh!(only: statistics) + def update_statistics(statistics = []) + try_obtain_lease do + Rails.logger.info("Updating statistics for project #{@project.id}") + @project.statistics.refresh!(only: statistics.to_a.map(&:to_sym)) + end end - private + def lease_timeout + LEASE_TIMEOUT + end - def try_obtain_lease_for(project_id, section) - Gitlab::ExclusiveLease - .new("project_cache_worker:#{project_id}:#{section}", timeout: LEASE_TIMEOUT) - .try_obtain + def lease_key + "project_cache_worker:#{@project.id}:update_statistics" end end diff --git a/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml b/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml new file mode 100644 index 00000000000..bae6c2a8987 --- /dev/null +++ b/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml @@ -0,0 +1,5 @@ +--- +title: Cancel ExclusiveLease upon completion in ProjectCacheWorker +merge_request: 20103 +author: +type: fixed diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index b9b5445562f..8c4daac5f80 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -9,44 +9,50 @@ describe ProjectCacheWorker do let(:lease_key) { "project_cache_worker:#{project.id}:update_statistics" } let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT } - describe '#perform' do - before do - stub_exclusive_lease(lease_key, timeout: lease_timeout) - end + before do + stub_exclusive_lease(lease_key, timeout: lease_timeout) + + allow(Project).to receive(:find_by) + .with(id: project.id) + .and_return(project) + end + describe '#perform' do context 'with a non-existing project' do - it 'does nothing' do - expect(worker).not_to receive(:update_statistics) + it 'does not update statistic' do + allow(Project).to receive(:find_by).with(id: -1).and_return(nil) + + expect(subject).not_to receive(:update_statistics) - worker.perform(-1) + subject.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) + it 'does not update statistics' do + allow(project.repository).to receive(:exists?).and_return(false) - expect(worker).not_to receive(:update_statistics) + expect(subject).not_to receive(:update_statistics) - worker.perform(project.id) + subject.perform(project.id) end end 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 + expect(subject).to receive(:update_statistics) + .with(%w(repository_size)) + .and_call_original - worker.perform(project.id, [], %w(repository_size)) + subject.perform(project.id, [], %w(repository_size)) end it 'refreshes the method caches' do - expect_any_instance_of(Repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original + expect(project.repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original - worker.perform(project.id, %w(readme)) + subject.perform(project.id, %w(readme)) end context 'with plain readme' do @@ -54,23 +60,22 @@ describe ProjectCacheWorker do allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) allow(MarkupHelper).to receive(:plain?).and_return(true) - expect_any_instance_of(Repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original - worker.perform(project.id, %w(readme)) + expect(project.repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original + + subject.perform(project.id, %w(readme)) end end end - end - describe '#update_statistics' do context 'when a lease could not be obtained' do it 'does not update the repository size' do stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) - expect(statistics).not_to receive(:refresh!) + expect(project.statistics).not_to receive(:refresh!) - worker.update_statistics(project) + subject.perform(project.id, [], %w(repository_size)) end end @@ -78,11 +83,17 @@ describe ProjectCacheWorker do it 'updates the project statistics' do stub_exclusive_lease(lease_key, timeout: lease_timeout) - expect(statistics).to receive(:refresh!) - .with(only: %i(repository_size)) - .and_call_original + expect(project.statistics).to receive(:refresh!) + .with(only: %i(repository_size)) + .and_call_original + + subject.perform(project.id, [], %i(repository_size)) + end + + it 'cancels the lease after statistics has been updated' do + expect(subject).to receive(:release_lease).with('uuid') - worker.update_statistics(project, %i(repository_size)) + subject.perform(project.id, [], %i(repository_size)) end end end |