Skip to content

Swap FK ci_pipelines.merge_request_id to merge_requests for LFK

Kamil Trzciński requested to merge remove-merge_requests_ci_pipelines_merge_request_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: #348270 (closed)

Validations

  • Best team to review (check off when reviewed): @fabiopitino grouppipeline execution devopsverify
  • No way for user to access once parent is deleted. Please explain:
  • Possible to access once parent deleted but low user impact. Please explain:
  • Possible Sidekiq workers that may load directly and possibly lead to exceptions. Please explain:
  • Possible user impact to be evaluated or mitigated. Please explain: In Ci::Pipeline#merge_request? we rely solely on the presence of the merge_request_id. If present we assume that we can safely use pipeline.merge_request object. This won't be the case once the FK is removed. You can grep for merge_request? in Ci::Pipeline model.
    • required remediation: change merge_request? method to merge_request_id.present? && merge_request. This should also be safe from performance perspective because all the places where we check for merge_request? we end up using the memoized object.

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 Kamil Trzciński

Merge request reports