Reorder source_project_id foreign key constraint

Problem Statement:

Project deletion was occasionally failing due to a SQL timeout while setting source_project_id to nil in merge_requests table.

Deletion failed on <projectname> with the following message: PG::QueryCanceled: ERROR: canceling statement due to statement timeout
CONTEXT: SQL statement "UPDATE ONLY "public"."merge_requests" SET "source_project_id" = NULL WHERE $1 OPERATOR(pg_catalog.=) "source_project_id""

Problem discovery

merge_requests table has ON DELETE SET NULL constraint on the foreign key column source_project_id. This implies when a particular source_project gets deleted, it sets source_project_id as nil for all it's merge requests.

    "fk_3308fe130c" FOREIGN KEY (source_project_id) REFERENCES projects(id) ON DELETE SET NULL

One of the reason why this was happening is:

For a non-forked project, the source_project_id and target_project_id will be same as the project in which the MR is created.

Now, source_project_id has constraint ON DELETE SET NULL and target_project_id has constraint ON DELETE CASCADE.

When the project gets deleted, the source_project_id is first set to nil and then the cascade deletion happens and all records are deleted.

This occurs because the constraint for source_project_id is defined earlier than the target_project_id.

Setting the source_project_id to nil is redundant here, as the merge_request object is soon deleted because of ON DELETE CASCADE constraint on the target_project_id.

Solution:

This MR fixes this issue by re-creating the foreign key so that the target_project_id constraint is placed before source_project_id.

Mentions #295201 (closed)

Migrations

UP

== 20210423171304 ReOrderFkSourceProjectIdInMergeRequests: migrating ==========
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:merge_requests)
   -> 0.0052s
-- execute("ALTER TABLE merge_requests\nADD CONSTRAINT fk_source_project\nFOREIGN KEY (source_project_id)\nREFERENCES projects (id)\nON DELETE SET NULL\nNOT VALID;\n")
   -> 0.0035s
-- execute("SET statement_timeout TO 0")
   -> 0.0007s
-- execute("ALTER TABLE merge_requests VALIDATE CONSTRAINT fk_source_project;")
   -> 0.0090s
-- execute("RESET ALL")
   -> 0.0016s
-- foreign_keys(:merge_requests)
   -> 0.0024s
-- remove_foreign_key(:merge_requests, {:column=>:source_project_id, :name=>"fk_3308fe130c"})
   -> 0.0068s
== 20210423171304 ReOrderFkSourceProjectIdInMergeRequests: migrated (0.0412s) =

DOWN

== 20210423171304 ReOrderFkSourceProjectIdInMergeRequests: reverting ==========
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:merge_requests)
   -> 0.0050s
-- execute("ALTER TABLE merge_requests\nADD CONSTRAINT fk_3308fe130c\nFOREIGN KEY (source_project_id)\nREFERENCES projects (id)\nON DELETE SET NULL\nNOT VALID;\n")
   -> 0.0035s
-- execute("SET statement_timeout TO 0")
   -> 0.0008s
-- execute("ALTER TABLE merge_requests VALIDATE CONSTRAINT fk_3308fe130c;")
   -> 0.0076s
-- execute("RESET ALL")
   -> 0.0010s
-- foreign_keys(:merge_requests)
   -> 0.0031s
-- remove_foreign_key(:merge_requests, {:column=>:source_project_id, :name=>"fk_source_project"})
   -> 0.0054s
== 20210423171304 ReOrderFkSourceProjectIdInMergeRequests: reverted (0.0401s) =

Screenshots (strongly suggested)

Order of foreign key constraint execution (logs):

Before	After

Local testing

1. Enable logs in PostgreSQL

   • gdk psql
   • in psql console: SHOW config_file;
   • open the config file, and edit with the below configuration (edit as per your preference for logging):

   log_destination = 'csvlog'
   logging_collector = on
   log_directory = 'pg_log'
   log_filename = 'postgresql-%Y-%m-%d_%H%M%S.log'
   log_statement = 'all'

2. Since foreign key constraint does not automatically output to logs, create the following triggers in psql console gdk psql:

   CREATE OR REPLACE FUNCTION log_delete() RETURNS trigger AS $$
   BEGIN
      RAISE LOG 'Deleting row % (statement is %)', OLD, current_query();
      RETURN NULL;
      END;
      $$ LANGUAGE plpgsql;
   
   CREATE OR REPLACE FUNCTION log_mr_updates_fn() RETURNS trigger AS $$
   BEGIN
      RAISE LOG 'Updating merge request row % (statement is %)', OLD, current_query();
      RETURN NULL;
      END;
      $$ LANGUAGE plpgsql;
   
   CREATE TRIGGER log_deletions BEFORE DELETE ON projects EXECUTE PROCEDURE log_delete ();
   CREATE TRIGGER log_deletions BEFORE DELETE ON merge_requests EXECUTE PROCEDURE log_delete ();
   CREATE TRIGGER log_mr_updates BEFORE UPDATE ON merge_requests EXECUTE PROCEDURE log_mr_updates_fn ();

   This will trigger logger statement for the below cases:

   • before deletion of a project and merge_request row
   • before update of merge_request

3. In rails console, try deleting a project:

   project = Project.find <id>
   project.destroy!

4. Check logs in <gdk_path>/postgresql/data/pg_log

Repeat steps (3) and (4) with migration up and down steps

Does this MR meet the acceptance criteria?

Conformity

• 📋 Does this MR need a changelog?
  • I have included a changelog entry.
  • I have not included a changelog entry because _____.
• Documentation (if required)
• Code review guidelines
• Merge request performance guidelines
• Style guides
• Database guides
• Separation of EE specific content

Availability and Testing

• Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
• Tested in all supported browsers
• Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

• Label as security and @ mention @gitlab-com/gl-security/appsec
• The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
• Security reports checked/validated by a reviewer from the AppSec team