From bc31a489dd8c329cf011ac898498735809c319dd Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 20 Oct 2016 12:59:39 +0200
Subject: [PATCH] Restrict ProjectCacheWorker jobs to one per 15 min

This ensures ProjectCacheWorker jobs for a given project are performed
at most once per 15 minutes. This should reduce disk load a bit in cases
where there are multiple pushes happening (which should schedule
multiple ProjectCacheWorker jobs).
---
 CHANGELOG.md                              |  3 +-
 app/workers/project_cache_worker.rb       | 27 ++++++++++++++++
 spec/workers/project_cache_worker_spec.rb | 38 +++++++++++++++++------
 3 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 399a7f65267..ba5d72d3ef5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,7 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 ## 8.14.0 (2016-11-22)
   - Adds user project membership expired event to clarify why user was removed (Callum Dryden)
-  - Simpler arguments passed to named_route on toggle_award_url helper method 
+  - Simpler arguments passed to named_route on toggle_award_url helper method
 
 ## 8.13.0 (2016-10-22)
 
@@ -35,6 +35,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - AbstractReferenceFilter caches project_refs on RequestStore when active
   - Replaced the check sign to arrow in the show build view. !6501
   - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar)
+  - ProjectCacheWorker updates caches at most once per 15 minutes per project
   - Fix Error 500 when viewing old merge requests with bad diff data
   - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar)
   - Speed-up group milestones show page
diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb
index ccefd0f71a0..0d524e88dc3 100644
--- a/app/workers/project_cache_worker.rb
+++ b/app/workers/project_cache_worker.rb
@@ -1,9 +1,30 @@
+# Worker for updating any project specific caches.
+#
+# This worker runs at most once every 15 minutes per project. This is to ensure
+# that multiple instances of jobs for this worker don't hammer the underlying
+# storage engine as much.
 class ProjectCacheWorker
   include Sidekiq::Worker
 
   sidekiq_options queue: :default
 
+  LEASE_TIMEOUT = 15.minutes.to_i
+
   def perform(project_id)
+    if try_obtain_lease_for(project_id)
+      Rails.logger.
+        info("Obtained ProjectCacheWorker lease for project #{project_id}")
+    else
+      Rails.logger.
+        info("Could not obtain ProjectCacheWorker lease for project #{project_id}")
+
+      return
+    end
+
+    update_caches(project_id)
+  end
+
+  def update_caches(project_id)
     project = Project.find(project_id)
 
     return unless project.repository.exists?
@@ -15,4 +36,10 @@ class ProjectCacheWorker
       project.repository.build_cache
     end
   end
+
+  def try_obtain_lease_for(project_id)
+    Gitlab::ExclusiveLease.
+      new("project_cache_worker:#{project_id}", timeout: LEASE_TIMEOUT).
+      try_obtain
+  end
 end
diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb
index 5785a6a06ff..f5b60b90d11 100644
--- a/spec/workers/project_cache_worker_spec.rb
+++ b/spec/workers/project_cache_worker_spec.rb
@@ -6,21 +6,39 @@ describe ProjectCacheWorker do
   subject { described_class.new }
 
   describe '#perform' do
-    it 'updates project cache data' do
-      expect_any_instance_of(Repository).to receive(:size)
-      expect_any_instance_of(Repository).to receive(:commit_count)
+    context 'when an exclusive lease can be obtained' do
+      before do
+        allow(subject).to receive(:try_obtain_lease_for).with(project.id).
+          and_return(true)
+      end
 
-      expect_any_instance_of(Project).to receive(:update_repository_size)
-      expect_any_instance_of(Project).to receive(:update_commit_count)
+      it 'updates project cache data' do
+        expect_any_instance_of(Repository).to receive(:size)
+        expect_any_instance_of(Repository).to receive(:commit_count)
 
-      subject.perform(project.id)
+        expect_any_instance_of(Project).to receive(:update_repository_size)
+        expect_any_instance_of(Project).to receive(:update_commit_count)
+
+        subject.perform(project.id)
+      end
+
+      it 'handles missing repository data' do
+        expect_any_instance_of(Repository).to receive(:exists?).and_return(false)
+        expect_any_instance_of(Repository).not_to receive(:size)
+
+        subject.perform(project.id)
+      end
     end
 
-    it 'handles missing repository data' do
-      expect_any_instance_of(Repository).to receive(:exists?).and_return(false)
-      expect_any_instance_of(Repository).not_to receive(:size)
+    context 'when an exclusive lease can not be obtained' do
+      it 'does nothing' do
+        allow(subject).to receive(:try_obtain_lease_for).with(project.id).
+          and_return(false)
 
-      subject.perform(project.id)
+        expect(subject).not_to receive(:update_caches)
+
+        subject.perform(project.id)
+      end
     end
   end
 end