Commit c0ad1c83 authored by Marc Shaw's avatar Marc Shaw

Ensure a project iid is set before transitioning on pipeline error

Issue: gitlab.com/gitlab-org/gitlab/-/issues/325861
MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/57783

We need ensure_project_iid to run without any parent transaction,
otherwise the InternalId table for that scope, in our case project,
will be locked for the entire transaction. If we error before the seed
chain method is called, then the pipeline will not have an iid set,
and the transition to drop! will have a transaction, so when we
ensure the project iid, it will be merged into the transition
transaction.
parent 4c99bf93
---
title: Ensure a project iid is set before transitioning on pipeline error
merge_request: 57783
author:
type: performance
---
name: ci_pipeline_ensure_iid_on_drop
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57783
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326886
milestone: '13.11'
type: development
group: group::code review
default_enabled: false
......@@ -12,7 +12,17 @@ module Gitlab
end
pipeline.add_error_message(message)
pipeline.drop!(drop_reason) if drop_reason && persist_pipeline?
if drop_reason && persist_pipeline?
if Feature.enabled?(:ci_pipeline_ensure_iid_on_drop, pipeline.project, default_enabled: :yaml)
# Project iid must be called outside a transaction, so we ensure it is set here
# otherwise it may be set within the state transition transaction of the drop! call
# which it will lock the InternalId row for the whole transaction
pipeline.ensure_project_iid!
end
pipeline.drop!(drop_reason)
end
# TODO: consider not to rely on AR errors directly as they can be
# polluted with other unrelated errors (e.g. state machine)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do
let(:helper_class) do
Class.new do
include Gitlab::Ci::Pipeline::Chain::Helpers
attr_accessor :pipeline, :command
def initialize(pipeline, command)
self.pipeline = pipeline
self.command = command
end
end
end
subject(:helper) { helper_class.new(pipeline, command) }
let(:pipeline) { build(:ci_empty_pipeline) }
let(:command) { double(save_incompleted: true) }
let(:message) { 'message' }
describe '.error' do
shared_examples 'error function' do
specify do
expect(pipeline).to receive(:drop!).with(drop_reason).and_call_original
expect(pipeline).to receive(:add_error_message).with(message).and_call_original
expect(pipeline).to receive(:ensure_project_iid!).twice.and_call_original
subject.error(message, config_error: config_error, drop_reason: drop_reason)
expect(pipeline.yaml_errors).to eq(yaml_error)
expect(pipeline.errors[:base]).to include(message)
end
end
context 'when given a drop reason' do
context 'when config error is true' do
context 'sets the yaml error and overrides the drop reason' do
let(:drop_reason) { :config_error }
let(:config_error) { true }
let(:yaml_error) { message }
it_behaves_like "error function"
end
end
context 'when config error is false' do
context 'does not set the yaml error or override the drop reason' do
let(:drop_reason) { :size_limit_exceeded }
let(:config_error) { false }
let(:yaml_error) { nil }
it_behaves_like "error function"
end
end
end
context 'when the ci_pipeline_ensure_iid_on_drop feature flag is false' do
it 'does not ensure the project iid' do
stub_feature_flags(ci_pipeline_ensure_iid_on_drop: false)
expect(pipeline).to receive(:ensure_project_iid!).once
subject.error(message, config_error: true)
end
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