Preload all label-related associations
What does this MR do and why?
This MR extends the LabelsPreloader
class to preload all associations which are required for authorizing label records and eliminates N+1 queries.
Problem 1: STI
We have two kinds of labels: ProjectLabel
and GroupLabel
. The parent container record (project or group) can be easily preloaded using the parent_container
association however, further associations which are special to the ProjectLabel's
Project
are not possible to preload :
Label.where().preload(parent_container: [:route, :group])
Here, preloading the Group
association would work if the label is a ProjectLabel
(label -> project -> group
) but it would fail for a GroupLabel
(label -> group -> group
). To fix the problem, the LabelsPreloader
was introduced some time ago.
Problem 2: associations
Unfortunately, LabelsPreloader
does not work all the time because we have two associations for the same record:
class GroupLabel < Label
belongs_to :group
belongs_to :parent_container, foreign_key: :group_id, class_name: 'Group'
#...
Preloading the parent_container
is not enough, in some cases, the backend code calls .group
, which is not preloaded and causes N+1
queries. Preloading both associations would double the number of preloading queries.
Problem 3: with_preloaded_container
scope
The with_preloaded_container
scope is not compatible with the LabelsPreloader
because it preloads different associations (similar to problem 2).
Fix
Keep the with_preloaded_container
scope by default so we won't break existing functionality, this needs to be removed at some point and use the LabelsPreloader
everywhere. Follow-up: #391561 (closed)
Where it's explicitly supported and tested (GraphQL), use the LabelsPreloader
without calling the with_preloaded_container
scope. This ensures that all associations are preloaded. This behavior can be enabled with the preload_max_access_levels_for_labels_finder
feature flag.
Test
I've used this script with PG.ai (with the real gitlab-org) group to check how effective the preloading is: query.rb
Results:
action | query count without preload | query count with preload |
---|---|---|
Group labels query | 225 | 19 |
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #388947 (closed)