diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-07-04 14:04:58 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-07-04 14:04:58 +0300 |
commit | 14d2b52b00b49443146fbfc5c5b90de67fafa4e0 (patch) | |
tree | f9e6f7a1de90f131dffc4ed203c15c8a83c96e40 | |
parent | 935969be250435b482863e42684fdd7cef7d37c0 (diff) |
Revert "Merge branch '44726-cancel_lease_upon_completion_in_project_cache_worker' into 'master'"
This reverts merge request !20103
-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, 47 insertions, 64 deletions
diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index abe86066fb4..b0e1d8837d9 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -3,7 +3,6 @@ # Worker for updating any project specific caches. class ProjectCacheWorker include ApplicationWorker - include ExclusiveLeaseGuard LEASE_TIMEOUT = 15.minutes.to_i @@ -14,30 +13,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) - return unless @project&.repository&.exists? + project = Project.find_by(id: project_id) - update_statistics(statistics) + return unless project && project.repository.exists? - @project.repository.refresh_method_caches(files.map(&:to_sym)) + update_statistics(project, statistics.map(&:to_sym)) - @project.cleanup + project.repository.refresh_method_caches(files.map(&:to_sym)) + + project.cleanup end - private + def update_statistics(project, statistics = []) + return unless try_obtain_lease_for(project.id, :update_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 + Rails.logger.info("Updating statistics for project #{project.id}") - def lease_timeout - LEASE_TIMEOUT + project.statistics.refresh!(only: statistics) end - def lease_key - "project_cache_worker:#{@project.id}:update_statistics" + private + + def try_obtain_lease_for(project_id, section) + Gitlab::ExclusiveLease + .new("project_cache_worker:#{project_id}:#{section}", timeout: LEASE_TIMEOUT) + .try_obtain 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 deleted file mode 100644 index bae6c2a8987..00000000000 --- a/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -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 8c4daac5f80..b9b5445562f 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -9,50 +9,44 @@ describe ProjectCacheWorker do let(:lease_key) { "project_cache_worker:#{project.id}:update_statistics" } let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT } - 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 not update statistic' do - allow(Project).to receive(:find_by).with(id: -1).and_return(nil) + before do + stub_exclusive_lease(lease_key, timeout: lease_timeout) + end - expect(subject).not_to receive(:update_statistics) + context 'with a non-existing project' do + it 'does nothing' do + expect(worker).not_to receive(:update_statistics) - subject.perform(-1) + worker.perform(-1) end end context 'with an existing project without a repository' do - it 'does not update statistics' do - allow(project.repository).to receive(:exists?).and_return(false) + it 'does nothing' do + allow_any_instance_of(Repository).to receive(:exists?).and_return(false) - expect(subject).not_to receive(:update_statistics) + expect(worker).not_to receive(:update_statistics) - subject.perform(project.id) + worker.perform(project.id) end end context 'with an existing project' do it 'updates the project statistics' do - expect(subject).to receive(:update_statistics) - .with(%w(repository_size)) - .and_call_original + expect(worker).to receive(:update_statistics) + .with(kind_of(Project), %i(repository_size)) + .and_call_original - subject.perform(project.id, [], %w(repository_size)) + worker.perform(project.id, [], %w(repository_size)) end it 'refreshes the method caches' do - expect(project.repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original + expect_any_instance_of(Repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original - subject.perform(project.id, %w(readme)) + worker.perform(project.id, %w(readme)) end context 'with plain readme' do @@ -60,22 +54,23 @@ describe ProjectCacheWorker do allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) allow(MarkupHelper).to receive(:plain?).and_return(true) - expect(project.repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original - - subject.perform(project.id, %w(readme)) + expect_any_instance_of(Repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original + worker.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(project.statistics).not_to receive(:refresh!) + expect(statistics).not_to receive(:refresh!) - subject.perform(project.id, [], %w(repository_size)) + worker.update_statistics(project) end end @@ -83,17 +78,11 @@ describe ProjectCacheWorker do it 'updates the project statistics' do stub_exclusive_lease(lease_key, timeout: lease_timeout) - 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') + expect(statistics).to receive(:refresh!) + .with(only: %i(repository_size)) + .and_call_original - subject.perform(project.id, [], %i(repository_size)) + worker.update_statistics(project, %i(repository_size)) end end end |