Merge branch 'pedropombeiro/459999/2-add-and-backfill-project_id' into 'master'

Add and backfill project_id for p_ci_runner_machine_builds

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164386



Merged-by: Max Fan <mfan@gitlab.com>
Approved-by: Krasimir Angelov <kangelov@gitlab.com>
Approved-by: Max Fan <mfan@gitlab.com>
Reviewed-by: Jarka Košanová <jarka@gitlab.com>
Reviewed-by: Pedro Pombeiro <noreply@pedro.pombei.ro>
Reviewed-by: Krasimir Angelov <kangelov@gitlab.com>
Reviewed-by: Tianwen Chen <tchen@gitlab.com>
Reviewed-by: Dzmitry (Dima) Meshcharakou <12459192-dmeshcharakou@users.noreply.gitlab.com>
Reviewed-by: Max Fan <mfan@gitlab.com>
Co-authored-by: Pedro Pombeiro <noreply@pedro.pombei.ro>

app/models/ci/runner_manager_build.rb

@@ -12,12 +12,15 @@ class RunnerManagerBuild < Ci::ApplicationRecord
     alias_attribute :runner_manager_id, :runner_machine_id
+    before_validation :ensure_project_id, on: :create
+
     belongs_to :build, inverse_of: :runner_manager_build, class_name: 'Ci::Build'
     belongs_to :runner_manager, foreign_key: :runner_machine_id, inverse_of: :runner_manager_builds,
       class_name: 'Ci::RunnerManager'
     validates :build, presence: true
     validates :runner_manager, presence: true
+    validates :project_id, presence: true, on: :create
     scope :for_build, ->(build_id) { where(build_id: build_id) }
@@ -26,5 +29,11 @@ def self.pluck_build_id_and_runner_manager_id
         .pluck(:build_id, :runner_manager_id)
         .to_h
     end
+
+    private
+
+    def ensure_project_id
+      self.project_id ||= build&.project_id
+    end
   end
 end

db/docs/batched_background_migrations/backfill_p_ci_runner_machine_builds_project_id.yml

+---
+migration_job_name: BackfillPCiRunnerMachineBuildsProjectId
+description: Backfills sharding key `p_ci_runner_machine_builds.project_id` from `p_ci_builds`.
+feature_category: runner
+introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164386'
+milestone: '17.4'
+queued_migration_version: 20240829160537
+finalize_after: '2024-10-15'
+finalized_by: # version of the migration that finalized this BBM

db/docs/p_ci_runner_machine_builds.yml

@@ -4,7 +4,7 @@ classes:
 feature_categories:
 - fleet_visibility
 description: Relationships between builds and runner managers
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111476
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111484
 milestone: '15.9'
 gitlab_schema: gitlab_ci
 desired_sharding_key:
@@ -17,3 +17,4 @@ desired_sharding_key:
         sharding_key: project_id
         belongs_to: build
         foreign_key_name: fk_bb490f12fe_p
+desired_sharding_key_migration_job_name: BackfillPCiRunnerMachineBuildsProjectId

db/migrate/20240829160016_add_project_id_to_p_ci_runner_manager_builds.rb

+# frozen_string_literal: true
+
+class AddProjectIdToPCiRunnerManagerBuilds < Gitlab::Database::Migration[2.2]
+  milestone '17.4'
+
+  def change
+    add_column(:p_ci_runner_machine_builds, :project_id, :bigint)
+  end
+end

db/post_migrate/20240829160131_index_p_ci_runner_machine_builds_on_project_id.rb

