Commit a8961dca authored by Nick Thomas's avatar Nick Thomas

Resolve an N+1 in merge request CI variables

When building a CI pipeline, variables must be gathered at least once
per build, so it's important that the variable declarations ensure any
activerecord-dependent data is cached on the first call, to avoid
repeated SQL queries.

For this case, `present?` loads the list of assignees into memory for
subsequent calls, while `any?` issues a simple `EXISTS` query that is
repeated each time.
parent 919fd2dc
......@@ -1290,7 +1290,7 @@ class MergeRequest < ApplicationRecord
variables.append(key: 'CI_MERGE_REQUEST_PROJECT_URL', value: project.web_url)
variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME', value: target_branch.to_s)
variables.append(key: 'CI_MERGE_REQUEST_TITLE', value: title)
variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee_username_list) if assignees.any?
variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee_username_list) if assignees.present?
variables.append(key: 'CI_MERGE_REQUEST_MILESTONE', value: milestone.title) if milestone
variables.append(key: 'CI_MERGE_REQUEST_LABELS', value: label_names.join(',')) if labels.present?
variables.concat(source_project_variables)
......
---
title: Resolve an N+1 in merge request CI variables
merge_request: 28688
author:
type: performance
......@@ -3701,4 +3701,18 @@ describe MergeRequest do
end
end
end
describe '#predefined_variables' do
let(:merge_request) { create(:merge_request) }
it 'caches all SQL-sourced data on the first call' do
control = ActiveRecord::QueryRecorder.new { merge_request.predefined_variables }.count
expect(control).to be > 0
count = ActiveRecord::QueryRecorder.new { merge_request.predefined_variables }.count
expect(count).to eq(0)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment