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
-
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'
-
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
andmerge_request
row - before update of
merge_request
- before deletion of a
-
In
rails console
, try deleting a project:project = Project.find <id> project.destroy!
-
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