Skip to content

Refactoring merge train refresh service

Shinya Maeda requested to merge remove-tech-debts-on-refresh-mr-services into master

What does this MR do?

This MR resolves the ~"technical debt" around merge train refreshing service. Specifically,

  • Improve the readability on MergeTrain class to return MergeTrain object as car. The complete terminology refactoring will be done in #298541 (closed).
  • Remove the debug-logging in the RefreshMergeRequestsService as the culprit was identified and resolved a long time ago.
  • Remove unused/dead code for better readability
  • Rename RefreshMergeRequestsService to MergeTrains::RefreshService.
  • MergeTrains::RefreshService is the single service to refresh all merge requests on a train.
  • MergeTrains::RefreshService receives target_project_id and target_branch as arguments as the definition of a train. Previously, it received merge_request object but it's no longer relevant after the process change.
  • Introduce MergeTrains::RefreshWorker as the single worker to execute MergeTrains::RefreshService service.
  • Reduce redundant database queries.
  • Add an explanation on the RefreshMergeTrainService process flow.

Close #281065 (closed)

Related !49520 (merged)

Manual QA

  • Conditions:
    • Create a sample project
    • Created 10 MRs and add to the merge train
for i in {1..10}; do
export identifier=$(date +%s) &&
  git checkout master &&
  git co -b "feature-$identifier" &&
  echo 'a' > $identifier.txt &&
  git add . &&
  git commit -m "Test MR $identifier" &&
  git push origin -o merge_request.create
done
  • Expectations:
    • All MRs are merged.
    • Both auto_merge:auto_merge_process and auto_merge:merge_trains_refresh worker jobs are correctly processed.
  • Result: PASS

Screenshots (strongly suggested)

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 Shinya Maeda

Merge request reports