Commit 8e218edb authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'project-cache-worker-lease' into 'master'

Restrict ProjectCacheWorker jobs to one per 15 min

## What does this MR do?

This restricts performing ProjectCacheWorker jobs to once every 15 minutes per project. The goal of this is to reduce disk load in the event of a lot of pushes happening in a short period of time. 

The impact is that cached data (e.g. commit count) may be updated less frequently. Furthermore it could mean that separate push payloads won't lead to e.g. READMEs being updated. Most of the cached data isn't updated very frequently, so this trade off should be worth it considering the alternative is a burning storage layer. 

In the future we'll have to flush caches in a smarter way. For example, refreshing the README cache could probably be done separately from the other caches with its own lease and such. See https://gitlab.com/gitlab-org/gitlab-ce/issues/23550 for more information.

See merge request !7017
parents 9180bdd4 bc31a489
...@@ -2,7 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -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) ## 8.14.0 (2016-11-22)
- Adds user project membership expired event to clarify why user was removed (Callum Dryden) - 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) ## 8.13.0 (2016-10-22)
...@@ -36,6 +36,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -36,6 +36,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 - AbstractReferenceFilter caches project_refs on RequestStore when active
- Replaced the check sign to arrow in the show build view. !6501 - 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) - 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 - 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) - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar)
- Speed-up group milestones show page - Speed-up group milestones show page
......
# 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 class ProjectCacheWorker
include Sidekiq::Worker include Sidekiq::Worker
sidekiq_options queue: :default sidekiq_options queue: :default
LEASE_TIMEOUT = 15.minutes.to_i
def perform(project_id) 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) project = Project.find(project_id)
return unless project.repository.exists? return unless project.repository.exists?
...@@ -15,4 +36,10 @@ class ProjectCacheWorker ...@@ -15,4 +36,10 @@ class ProjectCacheWorker
project.repository.build_cache project.repository.build_cache
end end
end end
def try_obtain_lease_for(project_id)
Gitlab::ExclusiveLease.
new("project_cache_worker:#{project_id}", timeout: LEASE_TIMEOUT).
try_obtain
end
end end
...@@ -6,21 +6,39 @@ describe ProjectCacheWorker do ...@@ -6,21 +6,39 @@ describe ProjectCacheWorker do
subject { described_class.new } subject { described_class.new }
describe '#perform' do describe '#perform' do
it 'updates project cache data' do context 'when an exclusive lease can be obtained' do
expect_any_instance_of(Repository).to receive(:size) before do
expect_any_instance_of(Repository).to receive(:commit_count) 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) it 'updates project cache data' do
expect_any_instance_of(Project).to receive(:update_commit_count) 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 end
it 'handles missing repository data' do context 'when an exclusive lease can not be obtained' do
expect_any_instance_of(Repository).to receive(:exists?).and_return(false) it 'does nothing' do
expect_any_instance_of(Repository).not_to receive(:size) allow(subject).to receive(:try_obtain_lease_for).with(project.id).
and_return(false)
expect(subject).not_to receive(:update_caches)
subject.perform(project.id) subject.perform(project.id)
end
end 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