Only move state if the state will change while updating a merge request (!53537) · Merge requests · GitLab.org / GitLab

What does this MR do?

The thing I noticed is that we are calling mark_as_unchecked for every open merge request where the branch is the source or target.

One potential improvement is that we can only make this call if it will update the merge status state. It looks like this could be done since it doesn't seem like we do anything from unchecked -> unchecked, or cannot_be_merged_recheck -> cannot_be_merged_recheck 🤔

NOTE: This is on a very simple merge request set, where no transitions are actually made so the results won't be as good in production

Using very basic merge requests, set up via:

for i in {1..500}
do
git checkout -b "$i";
echo "$i" >> $i.txt;
git add .; git commit -am 'test';
git push origin "$i";
done

for i in {1..500}
do
curl -X POST --header "PRIVATE-TOKEN: INSERT_TOKEN_HERE" --data "source_branch=$i&target_branch=master&title=test" http://127.0.0.1:3000/api/v4/projects/6/merge_requests
done

Baseline:

"2021-02-18_08":"54":04.52024 [
   "34mrails-background-jobs":""[
      0m{
         "severity":"INFO",
         "time":"2021-02-18T08:54:04.519Z",
         "class":"UpdateMergeRequestsWorker",
         "args":[
            "701",
            "1",
            "17a28bb7181854683abab97fa44ca1d3c488d3fb",
            "1db4f3fe00dd4305eb35a99afd9ee9148ca5ff17",
            "refs/heads/master"
         ],
         "retry":3,
         "queue":"update_merge_requests",
         "backtrace":true,
         "version":0,
         "jid":"d52bfda0b0db216141c55bf8",
         "created_at":"2021-02-18T08:53:38.133Z",
         "correlation_id":"e06914f809766977bf2e48b58ade201e",
         "enqueued_at":"2021-02-18T08:53:38.146Z",
         "pid":68512,
         "message":"UpdateMergeRequestsWorker JID-d52bfda0b0db216141c55bf8: done: 26.372092 sec",
         "job_status":"done",
         "scheduling_latency_s":0.001338,
         "job_size_bytes":378,
         "cpu_s":20.45702,
         "gitaly_calls":5,
         "gitaly_duration_s":0.131336,
         "rugged_calls":27,
         "rugged_duration_s":0.031214,
         "db_count":6051,
         "db_write_count":9,
         "db_cached_count":1237,
         "duration_s":26.372092,
         "completed_at":"2021-02-18T08:54:04.519Z",
         "db_duration_s":4.344313
      }

Only moving state when a change will occur

"2021-02-18_08":"49":03.93549 [
   "34mrails-background-jobs":""[
      0m{
         "severity":"INFO",
         "time":"2021-02-18T08:49:03.935Z",
         "class":"UpdateMergeRequestsWorker",
         "args":[
            "701",
            "1",
            "17a28bb7181854683abab97fa44ca1d3c488d3fb",
            "1db4f3fe00dd4305eb35a99afd9ee9148ca5ff17",
            "refs/heads/master"
         ],
         "retry":3,
         "queue":"update_merge_requests",
         "backtrace":true,
         "version":0,
         "jid":"21425c4a54a890da58c3cbca",
         "created_at":"2021-02-18T08:48:45.560Z",
         "correlation_id":"5dd1f764ab92e40f72d0852166baf67a",
         "enqueued_at":"2021-02-18T08:48:45.562Z",
         "pid":65380,
         "message":"UpdateMergeRequestsWorker JID-21425c4a54a890da58c3cbca: done: 17.647129 sec",
         "job_status":"done",
         "scheduling_latency_s":0.725921,
         "job_size_bytes":377,
         "cpu_s":3.939844,
         "gitaly_calls":2,
         "gitaly_duration_s":0.049866,
         "rugged_calls":27,
         "rugged_duration_s":0.057831,
         "db_count":1211,
         "db_write_count":0,
         "db_cached_count":3,
         "duration_s":17.647129,
         "completed_at":"2021-02-18T08:49:03.935Z",
         "db_duration_s":0.928852
      }

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

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

Only move state if the state will change while updating a merge request