diff --git a/app/models/concerns/ci/partitionable.rb b/app/models/concerns/ci/partitionable.rb index ec6c85d888de5a08b0448de781f50bd2db2508cc..7290d342d11bf8928bb134e3b9f0d39605ddcc40 100644 --- a/app/models/concerns/ci/partitionable.rb +++ b/app/models/concerns/ci/partitionable.rb @@ -22,6 +22,7 @@ module Partitionable module Testing InclusionError = Class.new(StandardError) + # EE only features are listed on EE::Ci::Partitionable::Testing::EE_PARTITIONABLE_MODELS PARTITIONABLE_MODELS = %w[ CommitStatus Ci::BuildMetadata @@ -44,8 +45,12 @@ module Testing Ci::UnitTestFailure ].freeze + def self.partitionable_models + PARTITIONABLE_MODELS + end + def self.check_inclusion(klass) - return if PARTITIONABLE_MODELS.include?(klass.name) + return if partitionable_models.include?(klass.name) raise Partitionable::Testing::InclusionError, "#{klass} must be included in PARTITIONABLE_MODELS" @@ -112,3 +117,5 @@ def handle_partitionable_ddl(partitioned) end end end + +Ci::Partitionable::Testing.prepend_mod diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index bfd737baec984a86a544deb655e75aeab33ccf3c..ca961b713c53f40a15c6e7c641071360c65924dc 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -17,7 +17,8 @@ IncidentManagement::PendingEscalations::Alert, IncidentManagement::PendingEscalations::Issue, Security::Finding, - Analytics::ValueStreamDashboard::Count + Analytics::ValueStreamDashboard::Count, + Ci::RunnerFailedBuild ]) else Gitlab::Database::Partitioning.register_tables( diff --git a/db/docs/p_ci_runner_failed_builds.yml b/db/docs/p_ci_runner_failed_builds.yml new file mode 100644 index 0000000000000000000000000000000000000000..2e55f604b044732d7abd95e29ae887953e784c6d --- /dev/null +++ b/db/docs/p_ci_runner_failed_builds.yml @@ -0,0 +1,9 @@ +table_name: p_ci_runner_failed_builds +classes: +- Ci::RunnerFailedBuild +feature_categories: +- runner_fleet +description: Builds failed with runner system failures +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129972 +milestone: '16.4' +gitlab_schema: gitlab_ci diff --git a/db/migrate/20230822124322_add_ci_runner_failed_builds_table.rb b/db/migrate/20230822124322_add_ci_runner_failed_builds_table.rb new file mode 100644 index 0000000000000000000000000000000000000000..d005021230270ca28800891f600bb8026e5b3875 --- /dev/null +++ b/db/migrate/20230822124322_add_ci_runner_failed_builds_table.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class AddCiRunnerFailedBuildsTable < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + + TABLE_NAME = :p_ci_runner_failed_builds + TARGET_TABLE_NAME = :ci_builds + + def up + create_table(TABLE_NAME, primary_key: [:build_id, :partition_id], + options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t| + t.bigint :partition_id, null: false + t.bigint :build_id, null: false + t.timestamp :finished_at, null: false # rubocop:disable Migration/Datetime + + t.index [:finished_at, :partition_id, :build_id], unique: true, + name: :idx_p_ci_runner_failed_builds_on_finished_at_and_build_id + end + + add_concurrent_partitioned_foreign_key( + TABLE_NAME, TARGET_TABLE_NAME, + column: [:partition_id, :build_id], + target_column: [:partition_id, :id], + on_update: :cascade, + on_delete: :cascade, + reverse_lock_order: true + ) + end + + def down + drop_table TABLE_NAME + end +end diff --git a/db/migrate/20230822134352_create_initial_ci_runner_failed_builds_partition.rb b/db/migrate/20230822134352_create_initial_ci_runner_failed_builds_partition.rb new file mode 100644 index 0000000000000000000000000000000000000000..eb6478ac8d8bd2fb776c397e15e640edbbe12f50 --- /dev/null +++ b/db/migrate/20230822134352_create_initial_ci_runner_failed_builds_partition.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class CreateInitialCiRunnerFailedBuildsPartition < Gitlab::Database::Migration[2.1] + PARTITION_NAME = 'gitlab_partitions_dynamic.ci_runner_failed_builds_100' + TABLE_NAME = 'p_ci_runner_failed_builds' + FIRST_PARTITION = 100 + BUILDS_TABLE = 'p_ci_builds' + + disable_ddl_transaction! + + def up + with_lock_retries(**lock_args) do + connection.execute(<<~SQL) + LOCK TABLE #{BUILDS_TABLE} IN SHARE UPDATE EXCLUSIVE MODE; + LOCK TABLE ONLY #{TABLE_NAME} IN ACCESS EXCLUSIVE MODE; + SQL + + connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS #{PARTITION_NAME} + PARTITION OF #{TABLE_NAME} + FOR VALUES IN (#{FIRST_PARTITION}); + SQL + end + end + + private + + def lock_args + { + raise_on_exhaustion: true, + timing_configuration: lock_timing_configuration + } + end + + def lock_timing_configuration + iterations = Gitlab::Database::WithLockRetries::DEFAULT_TIMING_CONFIGURATION + aggressive_iterations = Array.new(5) { [10.seconds, 1.minute] } + + iterations + aggressive_iterations + end +end diff --git a/db/schema_migrations/20230822124322 b/db/schema_migrations/20230822124322 new file mode 100644 index 0000000000000000000000000000000000000000..a5445b335d726bc55f627612706923d8a0cf0c17 --- /dev/null +++ b/db/schema_migrations/20230822124322 @@ -0,0 +1 @@ +7197aace86bce6a0e9ea337d81b41c7ad548f26d5633e575e66a24aba47a3b9e \ No newline at end of file diff --git a/db/schema_migrations/20230822134352 b/db/schema_migrations/20230822134352 new file mode 100644 index 0000000000000000000000000000000000000000..c164fb27e9606848eb1554df40f2b7cf2d6e5c02 --- /dev/null +++ b/db/schema_migrations/20230822134352 @@ -0,0 +1 @@ +3cdf4d7c9c05b1c2d60fa2a77ae361c2a0d5db5ca487261d2b17ab615ee84c6b \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 2f9599678266b76f068c6a50215eb85f2dccc868..b3a6a4330f2362e912fa07744818fa1ea80d2713 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -556,6 +556,13 @@ CREATE TABLE p_ci_job_annotations ( ) PARTITION BY LIST (partition_id); +CREATE TABLE p_ci_runner_failed_builds ( + partition_id bigint NOT NULL, + build_id bigint NOT NULL, + finished_at timestamp without time zone NOT NULL +) +PARTITION BY LIST (partition_id); + CREATE TABLE p_ci_runner_machine_builds ( partition_id bigint NOT NULL, build_id bigint NOT NULL, @@ -28299,6 +28306,9 @@ ALTER TABLE ONLY p_batched_git_ref_updates_deletions ALTER TABLE ONLY p_ci_job_annotations ADD CONSTRAINT p_ci_job_annotations_pkey PRIMARY KEY (id, partition_id); +ALTER TABLE ONLY p_ci_runner_failed_builds + ADD CONSTRAINT p_ci_runner_failed_builds_pkey PRIMARY KEY (build_id, partition_id); + ALTER TABLE ONLY p_ci_runner_machine_builds ADD CONSTRAINT p_ci_runner_machine_builds_pkey PRIMARY KEY (build_id, partition_id); @@ -30478,6 +30488,8 @@ CREATE UNIQUE INDEX idx_on_external_status_checks_project_id_name ON external_st CREATE INDEX idx_open_issues_on_project_and_confidential_and_author_and_id ON issues USING btree (project_id, confidential, author_id, id) WHERE (state_id = 1); +CREATE UNIQUE INDEX idx_p_ci_runner_failed_builds_on_finished_at_and_build_id ON ONLY p_ci_runner_failed_builds USING btree (finished_at, partition_id, build_id); + CREATE INDEX idx_packages_debian_group_component_files_on_architecture_id ON packages_debian_group_component_files USING btree (architecture_id); CREATE INDEX idx_packages_debian_project_component_files_on_architecture_id ON packages_debian_project_component_files USING btree (architecture_id); @@ -38195,6 +38207,9 @@ ALTER TABLE ONLY badges ALTER TABLE ONLY vulnerability_finding_signatures ADD CONSTRAINT fk_rails_9e0baf9dcd FOREIGN KEY (finding_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE; +ALTER TABLE p_ci_runner_failed_builds + ADD CONSTRAINT fk_rails_9e8915bcec FOREIGN KEY (partition_id, build_id) REFERENCES ci_builds(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE; + ALTER TABLE ONLY target_branch_rules ADD CONSTRAINT fk_rails_9e9cf81c8e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/ci/runner_failed_build.rb b/ee/app/models/ci/runner_failed_build.rb new file mode 100644 index 0000000000000000000000000000000000000000..9f7e13991e7407f738f9e582a3ddea741a7b79c0 --- /dev/null +++ b/ee/app/models/ci/runner_failed_build.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Ci + class RunnerFailedBuild < Ci::ApplicationRecord + include Ci::Partitionable + + self.table_name = :p_ci_runner_failed_builds + self.primary_key = :build_id + + partitionable scope: :build, partitioned: true + + belongs_to :build, class_name: 'Ci::Build' + + validates :build, presence: true + validates :finished_at, presence: true + end +end diff --git a/ee/app/models/concerns/ee/ci/partitionable.rb b/ee/app/models/concerns/ee/ci/partitionable.rb new file mode 100644 index 0000000000000000000000000000000000000000..4e4fce0bb42817418bd301d6d2e94f0cc7e1be2f --- /dev/null +++ b/ee/app/models/concerns/ee/ci/partitionable.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module EE + module Ci + module Partitionable + extend ActiveSupport::Concern + + module Testing + extend ActiveSupport::Concern + + EE_PARTITIONABLE_MODELS = %w[ + Ci::RunnerFailedBuild + ].freeze + + class_methods do + include ::Gitlab::Utils::StrongMemoize + extend ::Gitlab::Utils::Override + + override :partitionable_models + def partitionable_models + super + EE_PARTITIONABLE_MODELS + end + strong_memoize_attr :partitionable_models + end + end + end + end +end diff --git a/ee/spec/factories/ci/runner_failed_builds.rb b/ee/spec/factories/ci/runner_failed_builds.rb new file mode 100644 index 0000000000000000000000000000000000000000..18eec1639267aa3cee3488abee616cc07ecc7568 --- /dev/null +++ b/ee/spec/factories/ci/runner_failed_builds.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :ci_runner_failed_build, class: 'Ci::RunnerFailedBuild' do + build factory: :ci_build, + scheduling_type: :dag, + status: :failed, + failure_reason: :runner_system_failure, + started_at: 'Di 29. Okt 09:51:28 CET 2013', + finished_at: 'Di 29. Okt 09:53:28 CET 2013' + + finished_at { build&.finished_at } + end +end diff --git a/ee/spec/models/ci/runner_failed_build_spec.rb b/ee/spec/models/ci/runner_failed_build_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..225facba784f643703d7be18adb0901db09de0f8 --- /dev/null +++ b/ee/spec/models/ci/runner_failed_build_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::RunnerFailedBuild, model: true, feature_category: :runner_fleet do + it { is_expected.to belong_to(:build) } + + describe 'validation' do + it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:finished_at) } + end + + describe 'partitioning' do + context 'with build' do + let(:build) { FactoryBot.build(:ci_build, partition_id: ci_testing_partition_id) } + let(:runner_failed_build) { FactoryBot.build(:ci_runner_failed_build, build: build) } + + it 'sets partition_id to the current partition value' do + expect { runner_failed_build.valid? }.to change { runner_failed_build.partition_id } + .to(ci_testing_partition_id) + end + + context 'when it is already set' do + let(:runner_failed_build) { FactoryBot.build(:ci_runner_failed_build, partition_id: 125) } + + it 'does not change the partition_id value' do + expect { runner_failed_build.valid? }.not_to change { runner_failed_build.partition_id } + end + end + end + + context 'without build' do + let(:runner_failed_build) { FactoryBot.build(:ci_runner_failed_build, build: nil) } + + it { is_expected.to validate_presence_of(:partition_id) } + + it 'does not change the partition_id value' do + expect { runner_failed_build.valid? }.not_to change { runner_failed_build.partition_id } + end + end + end + + describe 'ci_sliding_list partitioning' do + let(:connection) { described_class.connection } + let(:partition_manager) { Gitlab::Database::Partitioning::PartitionManager.new(described_class) } + + let(:partitioning_strategy) { described_class.partitioning_strategy } + + it { expect(partitioning_strategy.missing_partitions).to be_empty } + it { expect(partitioning_strategy.extra_partitions).to be_empty } + it { expect(partitioning_strategy.current_partitions).to include partitioning_strategy.initial_partition } + it { expect(partitioning_strategy.active_partition).to be_present } + end +end diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index fbd9c4d32ccadff62b0674209af1e33490048d92..9efa36016f71e5756a6eeb1f204d571c04e5f076 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -359,11 +359,11 @@ def get_schemas context 'for CI partitioned table' do # Check that each partitionable model with more than 1 column has the partition_id column at the trailing - # position. Using PARTITIONABLE_MODELS instead of iterating tables since when partitioning existing tables, + # position. Using partitionable_models instead of iterating tables since when partitioning existing tables, # the routing table only gets created after the PK has already been created, which would be too late for a check. skip_tables = %w[] - partitionable_models = Ci::Partitionable::Testing::PARTITIONABLE_MODELS + partitionable_models = Ci::Partitionable::Testing.partitionable_models (partitionable_models - skip_tables).each do |klass| model = klass.safe_constantize table_name = model.table_name diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 5bbb95b3ea5f6aa0499c6f92bf6659f9ad8a85ae..5c5aa546c84dcc94c435c0345e1d6bb96199d7df 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -420,6 +420,7 @@ builds: - job_artifacts_requirements_v2 - runner_manager - runner_manager_build +- runner_failed_build - runner_session - trace_metadata - terraform_state_versions diff --git a/spec/models/concerns/ci/partitionable/switch_spec.rb b/spec/models/concerns/ci/partitionable/switch_spec.rb index 0041a33e50efba75602663fa3d3bebe0cf1f201b..d2bbc9ddf294c4516c0bc3e1383034788f2e5d0e 100644 --- a/spec/models/concerns/ci/partitionable/switch_spec.rb +++ b/spec/models/concerns/ci/partitionable/switch_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Partitionable::Switch, :aggregate_failures do +RSpec.describe Ci::Partitionable::Switch, :aggregate_failures, feature_category: :database do let(:model) do Class.new(Ci::ApplicationRecord) do self.primary_key = :id @@ -55,7 +55,7 @@ def self.name CREATE TABLE _test_ci_jobs(id serial NOT NULL PRIMARY KEY); SQL - stub_const('Ci::Partitionable::Testing::PARTITIONABLE_MODELS', [model.name]) + allow(Ci::Partitionable::Testing).to receive(:partitionable_models).and_return([model.name]) model.include(Ci::Partitionable) diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index 6daafc78cffcefcadee0c0dda866335d2e8e2c28..f53e8f22b54138d2f5eadff3a6aebff8b24c24ef 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Partitionable do +RSpec.describe Ci::Partitionable, feature_category: :database do let(:ci_model) { Class.new(Ci::ApplicationRecord) } describe 'partitionable models inclusion' do @@ -15,7 +15,7 @@ context 'when is included in the models list' do before do - stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + allow(described_class::Testing).to receive(:partitionable_models).and_return([ci_model.name]) end it 'does not raise exceptions' do @@ -31,7 +31,7 @@ stub_env('DISABLE_PARTITIONABLE_SWITCH', disable_partitionable_switch) allow(ActiveSupport::DescendantsTracker).to receive(:store_inherited) - stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + allow(described_class::Testing).to receive(:partitionable_models).and_return([ci_model.name]) ci_model.include(described_class) ci_model.partitionable scope: ->(r) { 1 }, @@ -53,7 +53,7 @@ context 'with partitioned options' do before do - stub_const("#{described_class}::Testing::PARTITIONABLE_MODELS", [ci_model.name]) + allow(described_class::Testing).to receive(:partitionable_models).and_return([ci_model.name]) ci_model.include(described_class) ci_model.partitionable scope: ->(r) { 1 }, partitioned: partitioned diff --git a/spec/support/helpers/models/ci/partitioning_testing/cascade_check.rb b/spec/support/helpers/models/ci/partitioning_testing/cascade_check.rb index 81c2d2cb2250fc7e1463bf8ca8fee02e51ca3651..7bcb8e5fcacba1dde3b687a31d1484d39bc66c5b 100644 --- a/spec/support/helpers/models/ci/partitioning_testing/cascade_check.rb +++ b/spec/support/helpers/models/ci/partitioning_testing/cascade_check.rb @@ -25,7 +25,7 @@ def _bulk_insert_callback_allowed?(name, args) end end -Ci::Partitionable::Testing::PARTITIONABLE_MODELS.each do |klass| +Ci::Partitionable::Testing.partitionable_models.each do |klass| next if klass == 'Ci::Pipeline' model = klass.safe_constantize diff --git a/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb b/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb index a47aaffdb43731767876cfe13ee74cfbcabd61fa..9dbe2f043cb8b7f9ed9c07da947c89a9be1c1fa8 100644 --- a/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb +++ b/spec/support/helpers/models/ci/partitioning_testing/schema_helpers.rb @@ -28,7 +28,7 @@ def teardown(connection: Ci::ApplicationRecord.connection) end def each_partitionable_table - ::Ci::Partitionable::Testing::PARTITIONABLE_MODELS.each do |klass| + ::Ci::Partitionable::Testing.partitionable_models.each do |klass| model = klass.safe_constantize table_name = model.table_name.delete_prefix('p_')