Merge branch '353365-remove-artifacts_archive_id-column' into 'master'

Removing the ignored column project_pages_metadata.artifacts_archive_id

See merge request gitlab-org/gitlab!83660

config/gitlab_loose_foreign_keys.yml

@@ -207,10 +207,6 @@ pages_deployments:
   - table: ci_builds
     column: ci_build_id
     on_delete: async_nullify
-project_pages_metadata:
-  - table: ci_job_artifacts
-    column: artifacts_archive_id
-    on_delete: async_nullify
 requirements_management_test_reports:
   - table: ci_builds
     column: build_id

db/post_migrate/20220324110247_untrack_deletions_on_ci_job_artifacts.rb

+# frozen_string_literal: true
+
+class UntrackDeletionsOnCiJobArtifacts < Gitlab::Database::Migration[1.0]
+  include Gitlab::Database::MigrationHelpers::LooseForeignKeyHelpers
+
+  enable_lock_retries!
+
+  def up
+    untrack_record_deletions(:ci_job_artifacts)
+  end
+
+  def down
+    track_record_deletions(:ci_job_artifacts)
+  end
+end

db/post_migrate/20220324180717_remove_project_pages_metadata_artifacts_archive_id_column.rb

+# frozen_string_literal: true
+
+class RemoveProjectPagesMetadataArtifactsArchiveIdColumn < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  def up
+    remove_column :project_pages_metadata, :artifacts_archive_id
+  end
+
+  def down
+    unless column_exists?(:project_pages_metadata, :artifacts_archive_id)
+      add_column :project_pages_metadata, :artifacts_archive_id, :bigint
+    end
+
+    add_concurrent_index(
+      :project_pages_metadata,
+      :artifacts_archive_id,
+      name: "index_project_pages_metadata_on_artifacts_archive_id"
+    )
+  end
+end

db/post_migrate/20220329175119_remove_leftover_ci_job_artifact_deletions.rb

+# frozen_string_literal: true
+
+class RemoveLeftoverCiJobArtifactDeletions < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  def up
+    # Delete all pending record deletions in the public.ci_job_artifacts until
+    # there are no more rows left.
+    loop do
+      result = execute <<~SQL
+      DELETE FROM "loose_foreign_keys_deleted_records"
+      WHERE
+      ("loose_foreign_keys_deleted_records"."partition", "loose_foreign_keys_deleted_records"."id") IN (
+        SELECT "loose_foreign_keys_deleted_records"."partition", "loose_foreign_keys_deleted_records"."id"
+        FROM "loose_foreign_keys_deleted_records"
+        WHERE
+        "loose_foreign_keys_deleted_records"."fully_qualified_table_name" = 'public.ci_job_artifacts' AND
+        "loose_foreign_keys_deleted_records"."status" = 1
+        LIMIT 100
+      )
+      SQL
+
+      break if result.cmd_tuples == 0
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20220324110247

+35aaf03898795800027c814a6f907af6f011bd5095cae7188234b46f4b2ebb90
\ No newline at end of file

db/schema_migrations/20220324180717

+56706c8d4139c63427838d37b7120c57fd39c7be4caee0ebba39a05cd3c453f7
\ No newline at end of file

db/schema_migrations/20220329175119

+ec5dfe48e13cdbce5fc0c67612cddf1a5b555b3c84e1869ee3a7f047f0f3f1a0
\ No newline at end of file

db/structure.sql

@@ -19278,7 +19278,6 @@ ALTER SEQUENCE project_mirror_data_id_seq OWNED BY project_mirror_data.id;
 CREATE TABLE project_pages_metadata (
     project_id bigint NOT NULL,
     deployed boolean DEFAULT false NOT NULL,
-    artifacts_archive_id bigint,
     pages_deployment_id bigint
 );
 
@@ -28666,8 +28665,6 @@ CREATE UNIQUE INDEX index_project_mirror_data_on_project_id ON project_mirror_da
 
 CREATE INDEX index_project_mirror_data_on_status ON project_mirror_data USING btree (status);
 
