diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-02-09 17:19:12 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-02-09 17:19:12 +0000 |
commit | 883bbd61ca6f465a8fa15583bcefcafd83e40b02 (patch) | |
tree | 53f330b7e97af17a00c5f8d896128d95eae8523f | |
parent | c8b6ec47ade1c8685c085310d650fb9af1c44521 (diff) | |
parent | 2ce0d06389c3eb7ac9a2b81f9fd339e7e56512bb (diff) | |
download | gitlab-ce-883bbd61ca6f465a8fa15583bcefcafd83e40b02.tar.gz |
Merge branch 'smarter-diverging-commit-cache-flushing' into 'master'
Smarter flushing of branch statistics caches
This basically ensures we only flush caches of branches whenever we really have to. See commit c514f8b850219cd3e5526e73e1d00e6729e2b466 for the details.
cc @joshfng @rspeicher
See merge request !2769
-rw-r--r-- | app/models/repository.rb | 25 | ||||
-rw-r--r-- | app/services/git_push_service.rb | 5 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 34 | ||||
-rw-r--r-- | spec/services/git_push_service_spec.rb | 6 |
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 |