Skip to content

Preload all label-related associations

Adam Hegyi requested to merge 388947-fix-preloading-label-associations into master

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.

Related to #388947 (closed)

Edited by Adam Hegyi

Merge request reports