Skip to content

(Part 1) Adding Loose Foreign Key trigger to existing CI Partitions

Omar Qunsul requested to merge 507343-add-lfk-trigger-to-existing-ci-tables into master

What does this MR do and why?

Extracted from: !175310

Adding Loose Foreign Key trigger to existing CI Partitions

Addressing first part of issue #507343

Context

It was discovered as part of this issue that CI partitioned tables and their partitions can add different table name to the loose_foreign_keys_deleted_records.fully_qualified_table_name when the records are deleted. But this would force us to add all the dynamic partitions to config/gitlab_loose_foreign_keys.yml, which is not possible, because these partitions are managed by the PartitionManager are created dynamically.

It was also discovered that newly created partitions like on ci_builds, don't have this trigger at all. So if any DELETE statement runs directly against these partitions, no loose_foreign_keys_deleted_records are created at all. We have an issue to clean up the tables that have references to those partitions as part of this issue: #507345.

This MR adds a new trigger function that can override the table_name. We are attaching this trigger to:

  1. The partitioned tables (parents) p_ci_pipelines, p_ci_builds.
  2. Legacy partitions that are on the public (default schema) ci_piplines, ci_builds
  3. Newly created partitions in the Dynamic Partitions Schema. We have none for p_ci_pipelines, but we have 2 for ci_builds (hard-coded)

In a follow up MR: we will fix PartitionManager to automatically attach LFK triggers to any new partitions, if the parent (partitioned) table has LFK function attached as a trigger. We aren't going to add any new CI partitions any time soon. So it's safe to address the issue with the current CI partitions. You can have a look at this MR to get a sneak peek into the code: !175310.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  • Checkout out this branch 507343-add-lfk-trigger-to-existing-ci-tables and run the database migrations rails db:migrate
  • gdk stop rails-background-jobs # To make sure that the Cleanup worker is not running
  • gdk psql -d gitlabhq_development_ci -c 'delete from loose_foreign_keys_deleted_records'

1. Testing statements against partitions

project = Project.first; nil
pipeline = FactoryBot.create(:ci_pipeline, partition_id: 101, project: project); nil
build = FactoryBot.create(:ci_build, pipeline: pipeline, project: project); nil
Ci::Build.connection.execute("delete from gitlab_partitions_dynamic.ci_builds_101 where id = #{build.id}")
  • The following query should print ['public.p_ci_builds']
Gitlab::Database::SharedModel.using_connection(Ci::Build.connection) do
  puts LooseForeignKeys::DeletedRecord.connection.select_values("select fully_qualified_table_name FROM loose_foreign_keys_deleted_records where status = 1").inspect
end
  • But after running LooseForeignKeys::CleanupWorker.new.perform the previous query should print empty array

2. Testing statements against partitioned tables (parents)

build = FactoryBot.create(:ci_build, pipeline: pipeline, project: project); nil
Ci::Build.connection.execute("delete from p_ci_builds where id = #{build.id}")
  • The following query should print ['public.p_ci_builds']
Gitlab::Database::SharedModel.using_connection(Ci::Build.connection) do
  puts LooseForeignKeys::DeletedRecord.connection.select_values("select fully_qualified_table_name FROM loose_foreign_keys_deleted_records where status = 1").inspect
end
  • But after running LooseForeignKeys::CleanupWorker.new.perform the previous query should print empty array

Cleanup after testing

Please don't forget to rollback the migrations after testing locally.

References:

Related issue

Related to #507343

Edited by Omar Qunsul

Merge request reports

Loading