+# frozen_string_literal: true
+
+class IndexPCiRunnerMachineBuildsOnProjectId < Gitlab::Database::Migration[2.2]
+  include Gitlab::Database::PartitioningMigrationHelpers
+
+  milestone '17.4'
+
+  disable_ddl_transaction!
+
+  TABLE_NAME = :p_ci_runner_machine_builds
+  PARTITIONED_INDEX_NAME = :index_p_ci_runner_machine_builds_on_project_id
+
+  def up
+    add_concurrent_partitioned_index(TABLE_NAME, :project_id, name: PARTITIONED_INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_partitioned_index_by_name(TABLE_NAME, PARTITIONED_INDEX_NAME)
+  end
+end

db/post_migrate/20240829160238_add_p_ci_runner_machine_builds_project_id_null_constraint.rb

+# frozen_string_literal: true
+
+class AddPCiRunnerMachineBuildsProjectIdNullConstraint < Gitlab::Database::Migration[2.2]
+  milestone '17.4'
+
+  disable_ddl_transaction!
+
+  TABLE_NAME = :p_ci_runner_machine_builds
+  COLUMN_NAME = :project_id
+  CONSTRAINT_NAME = :check_149ee35c38
+
+  def up
+    add_not_null_constraint(TABLE_NAME, COLUMN_NAME, constraint_name: CONSTRAINT_NAME, validate: false)
+  end
+
+  def down
+    remove_not_null_constraint(TABLE_NAME, COLUMN_NAME, constraint_name: CONSTRAINT_NAME)
+  end
+end

db/post_migrate/20240829160537_queue_backfill_p_ci_runner_machine_builds_project_id.rb

+# frozen_string_literal: true
+
+class QueueBackfillPCiRunnerMachineBuildsProjectId < Gitlab::Database::Migration[2.2]
+  milestone '17.4'
+  restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+  MIGRATION = "BackfillPCiRunnerMachineBuildsProjectId"
+  TABLE_NAME = :p_ci_runner_machine_builds
+  BATCH_COLUMN = :build_id
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 25_000
+  SUB_BATCH_SIZE = 150
+  JOB_ARGS = %i[project_id p_ci_builds project_id build_id partition_id]
+
+  def up
+    queue_batched_background_migration(
+      MIGRATION,
+      TABLE_NAME,
+      BATCH_COLUMN,
+      *JOB_ARGS,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      sub_batch_size: SUB_BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(MIGRATION, TABLE_NAME, BATCH_COLUMN, JOB_ARGS)
+  end
+end

db/schema_migrations/20240829160016

+2bca5d40580b4c3e1e28e7bbcc67d9abc6f77b24af1699a94d6c978adfb7665b
\ No newline at end of file

db/schema_migrations/20240829160131

+005f1ad70a51dda06bb3fd04721c7076260004495aec58fa65fe683ccb0651df
\ No newline at end of file

db/schema_migrations/20240829160238

+e09fcf4890486f3162d084cfdcb1d651bb8efbaf2bcec74f4fe74b810175e195
\ No newline at end of file

db/schema_migrations/20240829160537

+db6ca34705e10326b707e26adca13882d270b76fbdd60370048c205c0dcf4cb5
\ No newline at end of file

db/structure.sql

@@ -2615,7 +2615,8 @@ PARTITION BY LIST (partition_id);
 CREATE TABLE p_ci_runner_machine_builds (
     partition_id bigint NOT NULL,
     build_id bigint NOT NULL,
-    runner_machine_id bigint NOT NULL
+    runner_machine_id bigint NOT NULL,
+    project_id bigint
 )
 PARTITION BY LIST (partition_id);
@@ -23389,6 +23390,9 @@ ALTER TABLE ONLY chat_names
 ALTER TABLE ONLY chat_teams
     ADD CONSTRAINT chat_teams_pkey PRIMARY KEY (id);
+ALTER TABLE p_ci_runner_machine_builds
+    ADD CONSTRAINT check_149ee35c38 CHECK ((project_id IS NOT NULL)) NOT VALID;
+
 ALTER TABLE workspaces
     ADD CONSTRAINT check_2a89035b04 CHECK ((personal_access_token_id IS NOT NULL)) NOT VALID;
@@ -29409,6 +29413,8 @@ CREATE UNIQUE INDEX index_p_ci_job_annotations_on_partition_id_job_id_name ON ON
 CREATE INDEX index_p_ci_job_annotations_on_project_id ON ONLY p_ci_job_annotations USING btree (project_id);
+CREATE INDEX index_p_ci_runner_machine_builds_on_project_id ON ONLY p_ci_runner_machine_builds USING btree (project_id);
+
 CREATE INDEX index_p_ci_runner_machine_builds_on_runner_machine_id ON ONLY p_ci_runner_machine_builds USING btree (runner_machine_id);
 CREATE INDEX index_packages_build_infos_on_pipeline_id ON packages_build_infos USING btree (pipeline_id);

lib/gitlab/background_migration/backfill_p_ci_runner_machine_builds_project_id.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class BackfillPCiRunnerMachineBuildsProjectId < BackfillDesiredShardingKeyPartitionJob
+      operation_name :backfill_p_ci_runner_machine_builds_project_id
+      feature_category :fleet_visibility
+    end
+  end
+end

spec/db/schema_spec.rb

@@ -89,6 +89,7 @@
     ci_pipeline_variables: %w[partition_id pipeline_id project_id],
     ci_pipelines: %w[partition_id auto_canceled_by_partition_id],
     p_ci_pipelines: %w[partition_id auto_canceled_by_partition_id auto_canceled_by_id],
+    p_ci_runner_machine_builds: %w[project_id],
     ci_runner_projects: %w[runner_id],
     ci_sources_pipelines: %w[partition_id source_partition_id source_job_id],
     ci_sources_projects: %w[partition_id],

spec/lib/gitlab/background_migration/backfill_p_ci_runner_machine_builds_project_id_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillPCiRunnerMachineBuildsProjectId,
+  migration: :gitlab_ci, feature_category: :fleet_visibility do
+  include_examples 'desired sharding key backfill job' do
+    let(:batch_table) { :p_ci_runner_machine_builds }
+    let(:batch_column) { :build_id }
+    let(:backfill_column) { :project_id }
+    let(:backfill_via_table) { :p_ci_builds }
+    let(:backfill_via_column) { :project_id }
+    let(:backfill_via_foreign_key) { :build_id }
+    let(:partition_column) { :partition_id }
+  end
+end

spec/migrations/20240829160537_queue_backfill_p_ci_runner_machine_builds_project_id_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillPCiRunnerMachineBuildsProjectId, migration: :gitlab_ci, feature_category: :fleet_visibility do
+  let!(:batched_migration) { described_class::MIGRATION }
+  let(:expected_job_args) { %i[project_id p_ci_builds project_id build_id partition_id] }
+
+  it 'schedules a new batched migration' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(batched_migration).not_to have_scheduled_batched_migration
+      }
+
+      migration.after -> {
+        expect(batched_migration).to have_scheduled_batched_migration(
+          table_name: described_class::TABLE_NAME,
+          column_name: :build_id,
+          interval: described_class::DELAY_INTERVAL,
+          batch_size: described_class::BATCH_SIZE,
+          sub_batch_size: described_class::SUB_BATCH_SIZE,
+          gitlab_schema: :gitlab_ci,
+          job_arguments: expected_job_args
+        )
+      }
+    end
+  end
+
+  context 'when executed on .com' do
+    before do
+      allow(Gitlab).to receive(:com_except_jh?).and_return(true)
+    end
+
+    it 'schedules a new batched migration' do
+      reversible_migration do |migration|
+        migration.before -> {
+          expect(batched_migration).not_to have_scheduled_batched_migration
+        }
+
+        migration.after -> {
+          expect(batched_migration).to have_scheduled_batched_migration(
+            table_name: described_class::TABLE_NAME,
+            column_name: :build_id,
+            interval: described_class::DELAY_INTERVAL,
+            batch_size: described_class::BATCH_SIZE,
+            sub_batch_size: described_class::SUB_BATCH_SIZE,
+            gitlab_schema: :gitlab_ci,
+            job_arguments: expected_job_args
+          )
+        }
+      end
+    end
+  end
+end

spec/models/ci/runner_manager_build_spec.rb

@@ -58,7 +58,7 @@
     it { expect(partitioning_strategy.active_partition).to be_present }
   end
-  context 'loose foreign key on p_ci_runner_manager_builds.runner_manager_id' do # rubocop:disable RSpec/ContextWording
+  context 'with loose foreign key on p_ci_runner_manager_builds.runner_manager_id' do
     it_behaves_like 'cleanup by a loose foreign key' do
       let!(:parent) { create(:ci_runner_machine) }
       let!(:model) { create(:ci_runner_machine_build, runner_manager: parent) }
@@ -81,7 +81,7 @@
       it { is_expected.to eq(described_class.where(build_id: build_id)) }
     end
-    context 'with non-existeng build_id' do
+    context 'with non-existing build_id' do
       let(:build_id) { non_existing_record_id }
       it { is_expected.to be_empty }
@@ -103,4 +103,23 @@
       it { is_expected.to be_empty }
     end
   end
+
+  describe '#ensure_project_id' do
+    it 'sets the project_id before validation' do
+      runner_machine_build = FactoryBot.build(:ci_runner_machine_build, build: build)
+
+      expect do
+        runner_machine_build.validate!
+      end.to change { runner_machine_build.project_id }.from(nil).to(runner_machine_build.build.project.id)
+    end
+
+    it 'does not override the project_id if set' do
+      another_project = create(:project)
+      runner_machine_build = FactoryBot.build(:ci_runner_machine_build, project_id: another_project.id)
+
+      expect do
+        runner_machine_build.validate!
+      end.not_to change { runner_machine_build.project_id }.from(another_project.id)
+    end
+  end
 end