Eliminate unnecessary and duplicate system hook fires

Previously `SystemHookPushWorker` would always be called after a push event,
and this would queue a Sidekiq job regardless of whether any system hooks
needed that event. Moreover, another call inside `Project#execute_hooks` would
also fire system hooks if they existed.

This change both removes the duplicate system hook calls. For installations
without system hooks for push events, this change also can save significant
amount of RAM used by Redis.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/50549
This commit is contained in:
Stan Hu 2018-08-21 13:32:50 -07:00
parent 382b6dabd9
commit 377e3670a9
3 changed files with 12 additions and 4 deletions

View file

@ -140,7 +140,6 @@ 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)

View file

@ -0,0 +1,5 @@
---
title: Eliminate unnecessary and duplicate system hook fires
merge_request: 21337
author:
type: performance

View file

@ -223,9 +223,13 @@ describe GitPushService, services: true do
end
end
context "Sends System Push data" do
it "when pushing on a branch" do
expect(SystemHookPushWorker).to receive(:perform_async).with(push_data, :push_hooks)
describe 'system hooks' do
let(:system_hook_service) { double() }
it "sends a system hook after pushing a branch" do
expect(SystemHooksService).to receive(:new).and_return(system_hook_service)
expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks)
execute_service(project, user, oldrev, newrev, ref)
end
end