Skip to content

Update squash commit SHA outside of merge method

Stan Hu requested to merge sh-update-squash-commit-sha-outside-merge into master

What does this MR do and why?

Previously a squash merge would update a merge requests squash_commit_sha within MergeService#try_merge. If a database statement timeout occurred, the merge request would erroneously catch this error and mark the merge request as failed to merge, even though the Gitaly RPC successfully went through.

To avoid this issue, we move the updating the squash_commit_sha to outside #try_merge method. This has the side benefit of reducing the number of UPDATE calls as well, since MergeService updates the merge_commit_sha after #try_merge. If a statement timeout happens here, the MergeWorker will fail and retry, allowing the post-merge activity to run.

Relates to #372376 (closed)

How to set up and validate locally

  1. Create a merge request with at least two commits.
  2. Ensure that Squash commits merge option is enabled.
  3. In the Rails console, verify there is the right data:
[4] pry(main)> MergeRequest.last.squash_commit_sha
  MergeRequest Load (0.6ms)  SELECT "merge_requests".* FROM "merge_requests" ORDER BY "merge_requests"."id" DESC LIMIT 1 /*application:console,db_config_name:main,line:(pry):4:in `__pry__'*/
=> nil
  1. Then click merge:
[5] pry(main)> MergeRequest.last.squash_commit_sha
  MergeRequest Load (0.6ms)  SELECT "merge_requests".* FROM "merge_requests" ORDER BY "merge_requests"."id" DESC LIMIT 1 /*application:console,db_config_name:main,line:(pry):5:in `__pry__'*/
=> "79cdf68ac0f51e3ec8c6d240b219377210fad69a"
[6] pry(main)> MergeRequest.last.merge_commit_sha
  MergeRequest Load (1.2ms)  SELECT "merge_requests".* FROM "merge_requests" ORDER BY "merge_requests"."id" DESC LIMIT 1 /*application:console,db_config_name:main,line:(pry):9:in `__pry__'*/
=> "c588e56b17810da0b93ee98f6118fddd9a60b9ed"

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 Stan Hu

Merge request reports