Commit 2fe57a41 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'fj-37736-improve-performance-post-receive-cleaning-code' into 'master'

Code cleaning in PostReceive services

See merge request gitlab-org/gitlab-ce!20916
parents 7eba8537 8402d67a
...@@ -312,6 +312,8 @@ class Repository ...@@ -312,6 +312,8 @@ class Repository
# types - An Array of file types (e.g. `:readme`) used to refresh extra # types - An Array of file types (e.g. `:readme`) used to refresh extra
# caches. # caches.
def refresh_method_caches(types) def refresh_method_caches(types)
return if types.empty?
to_refresh = [] to_refresh = []
types.each do |type| types.each do |type|
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class GitPushService < BaseService class GitPushService < BaseService
attr_accessor :push_data, :push_commits attr_accessor :push_data, :push_commits
include Gitlab::Access include Gitlab::Access
include Gitlab::Utils::StrongMemoize
# The N most recent commits to process in a single push payload. # The N most recent commits to process in a single push payload.
PROCESS_COMMIT_LIMIT = 100 PROCESS_COMMIT_LIMIT = 100
...@@ -21,14 +22,14 @@ class GitPushService < BaseService ...@@ -21,14 +22,14 @@ class GitPushService < BaseService
# 6. Checks if the project's main language has changed # 6. Checks if the project's main language has changed
# #
def execute def execute
@project.repository.after_create if @project.empty_repo? project.repository.after_create if project.empty_repo?
@project.repository.after_push_commit(branch_name) project.repository.after_push_commit(branch_name)
if push_remove_branch? if push_remove_branch?
@project.repository.after_remove_branch project.repository.after_remove_branch
@push_commits = [] @push_commits = []
elsif push_to_new_branch? elsif push_to_new_branch?
@project.repository.after_create_branch project.repository.after_create_branch
# Re-find the pushed commits. # Re-find the pushed commits.
if default_branch? if default_branch?
...@@ -38,14 +39,14 @@ class GitPushService < BaseService ...@@ -38,14 +39,14 @@ class GitPushService < BaseService
# Use the pushed commits that aren't reachable by the default branch # Use the pushed commits that aren't reachable by the default branch
# as a heuristic. This may include more commits than are actually pushed, but # as a heuristic. This may include more commits than are actually pushed, but
# that shouldn't matter because we check for existing cross-references later. # that shouldn't matter because we check for existing cross-references later.
@push_commits = @project.repository.commits_between(@project.default_branch, params[:newrev]) @push_commits = project.repository.commits_between(project.default_branch, params[:newrev])
# don't process commits for the initial push to the default branch # don't process commits for the initial push to the default branch
process_commit_messages process_commit_messages
end end
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
...@@ -64,7 +65,7 @@ class GitPushService < BaseService ...@@ -64,7 +65,7 @@ class GitPushService < BaseService
end end
def update_gitattributes def update_gitattributes
@project.repository.copy_gitattributes(params[:ref]) project.repository.copy_gitattributes(params[:ref])
end end
def update_caches def update_caches
...@@ -88,7 +89,7 @@ class GitPushService < BaseService ...@@ -88,7 +89,7 @@ class GitPushService < BaseService
types = [] types = []
end end
ProjectCacheWorker.perform_async(@project.id, types, [:commit_count, :repository_size]) ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size])
end end
def update_signatures def update_signatures
...@@ -121,10 +122,10 @@ class GitPushService < BaseService ...@@ -121,10 +122,10 @@ class GitPushService < BaseService
protected protected
def update_remote_mirrors def update_remote_mirrors
return unless @project.has_remote_mirror? return unless project.has_remote_mirror?
@project.mark_stuck_remote_mirrors_as_failed! project.mark_stuck_remote_mirrors_as_failed!
@project.update_remote_mirrors project.update_remote_mirrors
end end
def execute_related_hooks def execute_related_hooks
...@@ -132,14 +133,14 @@ class GitPushService < BaseService ...@@ -132,14 +133,14 @@ class GitPushService < BaseService
# could cause the last commit of a merge request to change. # could cause the last commit of a merge request to change.
# #
UpdateMergeRequestsWorker UpdateMergeRequestsWorker
.perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref])
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)
if push_remove_branch? if push_remove_branch?
AfterBranchDeleteService AfterBranchDeleteService
...@@ -149,52 +150,50 @@ class GitPushService < BaseService ...@@ -149,52 +150,50 @@ class GitPushService < BaseService
end end
def perform_housekeeping def perform_housekeeping
housekeeping = Projects::HousekeepingService.new(@project) housekeeping = Projects::HousekeepingService.new(project)
housekeeping.increment! housekeeping.increment!
housekeeping.execute if housekeeping.needed? housekeeping.execute if housekeeping.needed?
rescue Projects::HousekeepingService::LeaseTaken rescue Projects::HousekeepingService::LeaseTaken
end end
def process_default_branch def process_default_branch
@push_commits_count = project.repository.commit_count_for_ref(params[:ref]) offset = [push_commits_count - PROCESS_COMMIT_LIMIT, 0].max
offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max
@push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT)
@project.after_create_default_branch project.after_create_default_branch
end end
def build_push_data def build_push_data
@push_data ||= Gitlab::DataBuilder::Push.build( @push_data ||= Gitlab::DataBuilder::Push.build(
@project, project,
current_user, current_user,
params[:oldrev], params[:oldrev],
params[:newrev], params[:newrev],
params[:ref], params[:ref],
@push_commits, @push_commits,
commits_count: @push_commits_count) commits_count: push_commits_count)
end end
def push_to_existing_branch? def push_to_existing_branch?
# Return if this is not a push to a branch (e.g. new commits) # Return if this is not a push to a branch (e.g. new commits)
Gitlab::Git.branch_ref?(params[:ref]) && !Gitlab::Git.blank_ref?(params[:oldrev]) branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev])
end end
def push_to_new_branch? def push_to_new_branch?
Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:oldrev]) strong_memoize(:push_to_new_branch) do
branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev])
end
end end
def push_remove_branch? def push_remove_branch?
Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:newrev]) strong_memoize(:push_remove_branch) do
branch_ref? && Gitlab::Git.blank_ref?(params[:newrev])
end end
def push_to_branch?
Gitlab::Git.branch_ref?(params[:ref])
end end
def default_branch? def default_branch?
Gitlab::Git.branch_ref?(params[:ref]) && branch_ref? &&
(Gitlab::Git.ref_name(params[:ref]) == project.default_branch || project.default_branch.nil?) (branch_name == project.default_branch || project.default_branch.nil?)
end end
def commit_user(commit) def commit_user(commit)
...@@ -202,7 +201,21 @@ class GitPushService < BaseService ...@@ -202,7 +201,21 @@ class GitPushService < BaseService
end end
def branch_name def branch_name
@branch_name ||= Gitlab::Git.ref_name(params[:ref]) strong_memoize(:branch_name) do
Gitlab::Git.ref_name(params[:ref])
end
end
def branch_ref?
strong_memoize(:branch_ref) do
Gitlab::Git.branch_ref?(params[:ref])
end
end
def push_commits_count
strong_memoize(:push_commits_count) do
project.repository.commit_count_for_ref(params[:ref])
end
end end
def last_pushed_commits def last_pushed_commits
......
...@@ -9,12 +9,12 @@ class GitTagPushService < BaseService ...@@ -9,12 +9,12 @@ class GitTagPushService < BaseService
@push_data = build_push_data @push_data = build_push_data
EventCreateService.new.push(project, current_user, @push_data) EventCreateService.new.push(project, current_user, push_data)
Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push)
SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks)
project.execute_hooks(@push_data.dup, :tag_push_hooks) project.execute_hooks(push_data.dup, :tag_push_hooks)
project.execute_services(@push_data.dup, :tag_push_hooks) project.execute_services(push_data.dup, :tag_push_hooks)
ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size])
......
# frozen_string_literal: true
module Gitlab module Gitlab
class GitPostReceive class GitPostReceive
include Gitlab::Identifier include Gitlab::Identifier
...@@ -14,10 +16,11 @@ module Gitlab ...@@ -14,10 +16,11 @@ module Gitlab
end end
def changes_refs def changes_refs
return enum_for(:changes_refs) unless block_given? return changes unless block_given?
changes.each do |change| changes.each do |change|
oldrev, newrev, ref = change.strip.split(' ') change.strip!
oldrev, newrev, ref = change.split(' ')
yield oldrev, newrev, ref yield oldrev, newrev, ref
end end
...@@ -26,13 +29,10 @@ module Gitlab ...@@ -26,13 +29,10 @@ module Gitlab
private private
def deserialize_changes(changes) def deserialize_changes(changes)
changes = utf8_encode_changes(changes) utf8_encode_changes(changes).each_line
changes.lines
end end
def utf8_encode_changes(changes) def utf8_encode_changes(changes)
changes = changes.dup
changes.force_encoding('UTF-8') changes.force_encoding('UTF-8')
return changes if changes.valid_encoding? return changes if changes.valid_encoding?
......
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