From 2ac50a8ce7a485bac517029185f48cde9558ce80 Mon Sep 17 00:00:00 2001
From: Shubham Kumar <shukumar@gitlab.com>
Date: Tue, 17 Dec 2024 13:52:35 +0000
Subject: [PATCH] Add and backfill namespace_id for issuable_slas

## What does this MR do and why?

Add and backfill namespace_id for issuable_slas.

This table has a
[desired sharding key](https://docs.gitlab.com/ee/development/database/multiple_databases.html#define-a-desired_sharding_key-to-automatically-backfill-a-sharding_key)
configured ([view configuration](https://gitlab.com/gitlab-org/gitlab/-/blob/master/db/docs/issuable_slas.yml)).

This merge request is the first step towards transforming the desired sharding key into a
[sharding key](https://docs.gitlab.com/ee/development/database/multiple_databases.html#defining-a-sharding-key-for-all-cell-local-tables).

This involves three changes:

- Adding a new column that will serve as the sharding key (along with the relevant index and foreign key).
- Populating the sharding key when new records are created by adding a database function and trigger.
- Scheduling a [batched background migration](https://docs.gitlab.com/ee/development/database/batched_background_migrations.html)
  to set the sharding key for existing records.

Once the background migration has completed, a second merge request will be created to finalize the background
migration and validate the not null constraint.

## How to verify

We have assigned a random backend engineer from ~"group::platform insights" to review these changes. Please review this merge
request from a ~backend perspective. The main thing we are looking to verify is that the added column and association
match the values specified by the [desired sharding key](https://gitlab.com/gitlab-org/gitlab/-/blob/master/db/docs/issuable_slas.yml)
configuration and that backfilling the column from this other table makes sense in the context of this feature.

When you are finished, please:

1. Trigger the [database testing pipeline](https://docs.gitlab.com/ee/development/database/database_migration_pipeline.html)
   as instructed by Danger.
1. Request a review from the ~backend maintainer and ~database reviewer suggested by Danger.

If you have any questions or concerns, reach out to `@tigerwnz` or @shubhamkrai.

This merge request was generated by a once off keep implemented in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143774

This change was generated by
[gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
using the Keeps::BackfillDesiredShardingKeySmallTable keep.

To provide feedback on your experience with `gitlab-housekeeper` please create an issue with the
label ~"GitLab Housekeeper" and consider pinging the author of this keep.

Changelog: other
---
 .../backfill_issuable_slas_namespace_id.yml   |  8 ++++
 db/docs/issuable_slas.yml                     |  1 +
 ...42259_add_namespace_id_to_issuable_slas.rb |  9 +++++
 ...260_index_issuable_slas_on_namespace_id.rb | 16 ++++++++
 ...42261_add_issuable_slas_namespace_id_fk.rb | 16 ++++++++
 ..._add_issuable_slas_namespace_id_trigger.rb | 25 ++++++++++++
 ...eue_backfill_issuable_slas_namespace_id.rb | 40 +++++++++++++++++++
 db/schema_migrations/20241213142259           |  1 +
 db/schema_migrations/20241213142260           |  1 +
 db/schema_migrations/20241213142261           |  1 +
 db/schema_migrations/20241213142262           |  1 +
 db/schema_migrations/20241213142263           |  1 +
 db/structure.sql                              | 26 +++++++++++-
 .../backfill_issuable_slas_namespace_id.rb    | 10 +++++
 ...ackfill_issuable_slas_namespace_id_spec.rb | 15 +++++++
 ...ackfill_issuable_slas_namespace_id_spec.rb | 33 +++++++++++++++
 16 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 db/docs/batched_background_migrations/backfill_issuable_slas_namespace_id.yml
 create mode 100644 db/migrate/20241213142259_add_namespace_id_to_issuable_slas.rb
 create mode 100644 db/post_migrate/20241213142260_index_issuable_slas_on_namespace_id.rb
 create mode 100644 db/post_migrate/20241213142261_add_issuable_slas_namespace_id_fk.rb
 create mode 100644 db/post_migrate/20241213142262_add_issuable_slas_namespace_id_trigger.rb
 create mode 100644 db/post_migrate/20241213142263_queue_backfill_issuable_slas_namespace_id.rb
 create mode 100644 db/schema_migrations/20241213142259
 create mode 100644 db/schema_migrations/20241213142260
 create mode 100644 db/schema_migrations/20241213142261
 create mode 100644 db/schema_migrations/20241213142262
 create mode 100644 db/schema_migrations/20241213142263
 create mode 100644 lib/gitlab/background_migration/backfill_issuable_slas_namespace_id.rb
 create mode 100644 spec/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id_spec.rb
 create mode 100644 spec/migrations/20241213142263_queue_backfill_issuable_slas_namespace_id_spec.rb

diff --git a/db/docs/batched_background_migrations/backfill_issuable_slas_namespace_id.yml b/db/docs/batched_background_migrations/backfill_issuable_slas_namespace_id.yml
new file mode 100644
index 0000000000000..169c91f2e0070
--- /dev/null
+++ b/db/docs/batched_background_migrations/backfill_issuable_slas_namespace_id.yml
@@ -0,0 +1,8 @@
+---
+migration_job_name: BackfillIssuableSlasNamespaceId
+description: Backfills sharding key `issuable_slas.namespace_id` from `issues`.
+feature_category: incident_management
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/175719
+milestone: '17.8'
+queued_migration_version: 20241213142263
+finalized_by: # version of the migration that finalized this BBM
diff --git a/db/docs/issuable_slas.yml b/db/docs/issuable_slas.yml
index 7df1d2a5a0633..c157c08f46a72 100644
--- a/db/docs/issuable_slas.yml
+++ b/db/docs/issuable_slas.yml
@@ -18,3 +18,4 @@ desired_sharding_key:
         sharding_key: namespace_id
         belongs_to: issue
 table_size: small
+desired_sharding_key_migration_job_name: BackfillIssuableSlasNamespaceId
diff --git a/db/migrate/20241213142259_add_namespace_id_to_issuable_slas.rb b/db/migrate/20241213142259_add_namespace_id_to_issuable_slas.rb
new file mode 100644
index 0000000000000..f3d333619e26c
--- /dev/null
+++ b/db/migrate/20241213142259_add_namespace_id_to_issuable_slas.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddNamespaceIdToIssuableSlas < Gitlab::Database::Migration[2.2]
+  milestone '17.8'
+
+  def change
+    add_column :issuable_slas, :namespace_id, :bigint
+  end
+end
diff --git a/db/post_migrate/20241213142260_index_issuable_slas_on_namespace_id.rb b/db/post_migrate/20241213142260_index_issuable_slas_on_namespace_id.rb
new file mode 100644
index 0000000000000..fecd58060aec0
--- /dev/null
+++ b/db/post_migrate/20241213142260_index_issuable_slas_on_namespace_id.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class IndexIssuableSlasOnNamespaceId < Gitlab::Database::Migration[2.2]
+  milestone '17.8'
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_issuable_slas_on_namespace_id'
+
+  def up
+    add_concurrent_index :issuable_slas, :namespace_id, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :issuable_slas, INDEX_NAME
+  end
+end
diff --git a/db/post_migrate/20241213142261_add_issuable_slas_namespace_id_fk.rb b/db/post_migrate/20241213142261_add_issuable_slas_namespace_id_fk.rb
new file mode 100644
index 0000000000000..22216f29a4d7e
--- /dev/null
+++ b/db/post_migrate/20241213142261_add_issuable_slas_namespace_id_fk.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class AddIssuableSlasNamespaceIdFk < Gitlab::Database::Migration[2.2]
+  milestone '17.8'
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :issuable_slas, :namespaces, column: :namespace_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :issuable_slas, column: :namespace_id
+    end
+  end
+end
diff --git a/db/post_migrate/20241213142262_add_issuable_slas_namespace_id_trigger.rb b/db/post_migrate/20241213142262_add_issuable_slas_namespace_id_trigger.rb
new file mode 100644
index 0000000000000..88b146c77b1ed
--- /dev/null
+++ b/db/post_migrate/20241213142262_add_issuable_slas_namespace_id_trigger.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class AddIssuableSlasNamespaceIdTrigger < Gitlab::Database::Migration[2.2]
+  milestone '17.8'
+
+  def up
+    install_sharding_key_assignment_trigger(
+      table: :issuable_slas,
+      sharding_key: :namespace_id,
+      parent_table: :issues,
+      parent_sharding_key: :namespace_id,
+      foreign_key: :issue_id
+    )
+  end
+
+  def down
+    remove_sharding_key_assignment_trigger(
+      table: :issuable_slas,
+      sharding_key: :namespace_id,
+      parent_table: :issues,
+      parent_sharding_key: :namespace_id,
+      foreign_key: :issue_id
+    )
+  end
+end
diff --git a/db/post_migrate/20241213142263_queue_backfill_issuable_slas_namespace_id.rb b/db/post_migrate/20241213142263_queue_backfill_issuable_slas_namespace_id.rb
new file mode 100644
index 0000000000000..0ae839d1ad844
--- /dev/null
+++ b/db/post_migrate/20241213142263_queue_backfill_issuable_slas_namespace_id.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+class QueueBackfillIssuableSlasNamespaceId < Gitlab::Database::Migration[2.2]
+  milestone '17.8'
+  restrict_gitlab_migration gitlab_schema: :gitlab_main_cell
+
+  MIGRATION = "BackfillIssuableSlasNamespaceId"
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 1000
+  SUB_BATCH_SIZE = 100
+
+  def up
+    queue_batched_background_migration(
+      MIGRATION,
+      :issuable_slas,
+      :id,
+      :namespace_id,
+      :issues,
+      :namespace_id,
+      :issue_id,
+      job_interval: DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      sub_batch_size: SUB_BATCH_SIZE
+    )
+  end
+
+  def down
+    delete_batched_background_migration(
+      MIGRATION,
+      :issuable_slas,
+      :id,
+      [
+        :namespace_id,
+        :issues,
+        :namespace_id,
+        :issue_id
+      ]
+    )
+  end
+end
diff --git a/db/schema_migrations/20241213142259 b/db/schema_migrations/20241213142259
new file mode 100644
index 0000000000000..f7d0a9339fa5d
--- /dev/null
+++ b/db/schema_migrations/20241213142259
@@ -0,0 +1 @@
+974aaa995416645bb17f250224e2eda3327dc94c9f2194e11219624f4cf37efc
\ No newline at end of file
diff --git a/db/schema_migrations/20241213142260 b/db/schema_migrations/20241213142260
new file mode 100644
index 0000000000000..ac289713a7264
--- /dev/null
+++ b/db/schema_migrations/20241213142260
@@ -0,0 +1 @@
+3c0f16d01dc521e3032111746b0b2469378548b8d5c80e5e9c3355cec6ad1fed
\ No newline at end of file
diff --git a/db/schema_migrations/20241213142261 b/db/schema_migrations/20241213142261
new file mode 100644
index 0000000000000..a3bb3107256bc
--- /dev/null
+++ b/db/schema_migrations/20241213142261
@@ -0,0 +1 @@
+7e19b2234f7bfb386d2e39d5da33de45bd51d3d9d4fb560f70e3eedaa687163b
\ No newline at end of file
diff --git a/db/schema_migrations/20241213142262 b/db/schema_migrations/20241213142262
new file mode 100644
index 0000000000000..551218b1d5137
--- /dev/null
+++ b/db/schema_migrations/20241213142262
@@ -0,0 +1 @@
+a79a7f0b9972a57e5a73e19fa46457f6e84a00e5c81ba22d2691c88de95bea13
\ No newline at end of file
diff --git a/db/schema_migrations/20241213142263 b/db/schema_migrations/20241213142263
new file mode 100644
index 0000000000000..948adf836fa4a
--- /dev/null
+++ b/db/schema_migrations/20241213142263
@@ -0,0 +1 @@
+7c9d3234710dcccae97cd6a15eb1777fedb687eec2e8123b059d63db6e5b99af
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 4cd1ef0559a46..07bb978abc9ec 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -2838,6 +2838,22 @@ RETURN NEW;
 END
 $$;
 
+CREATE FUNCTION trigger_cd50823537a3() RETURNS trigger
+    LANGUAGE plpgsql
+    AS $$
+BEGIN
+IF NEW."namespace_id" IS NULL THEN
+  SELECT "namespace_id"
+  INTO NEW."namespace_id"
+  FROM "issues"
+  WHERE "issues"."id" = NEW."issue_id";
+END IF;
+
+RETURN NEW;
+
+END
+$$;
+
 CREATE FUNCTION trigger_cf646a118cbb() RETURNS trigger
     LANGUAGE plpgsql
     AS $$
@@ -13901,7 +13917,8 @@ CREATE TABLE issuable_slas (
     issue_id bigint NOT NULL,
     due_at timestamp with time zone NOT NULL,
     label_applied boolean DEFAULT false NOT NULL,
-    issuable_closed boolean DEFAULT false NOT NULL
+    issuable_closed boolean DEFAULT false NOT NULL,
+    namespace_id bigint
 );
 
 CREATE SEQUENCE issuable_slas_id_seq
@@ -31054,6 +31071,8 @@ CREATE INDEX index_issuable_slas_on_due_at_id_label_applied_issuable_closed ON i
 
 CREATE UNIQUE INDEX index_issuable_slas_on_issue_id ON issuable_slas USING btree (issue_id);
 
+CREATE INDEX index_issuable_slas_on_namespace_id ON issuable_slas USING btree (namespace_id);
+
 CREATE INDEX index_issue_assignees_on_user_id_and_issue_id ON issue_assignees USING btree (user_id, issue_id);
 
 CREATE INDEX index_issue_assignment_events_on_namespace_id ON issue_assignment_events USING btree (namespace_id);
@@ -35918,6 +35937,8 @@ CREATE TRIGGER trigger_cac7c0698291 BEFORE INSERT OR UPDATE ON evidences FOR EAC
 
 CREATE TRIGGER trigger_catalog_resource_sync_event_on_project_update AFTER UPDATE ON projects FOR EACH ROW WHEN ((((old.name)::text IS DISTINCT FROM (new.name)::text) OR (old.description IS DISTINCT FROM new.description) OR (old.visibility_level IS DISTINCT FROM new.visibility_level))) EXECUTE FUNCTION insert_catalog_resource_sync_event();
 
+CREATE TRIGGER trigger_cd50823537a3 BEFORE INSERT OR UPDATE ON issuable_slas FOR EACH ROW EXECUTE FUNCTION trigger_cd50823537a3();
+
 CREATE TRIGGER trigger_cf646a118cbb BEFORE INSERT OR UPDATE ON milestone_releases FOR EACH ROW EXECUTE FUNCTION trigger_cf646a118cbb();
 
 CREATE TRIGGER trigger_d4487a75bd44 BEFORE INSERT OR UPDATE ON terraform_state_versions FOR EACH ROW EXECUTE FUNCTION trigger_d4487a75bd44();
@@ -36300,6 +36321,9 @@ ALTER TABLE ONLY user_namespace_callouts
 ALTER TABLE ONLY user_details
     ADD CONSTRAINT fk_27ac767d6a FOREIGN KEY (bot_namespace_id) REFERENCES namespaces(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY issuable_slas
+    ADD CONSTRAINT fk_282ef683a5 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY work_item_dates_sources
     ADD CONSTRAINT fk_283fb4ad36 FOREIGN KEY (start_date_sourcing_milestone_id) REFERENCES milestones(id) ON DELETE SET NULL;
 
diff --git a/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id.rb b/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id.rb
new file mode 100644
index 0000000000000..2a07d8131fc59
--- /dev/null
+++ b/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    class BackfillIssuableSlasNamespaceId < BackfillDesiredShardingKeyJob
+      operation_name :backfill_issuable_slas_namespace_id
+      feature_category :incident_management
+    end
+  end
+end
diff --git a/spec/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id_spec.rb b/spec/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id_spec.rb
new file mode 100644
index 0000000000000..95fc64afe746e
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/backfill_issuable_slas_namespace_id_spec.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillIssuableSlasNamespaceId,
+  feature_category: :incident_management,
+  schema: 20241213142259 do
+  include_examples 'desired sharding key backfill job' do
+    let(:batch_table) { :issuable_slas }
+    let(:backfill_column) { :namespace_id }
+    let(:backfill_via_table) { :issues }
+    let(:backfill_via_column) { :namespace_id }
+    let(:backfill_via_foreign_key) { :issue_id }
+  end
+end
diff --git a/spec/migrations/20241213142263_queue_backfill_issuable_slas_namespace_id_spec.rb b/spec/migrations/20241213142263_queue_backfill_issuable_slas_namespace_id_spec.rb
new file mode 100644
index 0000000000000..625c46ee2a360
--- /dev/null
+++ b/spec/migrations/20241213142263_queue_backfill_issuable_slas_namespace_id_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillIssuableSlasNamespaceId, feature_category: :incident_management do
+  let!(:batched_migration) { described_class::MIGRATION }
+
+  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: :issuable_slas,
+          column_name: :id,
+          interval: described_class::DELAY_INTERVAL,
+          batch_size: described_class::BATCH_SIZE,
+          sub_batch_size: described_class::SUB_BATCH_SIZE,
+          gitlab_schema: :gitlab_main_cell,
+          job_arguments: [
+            :namespace_id,
+            :issues,
+            :namespace_id,
+            :issue_id
+          ]
+        )
+      }
+    end
+  end
+end
-- 
GitLab