Skip to content

Swap FK merge_requests.head_pipeline_id to ci_pipelines for LFK

Kamil Trzciński requested to merge remove-ci_pipelines_merge_requests_head_pipeline_id-fk into master

What does this MR do and why?

Per &7249 (closed)

As part of our CI "decomposition" efforts we need to remove all foreign keys that are cross-database (ie. between the planned main and ci databases). We are going to replace them all with "loose foreign keys".

Related: #348272 (closed)

Validations

  1. Best team to review (check off when reviewed): grouppipeline execution devopsverify
  2. No way for user to access once parent is deleted. Please explain: @fabiopitino: MergeRequest#actual_head_pipeline is used primarily rather than head_pipeline directly which uses a more accurate finder. In both cases it would return nil if the record is not found.
  3. Possible to access once parent deleted but low user impact. Please explain: @fabiopitino: We expose head_pipeline_id in some places like REST API or webhook payload but this has low impact since requests to pipeline ID would return 404.
  4. Possible Sidekiq workers that may load directly and possibly lead to exceptions. Please explain: <DETAIL>
  5. Possible user impact to be evaluated or mitigated. Please explain: there is a minor UX impact where we show "Checking pipeline status" indefinitely if they loaded the MR when the head_pipeline_id is not null and refers to a pipeline that is now deleted. Could be mitigated by changing the loading state indicator code. There weren't any errors though. We should investigate closer though as this is a critical part of the application. We should see what happens if you try to merge an MR in this state.

MR acceptance checklist

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

Edited by Fabio Pitino

Merge request reports