Merge branch 'revert-1ee3ac8c' into 'master'

Revert "Merge branch '44726-cancel_lease_upon_completion_in_project_cache_worker' into 'master'"

See merge request gitlab-org/gitlab-ce!20366

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

changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml

----
-title: Cancel ExclusiveLease upon completion in ProjectCacheWorker
-merge_request: 20103
-author:
-type: fixed

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