Commit 0bb58ff8 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'dm-large-push-performance' into 'master'

Load and process at most 100 commits when pushing into default branch

Closes #35531

See merge request !13132
parents 8b68b695 0e355e5c
...@@ -45,6 +45,7 @@ class GitPushService < BaseService ...@@ -45,6 +45,7 @@ class GitPushService < BaseService
elsif push_to_existing_branch? elsif push_to_existing_branch?
# Collect data for this git push # Collect data for this git push
@push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev])
process_commit_messages process_commit_messages
# Update the bare repositories info/attributes file using the contents of the default branches # Update the bare repositories info/attributes file using the contents of the default branches
...@@ -66,15 +67,21 @@ class GitPushService < BaseService ...@@ -66,15 +67,21 @@ class GitPushService < BaseService
def update_caches def update_caches
if is_default_branch? 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| @push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
commit.raw_deltas.each do |diff| commit.raw_deltas.each do |diff|
paths << diff.new_path paths << diff.new_path
end
end end
end
types = Gitlab::FileDetector.types_in_paths(paths.to_a) types = Gitlab::FileDetector.types_in_paths(paths.to_a)
end
else else
types = [] types = []
end end
...@@ -92,7 +99,7 @@ class GitPushService < BaseService ...@@ -92,7 +99,7 @@ class GitPushService < BaseService
def process_commit_messages def process_commit_messages
default = is_default_branch? 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? if commit.matches_cross_reference_regex?
ProcessCommitWorker ProcessCommitWorker
.perform_async(project.id, current_user.id, commit.to_hash, default) .perform_async(project.id, current_user.id, commit.to_hash, default)
...@@ -111,7 +118,7 @@ class GitPushService < BaseService ...@@ -111,7 +118,7 @@ class GitPushService < BaseService
EventCreateService.new.push(@project, current_user, build_push_data) EventCreateService.new.push(@project, current_user, build_push_data)
Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push)
SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks)
@project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks)
@project.execute_services(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks)
...@@ -131,7 +138,10 @@ class GitPushService < BaseService ...@@ -131,7 +138,10 @@ class GitPushService < BaseService
end end
def process_default_branch 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 # Ensure HEAD points to the default branch in case it is not master
project.change_head(branch_name) project.change_head(branch_name)
...@@ -160,7 +170,8 @@ class GitPushService < BaseService ...@@ -160,7 +170,8 @@ class GitPushService < BaseService
params[:oldrev], params[:oldrev],
params[:newrev], params[:newrev],
params[:ref], params[:ref],
push_commits) @push_commits,
commits_count: @push_commits_count)
end end
def push_to_existing_branch? def push_to_existing_branch?
......
---
title: Improve performance of large (initial) push into default branch
merge_request:
author:
...@@ -24,11 +24,11 @@ module Gitlab ...@@ -24,11 +24,11 @@ module Gitlab
# total_commits_count: Fixnum # 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) commits = Array(commits)
# Total commits count # Total commits count
commits_count = commits.size commits_count ||= commits.size
# Get latest 20 commits ASC # Get latest 20 commits ASC
commits_limited = commits.last(20) commits_limited = commits.last(20)
......
...@@ -658,8 +658,7 @@ describe GitPushService, services: true do ...@@ -658,8 +658,7 @@ describe GitPushService, services: true do
end end
it 'only schedules a limited number of commits' do it 'only schedules a limited number of commits' do
allow(service).to receive(:push_commits) service.push_commits = Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true))
.and_return(Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)))
expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times
...@@ -667,8 +666,7 @@ describe GitPushService, services: true do ...@@ -667,8 +666,7 @@ describe GitPushService, services: true do
end end
it "skips commits which don't include cross-references" do it "skips commits which don't include cross-references" do
allow(service).to receive(:push_commits) service.push_commits = [double(:commit, to_hash: {}, matches_cross_reference_regex?: false)]
.and_return([double(:commit, to_hash: {}, matches_cross_reference_regex?: false)])
expect(ProcessCommitWorker).not_to receive(:perform_async) expect(ProcessCommitWorker).not_to receive(:perform_async)
......
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