Skip to content

PoC: Store merge head diff in database

Patrick Bajao requested to merge merge-head-diff-poc into master

What does this MR do?

Re-use MergeRequestDiff model and tables with additional merge_head boolean attribute. This is to identify that the MR diff is a merge head diff.

This allow us to use the batch diffs performance improvements and sorting capability implemented in #26552 (closed).

NOTE: This MR is not meant to be merged as it's a spike.

Trade-offs

  1. Since we're merging to ref whenever we check for mergeability, we need to delete and create merge head diffs often.
  2. Looking more complex since now we have to differentiate between non-merge head diffs and merge head diffs.

For improvements

  1. Explain why we need merge_head distinction or find a better way.
  2. Index on merge_head attribute.
  3. Or, store it in separate tables to avoid conditionals but I imagine this will require us creating more indexes and tables as we need to replicate the indexes/associated tables we have in merge_request_diffs.
  4. Move the call for Discussions::CaptureDiffNotePositionsService to the new service. Intentionally left it in its current place to highlight the things that we need to do to make this work.
  5. Make the @comparable_diffs ivar not an ivar.
  6. Add a unique index on merge_request_id and merge_head attribute. Add a validation too.
  7. Confirm that project import/export still works as expected.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by Patrick Bajao

Merge request reports