From 5fe85bc8cae98d6996907fd5ce86760b72319306 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Tue, 28 Jun 2016 11:39:29 +0200 Subject: [PATCH] Expire branch/tag git data when needed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When pushing commits to existing branches we don’t need to flush branch git data (branch names / counts) When flushes the cache when pushing commits skip to flush branch and tag git data (names / counts) because those operations are managed explicitly in each case Repopulated expired cache as soon as possible --- CHANGELOG | 1 + app/models/repository.rb | 16 +++++----- spec/models/repository_spec.rb | 14 +++++---- spec/services/git_push_service_spec.rb | 36 ++++++++++++++++++++++ spec/services/git_tag_push_service_spec.rb | 25 +++++++++++++++ 5 files changed, 78 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4fbffb41436..38bcbbb024f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 8.10.0 (unreleased) - PipelinesFinder uses git cache data - Check for conflicts with existing Project's wiki path when creating a new project. - Remove unused front-end variable -> default_issues_tracker + - Better caching of git calls on ProjectsController#show. - Add API endpoint for a group issues !4520 (mahcsig) - Add Bugzilla integration !4930 (iamtjg) - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) diff --git a/app/models/repository.rb b/app/models/repository.rb index f45c3d06abd..eb232ea681b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -246,24 +246,26 @@ class Repository end end + # Keys for data that can be affected for any commit push. def cache_keys - %i(size branch_names tag_names branch_count tag_count commit_count + %i(size commit_count readme version contribution_guide changelog license_blob license_key gitignore) end + # Keys for data on branch/tag operations. + def cache_keys_for_branches_and_tags + %i(branch_names tag_names branch_count tag_count) + end + def build_cache - cache_keys.each do |key| + (cache_keys + cache_keys_for_branches_and_tags).each do |key| unless cache.exist?(key) send(key) end end end - def expire_gitignore - cache.expire(:gitignore) - end - def expire_tags_cache cache.expire(:tag_names) @tags = nil @@ -286,8 +288,6 @@ class Repository # This ensures this particular cache is flushed after the first commit to a # new repository. expire_emptiness_caches if empty? - expire_branch_count_cache - expire_tag_count_cache end def expire_branch_cache(branch_name = nil) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d8350000bf6..8ae083d7132 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -531,8 +531,6 @@ describe Repository, models: true do describe '#expire_cache' do it 'expires all caches' do expect(repository).to receive(:expire_branch_cache) - expect(repository).to receive(:expire_branch_count_cache) - expect(repository).to receive(:expire_tag_count_cache) repository.expire_cache end @@ -1055,12 +1053,14 @@ describe Repository, models: true do let(:cache) { repository.send(:cache) } it 'builds the caches if they do not already exist' do + cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags + expect(cache).to receive(:exist?). - exactly(repository.cache_keys.length). + exactly(cache_keys.length). times. and_return(false) - repository.cache_keys.each do |key| + cache_keys.each do |key| expect(repository).to receive(key) end @@ -1068,12 +1068,14 @@ describe Repository, models: true do end it 'does not build any caches that already exist' do + cache_keys = repository.cache_keys + repository.cache_keys_for_branches_and_tags + expect(cache).to receive(:exist?). - exactly(repository.cache_keys.length). + exactly(cache_keys.length). times. and_return(true) - repository.cache_keys.each do |key| + cache_keys.each do |key| expect(repository).not_to receive(key) end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index f99ad046f0d..1ceb7c3e6ff 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -40,6 +40,18 @@ describe GitPushService, services: true do subject end + + it 'flushes the branches cache' do + expect(project.repository).to receive(:expire_branches_cache) + + subject + end + + it 'flushes the branch count cache' do + expect(project.repository).to receive(:expire_branch_count_cache) + + subject + end end context 'existing branch' do @@ -52,6 +64,18 @@ describe GitPushService, services: true do subject end + + it 'does not flush the branches cache' do + expect(project.repository).not_to receive(:expire_branches_cache) + + subject + end + + it 'does not flush the branch count cache' do + expect(project.repository).not_to receive(:expire_branch_count_cache) + + subject + end end context 'rm branch' do @@ -66,6 +90,18 @@ describe GitPushService, services: true do subject end + it 'flushes the branches cache' do + expect(project.repository).to receive(:expire_branches_cache) + + subject + end + + it 'flushes the branch count cache' do + expect(project.repository).to receive(:expire_branch_count_cache) + + subject + end + it 'flushes general cached data' do expect(project.repository).to receive(:expire_cache). with('master', newrev) diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index a63656e6268..a4fcd44882d 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -11,6 +11,31 @@ describe GitTagPushService, services: true do let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0 let(:ref) { 'refs/tags/v1.1.0' } + describe "Push tags" do + subject do + service.execute + service + end + + it 'flushes general cached data' do + expect(project.repository).to receive(:expire_cache) + + subject + end + + it 'flushes the tags cache' do + expect(project.repository).to receive(:expire_tags_cache) + + subject + end + + it 'flushes the tag count cache' do + expect(project.repository).to receive(:expire_tag_count_cache) + + subject + end + end + describe "Git Tag Push Data" do before do service.execute