Add FK from partitioned web_hook_logs to web_hooks (Part 3 of partitioned web_hook_logs migrations)
What does this MR do?
Related issue: #323676 (closed)
We have split !59190 (closed) to three MRs as there is considerable overhead to add each index to each partition of web_hook_logs
. For more information and a full analysis, check !59190 (closed)
This is the third migration, in which we are adding a foreign key from the partitioned web_hook_logs
(web_hook_logs_part_0c5294f417
) to web_hooks
.
We are also adding a new partitioning migration helper (add_concurrent_partitioned_foreign_key
) that allows us to add foreign keys to partitioned tables with minimal locking and while overcoming PostgreSQL's restriction that does not allow adding NOT VALID
foreign keys to partitioned tables.
Does this MR meet the acceptance criteria?
Conformity
-
Does this MR need a changelog?-
I have included a changelog entry.
-
- [-] 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
Merge request reports
Activity
changed milestone to %13.11
1 Warning featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
Mentioning @gitlab-data/engineers to notify the team about changes to the db/structure.sql file.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Category Reviewer Maintainer backend Shreyas Agarwal ( @shreyasagarwal
) (UTC+5, 2 hours ahead of@iroussos
)Kerri Miller ( @kerrizor
) (UTC-7, 10 hours behind@iroussos
)database Alex Ives ( @alexives
) (UTC-5, 8 hours behind@iroussos
)Steve Abrams ( @sabrams
) (UTC-6, 9 hours behind@iroussos
)~migration No reviewer available No maintainer available If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖mentioned in merge request !59190 (closed)
I have started a
gitlab-com-database-testing
pipeline to test the FK addition - quick reference to the job@abrandl this is the MR with the new helper for adding FKs to partitioned tables and the actual migration that will run. I have started a test pipeline, so let's see how it will work :-)
Do you mind checking the helper and tell me if it looks good to you or whether you see any issues?
Also, now that we are moving to PG12 we may want to strip the experimental helpers for adding FKs that reference partitioned tables. Let's think about it and we can remove them in a follow-up MR.
requested review from @abrandl
- A deleted user
added database-testing-automation label
Database migrations
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access). Note that this includes pending migrations from
master
.Migration Total runtime Result DB size change Note 20210329102724 1.5 s +0.00 B (unrelated to this MR, but merged and pending on GitLab.com) 20210409185531 1.1 s -616.27 MiB (unrelated to this MR, but merged and pending on GitLab.com) 20210409185501 186.8 s +721.55 MiB (unrelated to this MR, but merged and pending on GitLab.com) 20210407150240 1.4 s +8.00 KiB (unrelated to this MR, but merged and pending on GitLab.com) 20210413132500 16.6 s +0.00 B (unrelated to this MR, but merged and pending on GitLab.com) 20210413130011 1249.6 s +32.00 KiB 20210412142223 17.2 s +19.74 MiB (unrelated to this MR, but merged and pending on GitLab.com) 20210413123832 63527.1 s +13.19 GiB (unrelated to this MR, but merged and pending on GitLab.com) Migration: 20210413130011
- Duration: 1249.6 s
- Database size change: +32.00 KiB
Query Calls Total Time Max Time Mean Time Rows ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202104 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 892986.3 ms 892986.3 ms 892986.3 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202103 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 173021.1 ms 173021.1 ms 173021.1 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202008 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 36172.7 ms 36172.7 ms 36172.7 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202012 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 32300.2 ms 32300.2 ms 32300.2 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202101 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 32122.9 ms 32122.9 ms 32122.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202102 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 28552.1 ms 28552.1 ms 28552.1 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202011 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 18308.3 ms 18308.3 ms 18308.3 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202010 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 16512.5 ms 16512.5 ms 16512.5 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202009 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 12755.1 ms 12755.1 ms 12755.1 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_000000 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 115.2 ms 115.2 ms 115.2 ms 0 ALTER TABLE "web_hook_logs_part_0c5294f417" ADD CONSTRAINT "fk_rails_bb3355782d" FOREIGN KEY ("web_hook_id") REFERENCES "web_hooks" ("id") ON DELETE CASCADE /*application:test*/
1 41.6 ms 41.6 ms 41.6 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_000000 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 34.7 ms 34.7 ms 34.7 ms 0 SELECT "postgres_partitions".*
FROM "postgres_partitions"
WHERE "postgres_partitions"."parent_identifier" = $1
ORDER BY "postgres_partitions"."name" ASC /*application:test*/1 9.4 ms 9.4 ms 9.4 ms 17 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202109 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 8.2 ms 8.2 ms 8.2 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202108 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 7.8 ms 7.8 ms 7.8 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202106 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 7.6 ms 7.6 ms 7.6 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202105 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 5.2 ms 5.2 ms 5.2 ms 0 SELECT "postgres_partitioned_tables".*
FROM "postgres_partitioned_tables"
WHERE (identifier = concat(current_schema(), $1, $2))
LIMIT $3 /*application:test*/1 4.4 ms 4.4 ms 4.4 ms 1 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202007 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 4.0 ms 4.0 ms 4.0 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202102 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 2.5 ms 2.5 ms 2.5 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202103 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.9 ms 1.9 ms 1.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202107 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 1.6 ms 1.6 ms 1.6 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202008 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.6 ms 1.6 ms 1.6 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202105 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 1.4 ms 1.4 ms 1.4 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202106 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 1.3 ms 1.3 ms 1.3 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202108 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 1.3 ms 1.3 ms 1.3 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202101 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.3 ms 1.3 ms 1.3 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202110 VALIDATE CONSTRAINT fk_rails_bb3355782d
1 1.2 ms 1.2 ms 1.2 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202104 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.2 ms 1.2 ms 1.2 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202110 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.1 ms 1.1 ms 1.1 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202012 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 1.0 ms 1.0 ms 1.0 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202009 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.9 ms 0.9 ms 0.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202011 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.9 ms 0.9 ms 0.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202109 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.9 ms 0.9 ms 0.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202107 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.9 ms 0.9 ms 0.9 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202007 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.8 ms 0.8 ms 0.8 ms 0 ALTER TABLE gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202010 ADD CONSTRAINT fk_rails_bb3355782d FOREIGN KEY (web_hook_id) REFERENCES web_hooks (id) ON DELETE CASCADE NOT VALID
1 0.8 ms 0.8 ms 0.8 ms 0 Artifacts
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Contributions welcome! Epic
- Resolved by Andreas Brandl
@abrandl the good news is that by both checking the migration output and the recorded queries above, it's clear that adding the final foreign key to the partitioned table does not result to revalidating all the partitions
So our approach is solid and works as expected.
With respect to the time it takes to add the FKs to all the partitions, it's a lot but it should not affect the database (and hopefully it will be much faster on production but let's discuss without taking that into account).
We could get down the rabbit hole of investigating solutions that would allow us to add the indexes and the FKs one partition at a time, split between deployments, but as partitions are dynamic for each instance that could be tricky and I am not sure if it is worth it.
added 396 commits
added databaseapproved label and removed databaseactive label
added 156 commits
-
326202f3...6eac70da - 155 commits from branch
master
- 7b7e4350 - Add FK to partitioned web_hook_logs
-
326202f3...6eac70da - 155 commits from branch
mentioned in epic &4785 (closed)
added 99 commits
-
7b7e4350...47d8b264 - 98 commits from branch
master
- 1298a19e - Add FK to partitioned web_hook_logs
-
7b7e4350...47d8b264 - 98 commits from branch
@abrandl the migrations for adding the indexes have reached production:
gitlabhq_production=> \d web_hook_logs_part_0c5294f417 ... ... ... Partition key: RANGE (created_at) Indexes: "web_hook_logs_part_0c5294f417_pkey" PRIMARY KEY, btree (id, created_at) "index_web_hook_logs_part_on_created_at_and_web_hook_id" btree (created_at, web_hook_id) "index_web_hook_logs_part_on_web_hook_id" btree (web_hook_id) Number of partitions: 17 (Use \d+ to list them.) gitlabhq_production=> \d gitlab_partitions_dynamic.web_hook_logs_part_0c5294f417_202104 ... ... ... Partition of: web_hook_logs_part_0c5294f417 FOR VALUES FROM ('2021-04-01 00:00:00') TO ('2021-05-01 00:00:00') Indexes: "web_hook_logs_part_0c5294f417_202104_pkey" PRIMARY KEY, btree (id, created_at) "index_5dc0dc60d8" btree (created_at, web_hook_id) "index_d6756f0be7" btree (web_hook_id)
I have made a small fix to cover the deprecation of sending hashes as params, but other than that everything is exactly the same as the version you already reviewed (I have of course tested that this change works as expected):
--- a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb @@ -59,11 +59,11 @@ def add_concurrent_partitioned_foreign_key(source, target, column:, on_delete: : partitioned_table = find_partitioned_table(source) partitioned_table.postgres_partitions.order(:name).each do |partition| - add_concurrent_foreign_key(partition.identifier, target, partition_options) + add_concurrent_foreign_key(partition.identifier, target, **partition_options) end with_lock_retries do - add_foreign_key(source, target, partition_options) + add_foreign_key(source, target, **partition_options) end end
Do you mind reviewing the one last time and setting to MWPS if everything looks good to you?
enabled an automatic merge when the pipeline for 5aaee635 succeeds
added 52 commits
-
1298a19e...21beb9ce - 51 commits from branch
master
- 85dbefe3 - Add FK to partitioned web_hook_logs
-
1298a19e...21beb9ce - 51 commits from branch
enabled an automatic merge when the pipeline for 4ac76e76 succeeds
mentioned in commit 887e7b54
mentioned in issue #323676 (closed)
mentioned in issue gitlab-org/database-team/team-tasks#165 (closed)
added workflowstaging label and removed workflowin review label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
added releasedcandidate label
added devopsdata stores label and removed devopssystems label
added groupdatabase frameworks label and removed groupdatabase [DEPRECATED] label
added pipeline:mr-approved label