diff --git a/app/models/ci/job_definition.rb b/app/models/ci/job_definition.rb index 09983642c8266871b8211a2b55d67bf65874ff3f..442f3a6f42ce0590ff139f4f33e42a8219eea16f 100644 --- a/app/models/ci/job_definition.rb +++ b/app/models/ci/job_definition.rb @@ -36,6 +36,7 @@ class JobDefinition < Ci::ApplicationRecord scope :for_project, ->(project_id) { where(project_id: project_id) } scope :for_checksum, ->(checksum) { where(checksum: checksum) } + scope :with_interruptible_true, -> { where(interruptible: true) } def self.fabricate(config:, project_id:, partition_id:) sanitized_config, checksum = sanitize_and_checksum(config) diff --git a/app/models/ci/job_definition_instance.rb b/app/models/ci/job_definition_instance.rb index 7eba50d56a9589ba198153bda9309759bdbfcbb5..38777dd4cb85db5d6ebc49ee671ddb6288f719c6 100644 --- a/app/models/ci/job_definition_instance.rb +++ b/app/models/ci/job_definition_instance.rb @@ -22,5 +22,10 @@ class JobDefinitionInstance < Ci::ApplicationRecord validates :project, presence: true validates :job, presence: true validates :job_definition, presence: true + + scope :scoped_job, -> do + where(arel_table[:job_id].eq(Ci::Processable.arel_table[:id])) + .where(arel_table[:partition_id].eq(Ci::Processable.arel_table[:partition_id])) + end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0c87cd253c9d84f6ed6024d640c1ec90ee484190..585ec072cf892b19a75b5d4a301e02e7802b1b52 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -474,9 +474,15 @@ class Pipeline < Ci::ApplicationRecord where_exists(Ci::Build.latest.scoped_pipeline.with_artifacts(reports_scope)) end + scope :legacy_conservative_interruptible, -> do + where_not_exists( + Ci::Build.scoped_pipeline.with_status(STARTED_STATUSES).with_metadata_interruptible_false + ) + end + scope :conservative_interruptible, -> do where_not_exists( - Ci::Build.scoped_pipeline.with_status(STARTED_STATUSES).not_interruptible + Ci::Build.scoped_pipeline.with_status(STARTED_STATUSES).with_interruptible_false ) end diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 05d9bbba7a61ab8ec3ab811014d6ea10620e5af7..b47e9b6ac938cc353a886f8507bf3002d8b30f74 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -67,16 +67,34 @@ class Processable < ::CommitStatus where('NOT EXISTS (?)', needs) end - scope :interruptible, -> do + scope :with_metadata_interruptible_true, -> do joins(:metadata).merge(Ci::BuildMetadata.with_interruptible) end - scope :not_interruptible, -> do + scope :with_interruptible_true, -> do + where_exists( + Ci::JobDefinitionInstance + .joins(:job_definition) + .scoped_job + .merge(Ci::JobDefinition.with_interruptible_true) + ) + end + + scope :with_metadata_interruptible_false, -> do joins(:metadata).where.not( Ci::BuildMetadata.table_name => { id: Ci::BuildMetadata.scoped_build.with_interruptible.select(:id) } ) end + scope :with_interruptible_false, -> do + where_not_exists( + Ci::JobDefinitionInstance + .joins(:job_definition) + .scoped_job + .merge(Ci::JobDefinition.with_interruptible_true) + ) + end + # The run after commit queue is processed LIFO # We need to ensure that the Redis data is persisted before any other callbacks the might depend on it. before_commit do |job| diff --git a/app/services/ci/cancel_pipeline_service.rb b/app/services/ci/cancel_pipeline_service.rb index 493b33eb4167fc2ebc12204077139d09b71e1cc1..c69b6c91c434f7892d896f7c0e6d6d942e5646dc 100644 --- a/app/services/ci/cancel_pipeline_service.rb +++ b/app/services/ci/cancel_pipeline_service.rb @@ -48,7 +48,14 @@ def force_execute if @safe_cancellation # Only build and bridge (trigger) jobs can be interruptible. # We do not cancel GenericCommitStatuses because they can't have the `interruptible` attribute. - cancel_jobs(pipeline.processables.cancelable.interruptible) + jobs = pipeline.processables.cancelable + jobs = if Feature.enabled?(:ci_read_interruptible_from_job_definitions, pipeline.project) + jobs.with_interruptible_true + else + jobs.with_metadata_interruptible_true + end + + cancel_jobs(jobs) else cancel_jobs(pipeline.cancelable_statuses) end diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 219374217bf52e71f69ad5dd11722a07affd68cf..b684ab9b0d6fa56af974d8cf0ea362698b32e065 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -121,7 +121,14 @@ def configured_cancellation_for(cancelable) def conservative_cancelable_pipeline_pks cancelable_status_pipeline_pks.each_slice(PK_BATCH_SIZE).with_object([]) do |pks_batch, conservative_pks| - conservative_pks.concat(::Ci::Pipeline.primary_key_in(pks_batch).conservative_interruptible.pluck_primary_key) + pipelines = ::Ci::Pipeline.primary_key_in(pks_batch) + pipelines = if Feature.enabled?(:ci_read_interruptible_from_job_definitions, project) + pipelines.conservative_interruptible + else + pipelines.legacy_conservative_interruptible + end + + conservative_pks.concat(pipelines.pluck_primary_key) end end strong_memoize_attr :conservative_cancelable_pipeline_pks diff --git a/config/feature_flags/wip/ci_read_interruptible_from_job_definitions.yml b/config/feature_flags/wip/ci_read_interruptible_from_job_definitions.yml new file mode 100644 index 0000000000000000000000000000000000000000..25bc7e8b43b15896cd300ecdfd5e0a28236e80d6 --- /dev/null +++ b/config/feature_flags/wip/ci_read_interruptible_from_job_definitions.yml @@ -0,0 +1,10 @@ +--- +name: ci_read_interruptible_from_job_definitions +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/552060 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/203541 +rollout_issue_url: +milestone: '18.4' +group: group::ci platform +type: wip +default_enabled: false diff --git a/spec/services/ci/cancel_pipeline_service_spec.rb b/spec/services/ci/cancel_pipeline_service_spec.rb index 3c2d4f8ffcf88dfdf1959098486f46644146b01a..64378cc1d8b723b4908a4f52b90f746871f75026 100644 --- a/spec/services/ci/cancel_pipeline_service_spec.rb +++ b/spec/services/ci/cancel_pipeline_service_spec.rb @@ -111,6 +111,25 @@ %w[bridge3 success] ]) end + + context 'when using the metadata queries' do + before do + stub_feature_flags(ci_read_interruptible_from_job_definitions: false) + end + + it 'cancels only interruptible jobs' do + expect(response).to be_success + expect(pipeline.all_jobs.pluck(:name, :status)).to match_array([ + %w[build1 running], + %w[build2 created], + %w[build3 success], + %w[build4 canceled], + %w[bridge1 running], + %w[bridge2 canceled], + %w[bridge3 success] + ]) + end + end end context 'when pipeline has child pipelines' do diff --git a/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb b/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb index fa3d58baea8239823d60d5fe1019eba9a8683251..fd314bb1032493faad2f659956cd067d1f4fcfca 100644 --- a/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb +++ b/spec/services/ci/pipeline_creation/cancel_redundant_pipelines_service_spec.rb @@ -74,6 +74,19 @@ ) end + context 'when using the metadata queries' do + before do + stub_feature_flags(ci_read_interruptible_from_job_definitions: false) + end + + it 'cancels only previous non started builds' do + execute + + expect(build_statuses(prev_pipeline)).to contain_exactly('canceled', 'success', 'canceled') + expect(build_statuses(pipeline)).to contain_exactly('pending') + end + end + context 'when the previous pipeline is running on the same SHA' do # This setup specifies the SHA for clarity, but every # FactoryBot Pipeline record has the same hardcoded SHA