<!-- Please review https://about.gitlab.com/handbook/engineering/infrastructure/change-management/ for the most recent information on our change plans and execution policies. --> # Production Change ### Change Summary We had a [recent incident](https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6642) where the post-migration for removing a foreign key on the `ci_builds` table has failed. We couldn't acquire the exclusive lock after several retries. To mitigate the issue the migration was marked "done" and it was skipped. This change request is for manually removing the foreign key during off-peak hours. It's important to remove this foreign key because it's responsible for another incident: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6625 ### Change Details 1. **Services Impacted** - Database 1. **Change Technician** - @msmiley @cindy 1. **Change Reviewer** - @pbair, @msmiley 1. **Time tracking** - 15-60 minutes 1. **Downtime Component** - <!-- woodhouse: '{{ .Downtime }}' --> No hard downtime is expected, but this will cause lock contention during each attempt. We are limiting the duration of that contention by setting `statement_timeout`. If the contention lasts long enough, it could saturate the pgbouncer connection pool, which could lead to a brief burst of slowness and timeouts. ## Detailed steps for the change ### Pre-Change Steps - steps to be completed before execution of the change *Estimated Time to Complete (3 mins)* - {+Estimated Time to Complete in Minutes+} - [x] Verify that the foreign key is still present PSQL console: ``` \d+ ci_runners ``` You should see the following FK entry: ``` TABLE "ci_builds" CONSTRAINT "fk_e4ef9c2f27" FOREIGN KEY (runner_id) REFERENCES ci_runners(id) ON DELETE SET NULL NOT VALID ``` ### Change Steps - steps to take to execute the change *Estimated Time to Complete (15 mins)* - {+Estimated Time to Complete in Minutes+} - [x] Remove the foreign key ```sql BEGIN; SET LOCAL statement_timeout='3s'; -- can be increased to 5-8s LOCK ci_builds, ci_runners IN ACCESS EXCLUSIVE MODE; ALTER TABLE ci_builds DROP CONSTRAINT fk_e4ef9c2f27; COMMIT; ``` If the locks cannot be acquired on the two tables, the statement can fail with a statement timeout. A `ROLLBACK;` can be issued to abort the current transaction, and the process can be retried. You might need to manually adjust the `statement_timeout` if the statement fails with statement timeout. ### Post-Change Steps - steps to take to verify the change *Estimated Time to Complete (3 mins)* - {+Estimated Time to Complete in Minutes+} - [x] Verify that the foreign key is gone PSQL console: ``` \d+ ci_runners ``` You shouldn't see `fk_e4ef9c2f27`. ## Rollback Rollback is not necessary. This FK is a `NOT VALID` FK and needs to be replaced with loose foreign keys: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83129 ## Monitoring ### Key metrics to observe A few seconds of lock timeout is not too noticeable, we might get a slight APDEX drop. ## Summary of infrastructure changes - [ ] Does this change introduce new compute instances? - [ ] Does this change re-size any existing compute instances? - [ ] Does this change introduce any additional usage of tooling like Elastic Search, CDNs, Cloudflare, etc? <!-- * If you answer yes to any of the items in this checklist, summarize below. --> {+Summary of the above+} ## Change Reviewer checklist <!-- To be filled out by the reviewer. --> ~C4 ~C3 ~C2 ~C1: - [x] The **scheduled day and time** of execution of the change is appropriate. - [x] The [change plan](#detailed-steps-for-the-change) is technically accurate. - [x] The change plan includes **estimated timing values** based on previous testing. - [x] The change plan includes a viable [rollback plan](#rollback). - [x] The specified [metrics/monitoring dashboards](#key-metrics-to-observe) provide sufficient visibility for the change. ~C2 ~C1: - [ ] The complexity of the plan is appropriate for the corresponding risk of the change. (i.e. the plan contains clear details). - [ ] The change plan includes success measures for all steps/milestones during the execution. - [ ] The change adequately minimizes risk within the environment/service. - [ ] The performance implications of executing the change are well-understood and documented. - [ ] The specified metrics/monitoring dashboards provide sufficient visibility for the change. - If not, is it possible (or necessary) to make changes to observability platforms for added visibility? - [ ] The change has a primary and secondary SRE with knowledge of the details available during the change window. ## Change Technician checklist <!-- To find out who is on-call, in #production channel run: /chatops run oncall production. --> - [x] This issue has a criticality label (e.g. ~C1, ~C2, ~C3, ~C4) and a change-type label (e.g. ~"change::unscheduled", ~"change::scheduled") based on the [Change Management Criticalities](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#change-criticalities). - [x] This issue has the change technician as the assignee. - [x] Pre-Change, Change, Post-Change, and Rollback steps and have been filled out and reviewed. - [x] This Change Issue is linked to the appropriate Issue and/or Epic - [ ] Necessary approvals have been completed based on the [Change Management Workflow](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#change-request-workflows). - [ ] Change has been tested in staging and results noted in a comment on this issue. - [ ] A dry-run has been conducted and results noted in a comment on this issue. - [x] SRE on-call has been informed prior to change being rolled out. (In #production channel, mention `@sre-oncall` and this issue and await their acknowledgement.) - [ ] Release managers have been informed (If needed! Cases include DB change) prior to change being rolled out. (In #production channel, mention `@release-managers` and this issue and await their acknowledgment.) - [ ] There are currently no [active incidents](https://gitlab.com/gitlab-com/gl-infra/production/-/issues?scope=all&utf8=%E2%9C%93&state=opened&label_name[]=Incident%3A%3AActive).