diff options
author | Stan Hu <stanhu@gmail.com> | 2019-08-13 01:31:58 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-08-13 08:28:49 +0300 |
commit | 4e2bb4e5e7df1273a4d2fdd370b6c17a27c394d8 (patch) | |
tree | 6da4006221d92fd0f059e71e0dd6e7269ab38b6a /app/services | |
parent | 6480bd6dc9f2e32bf3651606e836516cad65c2c8 (diff) |
Reduce Gitaly calls in PostReceive
This commit reduces I/O load and memory utilization during PostReceive
for the common case when no project hooks or services are set up.
We saw a Gitaly N+1 issue in `CommitDelta` when many tags or branches
are pushed. We can reduce this overhead in the common case because we
observe that most new projects do not have any Web hooks or services,
especially when they are first created. Previously, `BaseHooksService`
unconditionally iterated through the last 20 commits of each ref to
build the `push_data` structure. The `push_data` structured was used in
numerous places:
1. Building the push payload in `EventCreateService`
2. Creating a CI pipeline
3. Executing project Web or system hooks
4. Executing project services
5. As the return value of `BaseHooksService#execute`
6. `BranchHooksService#invalidated_file_types`
We only need to generate the full `push_data` for items 3, 4, and 6.
Item 1: `EventCreateService` only needs the last commit and doesn't
actually need the commit deltas.
Item 2: In addition, `Ci::CreatePipelineService` only needed a subset of
the parameters.
Item 5: The return value of `BaseHooksService#execute` also wasn't being
used anywhere.
Item 6: This is only used when pushing to the default branch, so if
many tags are pushed we can save significant I/O here.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/65878
Fic
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/git/base_hooks_service.rb | 45 |
1 files changed, 33 insertions, 12 deletions
diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index d30df34e54b..1db18fcf401 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -19,7 +19,7 @@ module Git update_remote_mirrors - push_data + success end private @@ -33,7 +33,7 @@ module Git end def limited_commits - commits.last(PROCESS_COMMIT_LIMIT) + @limited_commits ||= commits.last(PROCESS_COMMIT_LIMIT) end def commits_count @@ -48,21 +48,25 @@ module Git [] end + # Push events in the activity feed only show information for the + # last commit. def create_events - EventCreateService.new.push(project, current_user, push_data) + EventCreateService.new.push(project, current_user, event_push_data) end def create_pipelines return unless params.fetch(:create_pipelines, true) Ci::CreatePipelineService - .new(project, current_user, push_data) + .new(project, current_user, base_params) .execute(:push, pipeline_options) end def execute_project_hooks - project.execute_hooks(push_data, hook_name) - project.execute_services(push_data, hook_name) + # Creating push_data invokes one CommitDelta RPC per commit. Only + # build this data if we actually need it. + project.execute_hooks(push_data, hook_name) if project.has_active_hooks?(hook_name) + project.execute_services(push_data, hook_name) if project.has_active_services?(hook_name) end def enqueue_invalidate_cache @@ -73,18 +77,35 @@ module Git ) end - def push_data - @push_data ||= Gitlab::DataBuilder::Push.build( - project: project, - user: current_user, + def base_params + { oldrev: params[:oldrev], newrev: params[:newrev], ref: params[:ref], - commits: limited_commits, + push_options: params[:push_options] || {} + } + end + + def push_data_params(commits:, with_changed_files: true) + base_params.merge( + project: project, + user: current_user, + commits: commits, message: event_message, commits_count: commits_count, - push_options: params[:push_options] || {} + with_changed_files: with_changed_files ) + end + + def event_push_data + # We only need the last commit for the event push, and we don't + # need the full deltas either. + @event_push_data ||= Gitlab::DataBuilder::Push.build( + push_data_params(commits: commits.last, with_changed_files: false)) + end + + def push_data + @push_data ||= Gitlab::DataBuilder::Push.build(push_data_params(commits: limited_commits)) # Dependent code may modify the push data, so return a duplicate each time @push_data.dup |