From 1aa273c0c7df5125bc12169375b456483852105d Mon Sep 17 00:00:00 2001
From: Max Orefice <morefice@gitlab.com>
Date: Tue, 29 Oct 2024 16:09:40 +0000
Subject: [PATCH] Add ci_deleted_objects#project_id not null constraint
This commit adds a new not nul constraint at the database level
to prevent creating records without a project.
Changelog: other
---
db/docs/ci_deleted_objects.yml | 3 +-
...e_backfill_ci_deleted_object_project_id.rb | 23 ++++++++++++
...pdate_invalid_ci_deleted_object_records.rb | 27 ++++++++++++++
...d_not_null_ci_deleted_object_project_id.rb | 17 +++++++++
db/schema_migrations/20241028085040 | 1 +
db/schema_migrations/20241028085044 | 1 +
db/schema_migrations/20241028085339 | 1 +
db/structure.sql | 3 +-
spec/lib/gitlab/database/sharding_key_spec.rb | 1 +
..._invalid_ci_deleted_object_records_spec.rb | 36 +++++++++++++++++++
10 files changed, 111 insertions(+), 2 deletions(-)
create mode 100644 db/post_migrate/20241028085040_finalize_backfill_ci_deleted_object_project_id.rb
create mode 100644 db/post_migrate/20241028085044_update_invalid_ci_deleted_object_records.rb
create mode 100644 db/post_migrate/20241028085339_add_not_null_ci_deleted_object_project_id.rb
create mode 100644 db/schema_migrations/20241028085040
create mode 100644 db/schema_migrations/20241028085044
create mode 100644 db/schema_migrations/20241028085339
create mode 100644 spec/migrations/20241028085044_update_invalid_ci_deleted_object_records_spec.rb
diff --git a/db/docs/ci_deleted_objects.yml b/db/docs/ci_deleted_objects.yml
index 3bdf58b31850a..f746304b0b45c 100644
--- a/db/docs/ci_deleted_objects.yml
+++ b/db/docs/ci_deleted_objects.yml
@@ -8,4 +8,5 @@ description: Allows efficient batch deletion of data in object storage.
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/commit/9bf76fe03f8edf4f67023448161af27abb8fb521
milestone: '13.5'
gitlab_schema: gitlab_ci
-sharding_key_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/463245
+sharding_key:
+ project_id: projects
diff --git a/db/post_migrate/20241028085040_finalize_backfill_ci_deleted_object_project_id.rb b/db/post_migrate/20241028085040_finalize_backfill_ci_deleted_object_project_id.rb
new file mode 100644
index 0000000000000..d8860b2809748
--- /dev/null
+++ b/db/post_migrate/20241028085040_finalize_backfill_ci_deleted_object_project_id.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class FinalizeBackfillCiDeletedObjectProjectId < Gitlab::Database::Migration[2.2]
+ milestone '17.6'
+ disable_ddl_transaction!
+ restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+ MIGRATION = 'FixPickUpAtCiDeletedObject'
+
+ def up
+ ensure_batched_background_migration_is_finished(
+ job_class_name: MIGRATION,
+ table_name: :ci_deleted_objects,
+ column_name: :id,
+ job_arguments: [],
+ finalize: true
+ )
+ end
+
+ def down
+ # no-op
+ end
+end
diff --git a/db/post_migrate/20241028085044_update_invalid_ci_deleted_object_records.rb b/db/post_migrate/20241028085044_update_invalid_ci_deleted_object_records.rb
new file mode 100644
index 0000000000000..0c4d9c026c1b5
--- /dev/null
+++ b/db/post_migrate/20241028085044_update_invalid_ci_deleted_object_records.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class UpdateInvalidCiDeletedObjectRecords < Gitlab::Database::Migration[2.2]
+ milestone '17.6'
+ disable_ddl_transaction!
+ restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+ TABLE_NAME = :ci_deleted_objects
+ BATCH_SIZE = 1000
+
+ def up
+ return if Gitlab.com?
+
+ relation = define_batchable_model(TABLE_NAME).where(project_id: nil)
+
+ loop do
+ batch = relation.limit(BATCH_SIZE)
+ update_count = relation.where(id: batch.select(:id)).update_all(project_id: -1)
+
+ break if update_count == 0
+ end
+ end
+
+ def down
+ # no-op
+ end
+end
diff --git a/db/post_migrate/20241028085339_add_not_null_ci_deleted_object_project_id.rb b/db/post_migrate/20241028085339_add_not_null_ci_deleted_object_project_id.rb
new file mode 100644
index 0000000000000..d31b568b78dc5
--- /dev/null
+++ b/db/post_migrate/20241028085339_add_not_null_ci_deleted_object_project_id.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddNotNullCiDeletedObjectProjectId < Gitlab::Database::Migration[2.2]
+ milestone '17.6'
+ disable_ddl_transaction!
+
+ TABLE_NAME = :ci_deleted_objects
+ COLUMN_NAME = :project_id
+
+ def up
+ add_not_null_constraint(TABLE_NAME, COLUMN_NAME)
+ end
+
+ def down
+ remove_not_null_constraint(TABLE_NAME, COLUMN_NAME)
+ end
+end
diff --git a/db/schema_migrations/20241028085040 b/db/schema_migrations/20241028085040
new file mode 100644
index 0000000000000..61928c62057c3
--- /dev/null
+++ b/db/schema_migrations/20241028085040
@@ -0,0 +1 @@
+da93fba39bd21ed82571d63da65bb35b0d0628445f23a5f872dbdce426361773
\ No newline at end of file
diff --git a/db/schema_migrations/20241028085044 b/db/schema_migrations/20241028085044
new file mode 100644
index 0000000000000..ad9faf593b8ea
--- /dev/null
+++ b/db/schema_migrations/20241028085044
@@ -0,0 +1 @@
+d6dd5d6d203730e3e701dd0eb5a312403d6989a6d12128e2975d928eef5a93a3
\ No newline at end of file
diff --git a/db/schema_migrations/20241028085339 b/db/schema_migrations/20241028085339
new file mode 100644
index 0000000000000..a238ddab4721f
--- /dev/null
+++ b/db/schema_migrations/20241028085339
@@ -0,0 +1 @@
+6d36b298741eb05e66c9880e94e3376c484844bd73e11820198b72b92aa29bd7
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index f3aaf7324bcd8..4711290496b16 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -8552,7 +8552,8 @@ CREATE TABLE ci_deleted_objects (
store_dir text NOT NULL,
file text NOT NULL,
project_id bigint,
- CONSTRAINT check_5e151d6912 CHECK ((char_length(store_dir) <= 1024))
+ CONSTRAINT check_5e151d6912 CHECK ((char_length(store_dir) <= 1024)),
+ CONSTRAINT check_98f90d6c53 CHECK ((project_id IS NOT NULL))
);
CREATE SEQUENCE ci_deleted_objects_id_seq
diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb
index 95ca587b1f5b3..673e7e8c2c2f3 100644
--- a/spec/lib/gitlab/database/sharding_key_spec.rb
+++ b/spec/lib/gitlab/database/sharding_key_spec.rb
@@ -63,6 +63,7 @@
'ci_job_artifacts.project_id',
'ci_namespace_monthly_usages.namespace_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/321400
'ci_builds_metadata.project_id',
+ 'ci_deleted_objects.project_id', # LFK already present on p_ci_builds and cascade delete all ci resources
'p_ci_job_annotations.project_id', # LFK already present on p_ci_builds and cascade delete all ci resources
'ldap_group_links.group_id',
'namespace_descendants.namespace_id',
diff --git a/spec/migrations/20241028085044_update_invalid_ci_deleted_object_records_spec.rb b/spec/migrations/20241028085044_update_invalid_ci_deleted_object_records_spec.rb
new file mode 100644
index 0000000000000..829a21ac37d0e
--- /dev/null
+++ b/spec/migrations/20241028085044_update_invalid_ci_deleted_object_records_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe UpdateInvalidCiDeletedObjectRecords, feature_category: :job_artifacts, migration: :gitlab_ci do
+ let!(:ci_deleted_objects) { table(:ci_deleted_objects) }
+
+ describe '#up' do
+ before do
+ ci_deleted_objects.create!(project_id: 1, pick_up_at: Time.current, store_dir: "dir", file: "file1")
+ ci_deleted_objects.create!(project_id: nil, pick_up_at: Time.current, store_dir: "dir", file: "file2")
+ ci_deleted_objects.create!(project_id: nil, pick_up_at: Time.current, store_dir: "dir", file: "file3")
+
+ stub_const("#{described_class}::BATCH_SIZE", 1)
+ end
+
+ it 'sets project_id to -1 for records without a project_id' do
+ migrate!
+
+ expect(ci_deleted_objects.first).to have_attributes(project_id: 1)
+ expect(ci_deleted_objects.second).to have_attributes(project_id: -1)
+ expect(ci_deleted_objects.third).to have_attributes(project_id: -1)
+ end
+
+ it 'does nothing on gitlab.com' do
+ allow(Gitlab).to receive(:com?).and_return(true)
+
+ migrate!
+
+ expect(ci_deleted_objects.first).to have_attributes(project_id: 1)
+ expect(ci_deleted_objects.second).to have_attributes(project_id: nil)
+ expect(ci_deleted_objects.third).to have_attributes(project_id: nil)
+ end
+ end
+end
--
GitLab