Skip to content

Ensure all Loose Foreign Key triggers have YML definitions

Dylan Griffith requested to merge 354972-lfk-validate-triggers-have-definition into master

What does this MR do and why?

Loose foreign keys have 2 separate components:

  1. A trigger that tracks when records are deleted from a parent table and adds them to a queue of deleted parent records
  2. A YML file config/gitlab_loose_foreign_keys.yml that lists all the "child" tables that depend on this parent table and therefore need to be cascaded

We then have a cron worker that loops over all the dependent "child" tables and searches for corresponding deleted "parent" table records and cleans up the child tables as well as cleaning up the queue of deleted parent table records. If we added to the queue of deleted parent records for some table that had no children defined then this queue would just keep growing. This happened once before when we removed the YML definition but forgot to remove the trigger in !81866 (merged) .

Read more about loose foreign keys at https://docs.gitlab.com/ee/development/database/loose_foreign_keys.html .

This MR adds a spec that looks at all the triggers and ensures that they all have at least 1 corresponding YML definition that will handle the deleted records. It also fixes the ordering in the documentation about how to remove loose foreign keys. Technically they can all be in a single MR but at least having the ordering right in case people want a separate MR is a good idea.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

  1. Remove all references for some parent table in config/gitlab_loose_foreign_keys.yml: 
    diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml
index 8a9f3f0da43..ad71b6c7943 100644
--- a/config/gitlab_loose_foreign_keys.yml
+++ b/config/gitlab_loose_foreign_keys.yml
@@ -11,9 +11,6 @@ ci_builds:
   - table: projects
     column: project_id
     on_delete: async_delete
-  - table: ci_runners
-    column: runner_id
-    on_delete: async_nullify
 ci_builds_metadata:
   - table: projects
     column: project_id
@@ -159,10 +156,6 @@ ci_variables:
   - table: projects
     column: project_id
     on_delete: async_delete
-clusters_applications_runners:
-  - table: ci_runners
-    column: runner_id
-    on_delete: async_nullify
 dast_profiles_pipelines:
   - table: ci_pipelines
     column: ci_pipeline_id
  2. Run this spec bin/rspec spec/lib/gitlab/database/loose_foreign_keys_spec.rb
  3. See this failure: 
    Missing a loose foreign key definition for parent table: ci_runners with trigger: ci_runners_loose_fk_trigger. Loose foreign key definitions must be added before triggers are added and triggers must be removed before removing the loose foreign key definition. Read more at https://docs.gitlab.com/ee/development/database/loose_foreign_keys.html .

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #354972 (closed)

Merge request reports