PoC: Store merge head diff in database
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
- Since we're merging to ref whenever we check for mergeability, we need to delete and create merge head diffs often.
- Looking more complex since now we have to differentiate between non-merge head diffs and merge head diffs.
For improvements
- Explain why we need
merge_head
distinction or find a better way. - Index on
merge_head
attribute. - 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
. - 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. - Make the
@comparable_diffs
ivar not an ivar. - Add a unique index on
merge_request_id
andmerge_head
attribute. Add a validation too. - Confirm that project import/export still works as expected.
Does this MR meet the acceptance criteria?
Conformity
-
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
Edited by Patrick Bajao