Commit c5a2690c authored by Robert May's avatar Robert May Committed by Stan Hu

Run PushRuleCheck in parallel

Ensures the two push rule checks inside .validate! run
in parallel as they both feature potentially-slow IO
when loading data from Gitaly.
parent cf605972
---
title: Run PushRuleCheck in parallel
merge_request: 45668
author:
type: performance
---
name: parallel_push_checks
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45668
rollout_issue_url:
type: ops
group: group::source_code
default_enabled: false
......@@ -7,14 +7,86 @@ module EE
def validate!
return unless push_rule
if ::Feature.enabled?(:parallel_push_checks, project, type: :ops)
run_checks_in_parallel!
else
run_checks_in_sequence!
end
end
private
# @return [Nil] returns nil unless an error is raised
# @raise [Gitlab::GitAccess::ForbiddenError] if check fails
def check_tag_or_branch!
if tag_name
PushRules::TagCheck.new(change_access).validate!
else
PushRules::BranchCheck.new(change_access).validate!
end
end
# @return [Nil] returns nil unless an error is raised
# @raise [Gitlab::GitAccess::ForbiddenError] if check fails
def check_file_size!
PushRules::FileSizeCheck.new(change_access).validate!
end
# Run the checks one after the other.
#
# @return [Nil] returns nil unless an error is raised
# @raise [Gitlab::GitAccess::ForbiddenError] if any check fails
def run_checks_in_sequence!
check_tag_or_branch!
check_file_size!
end
# Run the checks in separate threads for performance benefits
#
# @return [Nil] returns nil unless an error is raised
# @raise [Gitlab::GitAccess::ForbiddenError] if any check fails
def run_checks_in_parallel!
@threads = []
parallelize do
check_tag_or_branch!
end
parallelize do
check_file_size!
end
# Block whilst waiting for threads, however if one errors
# it will exit early and raise the error immediately as
# we set `abort_on_exception` to true.
@threads.each(&:join)
nil
ensure
# We want to make sure no threads are left dangling.
# Threads can exit early when an exception is raised
# and so we want to ensure any still running are exited
# as soon as possible.
@threads.each(&:exit)
end
# Runs a block inside a new thread. This thread will
# exit immediately upon an exception being raised.
#
# @raise [Gitlab::GitAccess::ForbiddenError]
def parallelize
@threads << Thread.new do
Thread.current.tap do |t|
t.name = "push_rule_check"
t.abort_on_exception = true
t.report_on_exception = false
end
yield
ensure # rubocop: disable Layout/RescueEnsureAlignment
ActiveRecord::Base.clear_active_connections!
end
end
end
end
end
......
......@@ -7,10 +7,24 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
let(:push_rule) { create(:push_rule, :commit_message) }
describe '#validate!' do
shared_examples "push checks" do
before do
expect_any_instance_of(EE::Gitlab::Checks::PushRules::FileSizeCheck)
.to receive(:validate!)
allow_any_instance_of(EE::Gitlab::Checks::PushRules::FileSizeCheck)
.to receive(:validate!).and_return(nil)
allow_any_instance_of(EE::Gitlab::Checks::PushRules::TagCheck)
.to receive(:validate!).and_return(nil)
allow_any_instance_of(EE::Gitlab::Checks::PushRules::BranchCheck)
.to receive(:validate!).and_return(nil)
end
it "returns nil on success" do
expect(subject.validate!).to be_nil
end
it "raises an error on failure" do
expect_any_instance_of(EE::Gitlab::Checks::PushRules::FileSizeCheck).to receive(:validate!).and_raise(Gitlab::GitAccess::ForbiddenError)
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError)
end
context 'when tag name exists' do
......@@ -43,4 +57,16 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
end
end
end
describe '#validate!' do
it_behaves_like "push checks"
context ":parallel_push_checks feature is disabled" do
before do
stub_feature_flags(parallel_push_checks: false)
end
it_behaves_like "push checks"
end
end
end
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