Commit 7a5acdfd authored by Adam Hegyi's avatar Adam Hegyi

Ignore Ci::Build 2PC offenses with Deployment

This change adds checks for 2PC offenses in the Ci:Build state machine
transitions.
parent 78684055
...@@ -306,7 +306,9 @@ module Ci ...@@ -306,7 +306,9 @@ module Ci
end end
after_transition pending: :running do |build| after_transition pending: :running do |build|
build.deployment&.run Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338867') do
build.deployment&.run
end
build.run_after_commit do build.run_after_commit do
build.pipeline.persistent_ref.create build.pipeline.persistent_ref.create
...@@ -328,7 +330,9 @@ module Ci ...@@ -328,7 +330,9 @@ module Ci
end end
after_transition any => [:success] do |build| after_transition any => [:success] do |build|
build.deployment&.succeed Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338867') do
build.deployment&.succeed
end
build.run_after_commit do build.run_after_commit do
BuildSuccessWorker.perform_async(id) BuildSuccessWorker.perform_async(id)
...@@ -341,7 +345,9 @@ module Ci ...@@ -341,7 +345,9 @@ module Ci
next unless build.deployment next unless build.deployment
begin begin
build.deployment.drop! Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338867') do
build.deployment.drop!
end
rescue StandardError => e rescue StandardError => e
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, build_id: build.id) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, build_id: build.id)
end end
...@@ -362,10 +368,12 @@ module Ci ...@@ -362,10 +368,12 @@ module Ci
end end
after_transition any => [:skipped, :canceled] do |build, transition| after_transition any => [:skipped, :canceled] do |build, transition|
if transition.to_name == :skipped Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338867') do
build.deployment&.skip if transition.to_name == :skipped
else build.deployment&.skip
build.deployment&.cancel else
build.deployment&.cancel
end
end end
end end
end end
......
...@@ -147,9 +147,16 @@ module Gitlab ...@@ -147,9 +147,16 @@ module Gitlab
# spec/support/database/prevent_cross_joins.rb # spec/support/database/prevent_cross_joins.rb
end end
# This method will allow cross database modifications within the block
# Example:
#
# allow_cross_database_modification_within_transaction(url: 'url-to-an-issue') do
# create(:build) # inserts ci_build and project record in one transaction
# end
def self.allow_cross_database_modification_within_transaction(url:) def self.allow_cross_database_modification_within_transaction(url:)
# this method is implemented in: # this method will be overridden in:
# spec/support/database/cross_database_modification_check.rb # spec/support/database/cross_database_modification_check.rb
yield
end end
def self.add_post_migrate_path_to_rails(force: false) def self.add_post_migrate_path_to_rails(force: false)
......
...@@ -1307,7 +1307,9 @@ RSpec.describe Ci::Build do ...@@ -1307,7 +1307,9 @@ RSpec.describe Ci::Build do
shared_examples_for 'avoid deadlock' do shared_examples_for 'avoid deadlock' do
it 'executes UPDATE in the right order' do it 'executes UPDATE in the right order' do
recorded = ActiveRecord::QueryRecorder.new { subject } recorded = with_cross_database_modification_prevented do
ActiveRecord::QueryRecorder.new { subject }
end
index_for_build = recorded.log.index { |l| l.include?("UPDATE \"ci_builds\"") } index_for_build = recorded.log.index { |l| l.include?("UPDATE \"ci_builds\"") }
index_for_deployment = recorded.log.index { |l| l.include?("UPDATE \"deployments\"") } index_for_deployment = recorded.log.index { |l| l.include?("UPDATE \"deployments\"") }
...@@ -1322,7 +1324,9 @@ RSpec.describe Ci::Build do ...@@ -1322,7 +1324,9 @@ RSpec.describe Ci::Build do
it_behaves_like 'avoid deadlock' it_behaves_like 'avoid deadlock'
it 'transits deployment status to running' do it 'transits deployment status to running' do
subject with_cross_database_modification_prevented do
subject
end
expect(deployment).to be_running expect(deployment).to be_running
end end
...@@ -1340,7 +1344,9 @@ RSpec.describe Ci::Build do ...@@ -1340,7 +1344,9 @@ RSpec.describe Ci::Build do
it_behaves_like 'calling proper BuildFinishedWorker' it_behaves_like 'calling proper BuildFinishedWorker'
it 'transits deployment status to success' do it 'transits deployment status to success' do
subject with_cross_database_modification_prevented do
subject
end
expect(deployment).to be_success expect(deployment).to be_success
end end
...@@ -1353,7 +1359,9 @@ RSpec.describe Ci::Build do ...@@ -1353,7 +1359,9 @@ RSpec.describe Ci::Build do
it_behaves_like 'calling proper BuildFinishedWorker' it_behaves_like 'calling proper BuildFinishedWorker'
it 'transits deployment status to failed' do it 'transits deployment status to failed' do
subject with_cross_database_modification_prevented do
subject
end
expect(deployment).to be_failed expect(deployment).to be_failed
end end
...@@ -1365,7 +1373,9 @@ RSpec.describe Ci::Build do ...@@ -1365,7 +1373,9 @@ RSpec.describe Ci::Build do
it_behaves_like 'avoid deadlock' it_behaves_like 'avoid deadlock'
it 'transits deployment status to skipped' do it 'transits deployment status to skipped' do
subject with_cross_database_modification_prevented do
subject
end
expect(deployment).to be_skipped expect(deployment).to be_skipped
end end
...@@ -1378,7 +1388,9 @@ RSpec.describe Ci::Build do ...@@ -1378,7 +1388,9 @@ RSpec.describe Ci::Build do
it_behaves_like 'calling proper BuildFinishedWorker' it_behaves_like 'calling proper BuildFinishedWorker'
it 'transits deployment status to canceled' do it 'transits deployment status to canceled' do
subject with_cross_database_modification_prevented do
subject
end
expect(deployment).to be_canceled expect(deployment).to be_canceled
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