summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2016-02-09 14:59:11 +0100
committerYorick Peterse <yorickpeterse@gmail.com>2016-02-09 17:17:56 +0100
commit2ce0d06389c3eb7ac9a2b81f9fd339e7e56512bb (patch)
tree67f6a2c4880cb910d2cdfcbdd4a2e0c5ac2b6600
parent643c61867cc99f626d58798a5365e7573f90b176 (diff)
downloadgitlab-ce-2ce0d06389c3eb7ac9a2b81f9fd339e7e56512bb.tar.gz
Smarter flushing of branch statistics caches
Instead of flushing the behind/ahead counts for all branches upon every push we now only flush the cache of branches that actually need to have these statistics recalculated. There are now basically 2 scenarios and their effects: 1. A user pushes a commit to the default branch, this results in the cache being flushed for all branches. 2. A user pushes to a non default branch, this results in _only_ the cache for that branch being flushed. The existing code (Repository#expire_cache) remains backwards compatible with the previous behaviour, the new behaviour is only applied when a branch name is passed as an argument. This ensures that when for example a project is deleted the cache for all branches is flushed.
-rw-r--r--app/models/repository.rb25
-rw-r--r--app/services/git_push_service.rb5
-rw-r--r--spec/models/repository_spec.rb34
-rw-r--r--spec/services/git_push_service_spec.rb6
4 files changed, 54 insertions, 16 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 27bdbac3e52..0e17d2b669a 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -205,12 +205,6 @@ class Repository
readme version contribution_guide changelog license)
end
- def branch_cache_keys
- branches.map do |branch|
- :"diverging_commit_counts_#{branch.name}"
- end
- end
-
def build_cache
cache_keys.each do |key|
unless cache.exist?(key)
@@ -235,17 +229,26 @@ class Repository
@branches = nil
end
- def expire_cache
+ def expire_cache(branch_name = nil)
cache_keys.each do |key|
cache.expire(key)
end
- expire_branch_cache
+ expire_branch_cache(branch_name)
end
- def expire_branch_cache
- branches.each do |branch|
- cache.expire(:"diverging_commit_counts_#{branch.name}")
+ def expire_branch_cache(branch_name = nil)
+ # When we push to the root branch we have to flush the cache for all other
+ # branches as their statistics are based on the commits relative to the
+ # root branch.
+ if !branch_name || branch_name == root_ref
+ branches.each do |branch|
+ cache.expire(:"diverging_commit_counts_#{branch.name}")
+ end
+ # In case a commit is pushed to a non-root branch we only have to flush the
+ # cache for said branch.
+ else
+ cache.expire(:"diverging_commit_counts_#{branch_name}")
end
end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 0f31756797d..e3bf14966c8 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -18,7 +18,9 @@ class GitPushService
def execute(project, user, oldrev, newrev, ref)
@project, @user = project, user
- project.repository.expire_cache
+ branch_name = Gitlab::Git.ref_name(ref)
+
+ project.repository.expire_cache(branch_name)
if push_remove_branch?(ref, newrev)
project.repository.expire_has_visible_content_cache
@@ -33,7 +35,6 @@ class GitPushService
@push_commits = project.repository.commits(newrev)
# Ensure HEAD points to the default branch in case it is not master
- branch_name = Gitlab::Git.ref_name(ref)
project.change_head(branch_name)
# Set protection on the default branch if configured
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 753012be578..72b4ac6d660 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -291,6 +291,12 @@ describe Repository, models: true do
repository.expire_cache
end
+
+ it 'expires the caches for a specific branch' do
+ expect(repository).to receive(:expire_branch_cache).with('master')
+
+ repository.expire_cache('master')
+ end
end
describe '#expire_root_ref_cache' do
@@ -320,4 +326,32 @@ describe Repository, models: true do
expect(repository.has_visible_content?).to eq(false)
end
end
+
+ describe '#expire_branch_ache' do
+ # This method is private but we need it for testing purposes. Sadly there's
+ # no other proper way of testing caching operations.
+ let(:cache) { repository.send(:cache) }
+
+ it 'expires the cache for all branches' do
+ expect(cache).to receive(:expire).
+ at_least(repository.branches.length).
+ times
+
+ repository.expire_branch_cache
+ end
+
+ it 'expires the cache for all branches when the root branch is given' do
+ expect(cache).to receive(:expire).
+ at_least(repository.branches.length).
+ times
+
+ repository.expire_branch_cache(repository.root_ref)
+ end
+
+ it 'expires the cache for a specific branch' do
+ expect(cache).to receive(:expire).once
+
+ repository.expire_branch_cache('foo')
+ end
+ end
end
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 349809743c7..eb3a5fe43f5 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -23,7 +23,7 @@ describe GitPushService, services: true do
it { is_expected.to be_truthy }
it 'flushes general cached data' do
- expect(project.repository).to receive(:expire_cache)
+ expect(project.repository).to receive(:expire_cache).with('master')
subject
end
@@ -43,7 +43,7 @@ describe GitPushService, services: true do
it { is_expected.to be_truthy }
it 'flushes general cached data' do
- expect(project.repository).to receive(:expire_cache)
+ expect(project.repository).to receive(:expire_cache).with('master')
subject
end
@@ -63,7 +63,7 @@ describe GitPushService, services: true do
end
it 'flushes general cached data' do
- expect(project.repository).to receive(:expire_cache)
+ expect(project.repository).to receive(:expire_cache).with('master')
subject
end