From 0e355e5c9293c712b5df65896371af0ba71c19b2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 27 Jul 2017 14:58:02 +0200 Subject: [PATCH] Load and process at most 100 commits when pushing into default branch --- app/services/git_push_service.rb | 31 +++++++++++++------ .../unreleased/dm-large-push-performance.yml | 4 +++ lib/gitlab/data_builder/push.rb | 4 +-- spec/services/git_push_service_spec.rb | 6 ++-- 4 files changed, 29 insertions(+), 16 deletions(-) create mode 100644 changelogs/unreleased/dm-large-push-performance.yml diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 20d1fb29289..57ce2a472aa 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -45,6 +45,7 @@ class GitPushService < BaseService elsif push_to_existing_branch? # Collect data for this git push @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) + process_commit_messages # Update the bare repositories info/attributes file using the contents of the default branches @@ -64,15 +65,21 @@ class GitPushService < BaseService def update_caches if is_default_branch? - paths = Set.new + if push_to_new_branch? + # If this is the initial push into the default branch, the file type caches + # will already be reset as a result of `Project#change_head`. + types = [] + else + paths = Set.new - @push_commits.each do |commit| - commit.raw_deltas.each do |diff| - paths << diff.new_path + @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| + commit.raw_deltas.each do |diff| + paths << diff.new_path + end end - end - types = Gitlab::FileDetector.types_in_paths(paths.to_a) + types = Gitlab::FileDetector.types_in_paths(paths.to_a) + end else types = [] end @@ -84,7 +91,7 @@ class GitPushService < BaseService def process_commit_messages default = is_default_branch? - push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| + @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| if commit.matches_cross_reference_regex? ProcessCommitWorker .perform_async(project.id, current_user.id, commit.to_hash, default) @@ -103,7 +110,7 @@ class GitPushService < BaseService EventCreateService.new.push(@project, current_user, build_push_data) Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) - + SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) @@ -123,7 +130,10 @@ class GitPushService < BaseService end def process_default_branch - @push_commits = project.repository.commits(params[:newrev]) + @push_commits_count = project.repository.commit_count_for_ref(params[:ref]) + + offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max + @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) # Ensure HEAD points to the default branch in case it is not master project.change_head(branch_name) @@ -152,7 +162,8 @@ class GitPushService < BaseService params[:oldrev], params[:newrev], params[:ref], - push_commits) + @push_commits, + commits_count: @push_commits_count) end def push_to_existing_branch? diff --git a/changelogs/unreleased/dm-large-push-performance.yml b/changelogs/unreleased/dm-large-push-performance.yml new file mode 100644 index 00000000000..f5fe1bd3b28 --- /dev/null +++ b/changelogs/unreleased/dm-large-push-performance.yml @@ -0,0 +1,4 @@ +--- +title: Improve performance of large (initial) push into default branch +merge_request: +author: diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index 8c8729b6557..5c5f507d44d 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -24,11 +24,11 @@ module Gitlab # total_commits_count: Fixnum # } # - def build(project, user, oldrev, newrev, ref, commits = [], message = nil) + def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil) commits = Array(commits) # Total commits count - commits_count = commits.size + commits_count ||= commits.size # Get latest 20 commits ASC commits_limited = commits.last(20) diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index f801506f1b6..cf9e63676b7 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -663,8 +663,7 @@ describe GitPushService, services: true do end it 'only schedules a limited number of commits' do - allow(service).to receive(:push_commits) - .and_return(Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true))) + service.push_commits = Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)) expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times @@ -672,8 +671,7 @@ describe GitPushService, services: true do end it "skips commits which don't include cross-references" do - allow(service).to receive(:push_commits) - .and_return([double(:commit, to_hash: {}, matches_cross_reference_regex?: false)]) + service.push_commits = [double(:commit, to_hash: {}, matches_cross_reference_regex?: false)] expect(ProcessCommitWorker).not_to receive(:perform_async)