Merge branch '323676-add-fk-to-partitioned-web-hook-logs' into 'master'

Add FK from partitioned web_hook_logs to web_hooks (Part 3 of partitioned web_hook_logs migrations)

See merge request gitlab-org/gitlab!59282

changelogs/unreleased/323676-add-fk-to-partitioned-web-hook-logs.yml

+---
+title: Add a foreign key from the partitioned web_hook_logs to web_hooks
+merge_request: 59282
+author:
+type: other

db/post_migrate/20210413130011_add_partitioned_web_hook_log_fk.rb

+# frozen_string_literal: true
+
+class AddPartitionedWebHookLogFk < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::PartitioningMigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_partitioned_foreign_key :web_hook_logs_part_0c5294f417,
+      :web_hooks,
+      column: :web_hook_id,
+      on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key_if_exists :web_hook_logs_part_0c5294f417, column: :web_hook_id
+    end
+  end
+end

db/schema_migrations/20210413130011

+943466b272406a95c478337de84f72388dae88a8cf88f3b389e3ade9d4ecd63d
\ No newline at end of file

db/structure.sql

@@ -26480,6 +26480,9 @@ ALTER TABLE ONLY approval_project_rules_users
 ALTER TABLE ONLY lists
     ADD CONSTRAINT fk_rails_baed5f39b7 FOREIGN KEY (milestone_id) REFERENCES milestones(id) ON DELETE CASCADE;
 
+ALTER TABLE web_hook_logs_part_0c5294f417
+    ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY security_findings
     ADD CONSTRAINT fk_rails_bb63863cf1 FOREIGN KEY (scan_id) REFERENCES security_scans(id) ON DELETE CASCADE;
 

lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb

@@ -6,6 +6,80 @@ module Gitlab
       module ForeignKeyHelpers
         include ::Gitlab::Database::SchemaHelpers
 
+        # Adds a foreign key with only minimal locking on the tables involved.
+        #
+        # In concept it works similarly to add_concurrent_foreign_key, but we have
+        # to add a special helper for partitioned tables for the following reasons:
+        # - add_concurrent_foreign_key sets the constraint to `NOT VALID`
+        #   before validating it
+        # - Setting an FK to NOT VALID is not supported currently in Postgres (up to PG13)
+        # - Also, PostgreSQL will currently ignore NOT VALID constraints on partitions
+        #   when adding a valid FK to the partitioned table, so they have to
+        #   also be validated before we can add the final FK.
+        # Solution:
+        # - Add the foreign key first to each partition by using
+        #   add_concurrent_foreign_key and validating it
+        # - Once all partitions have a foreign key, add it also to the partitioned
+        #   table (there will be no need for a validation at that level)
+        # For those reasons, this method does not include an option to delay the
+        # validation, we have to force validate: true.
+        #
+        # source - The source (partitioned) table containing the foreign key.
+        # target - The target table the key points to.
+        # column - The name of the column to create the foreign key on.
+        # on_delete - The action to perform when associated data is removed,
+        #             defaults to "CASCADE".
+        # name - The name of the foreign key.
+        #
+        def add_concurrent_partitioned_foreign_key(source, target, column:, on_delete: :cascade, name: nil)
+          partition_options = {
+            column: column,
+            on_delete: on_delete,
+
+            # We'll use the same FK name for all partitions and match it to
+            # the name used for the partitioned table to follow the convention
+            # used by PostgreSQL when adding FKs to new partitions
+            name: name.presence || concurrent_partitioned_foreign_key_name(source, column),
+
+            # Force the FK validation to true for partitions (and the partitioned table)
+            validate: true
+          }
+
+          if foreign_key_exists?(source, target, **partition_options)
+            warning_message = "Foreign key not created because it exists already " \
+              "(this may be due to an aborted migration or similar): " \
+              "source: #{source}, target: #{target}, column: #{partition_options[:column]}, "\
+              "name: #{partition_options[:name]}, on_delete: #{partition_options[:on_delete]}"
+
+            Gitlab::AppLogger.warn warning_message
+
+            return
+          end
+
+          partitioned_table = find_partitioned_table(source)
+
+          partitioned_table.postgres_partitions.order(:name).each do |partition|
+            add_concurrent_foreign_key(partition.identifier, target, **partition_options)
+          end
+
+          with_lock_retries do
+            add_foreign_key(source, target, **partition_options)
+          end
+        end
+
+        # Returns the name for a concurrent partitioned foreign key.
+        #
+        # Similar to concurrent_foreign_key_name (Gitlab::Database::MigrationHelpers)
+        # we just keep a separate method in case we want a different behavior
+        # for partitioned tables
+        #
+        def concurrent_partitioned_foreign_key_name(table, column, prefix: 'fk_rails_')
+          identifier = "#{table}_#{column}_fk"
+          hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)
+
+          "#{prefix}#{hashed_identifier}"
+        end
+
         # Creates a "foreign key" that references a partitioned table. Because foreign keys referencing partitioned
         # tables are not supported in PG11, this does not create a true database foreign key, but instead implements the
         # same functionality at the database level by using triggers.

spec/db/schema_spec.rb

@@ -85,8 +85,7 @@ RSpec.describe 'Database schema' do
     users: %w[color_scheme_id created_by_id theme_id email_opted_in_source_id],
     users_star_projects: %w[user_id],
     vulnerability_identifiers: %w[external_id],
-    vulnerability_scanners: %w[external_id],
-    web_hook_logs_part_0c5294f417: %w[web_hook_id]
+    vulnerability_scanners: %w[external_id]
   }.with_indifferent_access.freeze
 
   context 'for table' do