-CREATE INDEX index_project_pages_metadata_on_artifacts_archive_id ON project_pages_metadata USING btree (artifacts_archive_id);
-
 CREATE INDEX index_project_pages_metadata_on_pages_deployment_id ON project_pages_metadata USING btree (pages_deployment_id);
 
 CREATE INDEX index_project_pages_metadata_on_project_id_and_deployed_is_true ON project_pages_metadata USING btree (project_id) WHERE (deployed = true);
@@ -30968,8 +30965,6 @@ CREATE TRIGGER chat_names_loose_fk_trigger AFTER DELETE ON chat_names REFERENCIN
 
 CREATE TRIGGER ci_builds_loose_fk_trigger AFTER DELETE ON ci_builds REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records();
 
-CREATE TRIGGER ci_job_artifacts_loose_fk_trigger AFTER DELETE ON ci_job_artifacts REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records();
-
 CREATE TRIGGER ci_pipelines_loose_fk_trigger AFTER DELETE ON ci_pipelines REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records();
 
 CREATE TRIGGER ci_runners_loose_fk_trigger AFTER DELETE ON ci_runners REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records();

spec/migrations/20220329175119_remove_leftover_ci_job_artifact_deletions_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe RemoveLeftoverCiJobArtifactDeletions do
+  let(:deleted_records) { table(:loose_foreign_keys_deleted_records) }
+
+  target_table_name = Ci::JobArtifact.table_name
+
+  let(:pending_record1) do
+    deleted_records.create!(
+      id: 1,
+      fully_qualified_table_name: "public.#{target_table_name}",
+      primary_key_value: 1,
+      status: 1
+    )
+  end
+
+  let(:pending_record2) do
+    deleted_records.create!(
+      id: 2,
+      fully_qualified_table_name: "public.#{target_table_name}",
+      primary_key_value: 2,
+      status: 1
+    )
+  end
+
+  let(:other_pending_record1) do
+    deleted_records.create!(
+      id: 3,
+      fully_qualified_table_name: 'public.projects',
+      primary_key_value: 1,
+      status: 1
+    )
+  end
+
+  let(:other_pending_record2) do
+    deleted_records.create!(
+      id: 4,
+      fully_qualified_table_name: 'public.ci_builds',
+      primary_key_value: 1,
+      status: 1
+    )
+  end
+
+  let(:processed_record1) do
+    deleted_records.create!(
+      id: 5,
+      fully_qualified_table_name: 'public.external_pull_requests',
+      primary_key_value: 3,
+      status: 2
+    )
+  end
+
+  let(:other_processed_record1) do
+    deleted_records.create!(
+      id: 6,
+      fully_qualified_table_name: 'public.ci_builds',
+      primary_key_value: 2,
+      status: 2
+    )
+  end
+
+  let!(:persisted_ids_before) do
+    [
+      pending_record1,
+      pending_record2,
+      other_pending_record1,
+      other_pending_record2,
+      processed_record1,
+      other_processed_record1
+    ].map(&:id).sort
+  end
+
+  let!(:persisted_ids_after) do
+    [
+      other_pending_record1,
+      other_pending_record2,
+      processed_record1,
+      other_processed_record1
+    ].map(&:id).sort
+  end
+
+  def all_ids
+    deleted_records.all.map(&:id).sort
+  end
+
+  it 'deletes pending external_pull_requests records' do
+    expect { migrate! }.to change { all_ids }.from(persisted_ids_before).to(persisted_ids_after)
+  end
+end

spec/models/ci/job_artifact_spec.rb

@@ -700,10 +700,6 @@ RSpec.describe Ci::JobArtifact do
     MSG
   end
 
-  it_behaves_like 'it has loose foreign keys' do
-    let(:factory_name) { :ci_job_artifact }
-  end
-
   context 'loose foreign key on ci_job_artifacts.project_id' do
     it_behaves_like 'cleanup by a loose foreign key' do
       let!(:parent) { create(:project) }