From ae8fa1190f55c32b3451edcf841c7fddc319a074 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Tue, 2 Sep 2025 17:27:43 +0300 Subject: [PATCH] Add new job interruptible scopes --- app/models/ci/job_definition.rb | 1 + app/models/ci/job_definition_instance.rb | 5 +++++ app/models/ci/pipeline.rb | 8 ++++++- app/models/ci/processable.rb | 22 +++++++++++++++++-- app/services/ci/cancel_pipeline_service.rb | 9 +++++++- .../cancel_redundant_pipelines_service.rb | 9 +++++++- ...ead_interruptible_from_job_definitions.yml | 10 +++++++++ .../ci/cancel_pipeline_service_spec.rb | 19 ++++++++++++++++ ...cancel_redundant_pipelines_service_spec.rb | 13 +++++++++++ 9 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 config/feature_flags/wip/ci_read_interruptible_from_job_definitions.yml diff --git a/app/models/ci/job_definition.rb b/app/models/ci/job_definition.rb index 09983642c82668..442f3a6f42ce05 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 7eba50d56a9589..38777dd4cb85db 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 0c87cd253c9d84..585ec072cf892b 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 05d9bbba7a61ab..b47e9b6ac938cc 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 493b33eb4167fc..c69b6c91c434f7 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 219374217bf52e..b684ab9b0d6fa5 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 00000000000000..25bc7e8b43b158 --- /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 3c2d4f8ffcf88d..64378cc1d8b723 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 fa3d58baea8239..fd314bb1032493 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 -- GitLab