Commit 2d600716 authored by Stan Hu's avatar Stan Hu

Merge branch 'parallel-push-check' into 'master'

Run PushRuleCheck in parallel

See merge request gitlab-org/gitlab!45668
parents e0a0eba6 c5a2690c
---
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