Skip to content
Snippets Groups Projects

Improve the performance of CI predefined_merge_request_variables

Merged Furkan Ayhan requested to merge 515980-preload-merge-request-approved into master
All threads resolved!

What does this MR do and why?

This method is slow because of the MergeRequest#approved? method. Especially in EE, there is so much logic and it takes a lot of time.

In the codebase, the preloads of the MR associations are handled but not in this part. I am copying the preload logic here but I created another issue for the related team to handle all preloads of merge requests. (https://gitlab.com/gitlab-org/gitlab/-/issues/515959)

Related to #515980 (closed)

Feature flag: ci_merge_request_variables_preload (#515982)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

#515980 (comment 2322616243)

Edited by Furkan Ayhan

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 1 Warning
    :warning: The master pipeline status page reported failures in

    If these jobs fail in your merge request with the same errors, then they are not caused by your changes.
    Please check for any on-going incidents in the incident issue tracker or in the #master-broken Slack channel.

    1 Message
    :book: CHANGELOG missing:

    If this merge request needs a changelog entry, add the Changelog trailer to the commit message you want to add to the changelog.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    Reviewer roulette

    Category Reviewer Maintainer
    backend @Saahmed profile link current availability (UTC-5, 6 hours behind author) @felipe_artur profile link current availability (UTC-3, 4 hours behind author)
    devopsverify Reviewer review is optional for devopsverify @lauraXD profile link current availability (UTC+1, same timezone as author)

    Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by ****
  • Furkan Ayhan
  • Furkan Ayhan added 1 commit

    added 1 commit

    • 21efedd2 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Furkan Ayhan requested review from @mbobin

    requested review from @mbobin

  • Furkan Ayhan requested review from @hfyngvason

    requested review from @hfyngvason

  • Hordur Freyr Yngvason approved this merge request

    approved this merge request

  • Before you set this MR to auto-merge

    This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.

    Before you set this MR to auto-merge, please check the following:

    • You are the last maintainer of this merge request
    • The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
    • This pipeline is recent enough (created in the last 8 hours)

    If all the criteria above apply, please set auto-merge for this merge request.

    See pipeline tiers and merging a merge request for more details.

  • E2E Test Result Summary

    allure-report-publisher generated test report!

    e2e-test-on-cng: :white_check_mark: test report for 21efedd2

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Plan        | 86     | 0      | 8       | 0     | 94    | ✅     |
    | Create      | 143    | 0      | 19      | 0     | 162   | ✅     |
    | Verify      | 53     | 0      | 19      | 0     | 72    | ✅     |
    | Data Stores | 33     | 0      | 10      | 0     | 43    | ✅     |
    | Govern      | 84     | 0      | 10      | 0     | 94    | ✅     |
    | Package     | 29     | 0      | 15      | 0     | 44    | ✅     |
    | Monitor     | 8      | 0      | 12      | 0     | 20    | ✅     |
    | Fulfillment | 2      | 0      | 7       | 0     | 9     | ✅     |
    | ModelOps    | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Ai-powered  | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Secure      | 2      | 0      | 5       | 0     | 7     | ✅     |
    | Manage      | 1      | 0      | 9       | 0     | 10    | ✅     |
    | Release     | 5      | 0      | 1       | 0     | 6     | ✅     |
    | Analytics   | 2      | 0      | 0       | 0     | 2     | ✅     |
    | Growth      | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Configure   | 0      | 0      | 3       | 0     | 3     | ➖     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 448    | 0      | 123     | 0     | 571   | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+

    e2e-test-on-gdk: :white_check_mark: test report for 21efedd2

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Create      | 276    | 0      | 40      | 0     | 316   | ✅     |
    | Package     | 48     | 0      | 28      | 0     | 76    | ✅     |
    | Data Stores | 66     | 0      | 20      | 0     | 86    | ✅     |
    | Verify      | 104    | 0      | 40      | 2     | 144   | ✅     |
    | Govern      | 158    | 0      | 26      | 0     | 184   | ✅     |
    | Plan        | 164    | 0      | 16      | 0     | 180   | ✅     |
    | Analytics   | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Monitor     | 16     | 0      | 24      | 0     | 40    | ✅     |
    | Configure   | 0      | 0      | 6       | 0     | 6     | ➖     |
    | Fulfillment | 4      | 0      | 14      | 0     | 18    | ✅     |
    | Growth      | 0      | 0      | 4       | 0     | 4     | ➖     |
    | Secure      | 8      | 0      | 6       | 0     | 14    | ✅     |
    | Manage      | 2      | 0      | 18      | 0     | 20    | ✅     |
    | Release     | 10     | 0      | 2       | 0     | 12    | ✅     |
    | ModelOps    | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Ai-powered  | 0      | 0      | 4       | 0     | 4     | ➖     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 860    | 0      | 250     | 2     | 1110  | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+
    Edited by ****
  • Marius Bobin approved this merge request

    approved this merge request

  • Marius Bobin resolved all threads

    resolved all threads

  • Marius Bobin enabled automatic add to merge train when checks pass

    enabled automatic add to merge train when checks pass

  • merged

  • Marius Bobin mentioned in commit b64e3658

    mentioned in commit b64e3658

  • added workflowstaging label and removed workflowcanary label

  • Furkan Ayhan changed the description

    changed the description

  • Furkan Ayhan mentioned in merge request !180243 (merged)

    mentioned in merge request !180243 (merged)

  • Please register or sign in to reply
    Loading