Load each association once per defining class in CommitStatusPreloader

What does this MR do and why?

Ci::Preloaders::CommitStatusPreloader#execute preloads associations on a batch mixing Ci::Build, Ci::Bridge and GenericCommitStatus. Rails raises when a record's class lacks a requested association, so it ran one preloader per class. Associations that several classes define (project, pipeline, needs, job_definition and others) were queried once per class, with a separate Ruby object per class for the same row.

Now execute groups relations by the set of classes that define each association and preloads each group once over records of those classes. For example, Ci::Stage#preload_metadata saves 3 queries on stages that have builds and bridges.

  • Relations are normalized to one entry per association name, so a hash with several keys can be split between groups.
  • Names no class defines are reported with Gitlab::ErrorTracking.track_and_raise_for_dev_exception, then skipped. This raises in development and test; in production it only reports. The preloader's spec was passing :metadata and :tags, which no longer exist on these classes, so they are removed.
  • execute accepts an optional scope: keyword for Rails' preloader, which a follow-up MR uses for strict loading.
  • Ci::DropPipelineService's N+1 spec now compares runs with the same job mix. Sharing the project instance made its old control run three reads cheaper, so the example failed by one query.
  • The callers (Ci::Stage#preload_metadata, Ci::StagePresenter, Types::Ci::StageType, Ci::CancelPipelineService and Ci::DropPipelineService) were checked, and their relation lists match what the classes define.
Why redeclared associations still load in one query

Ci::Build and Ci::Bridge redeclare project and pipeline, so each class has its own reflection. Rails batches loaders whose query is the same, so these still load in one query with one instance per row. The spec pins the query count and instance identity. If Rails changed this, loading would fall back to one query per class, not to wrong results.

Database review notes. No new query shapes. For associations shared by several classes, lookups go from one query per class to one query per batch.

Feature flag

Skipped, labelled feature flagskipped. This changes how a fixed set of associations is loaded, not what callers see. All callers pass literal lists, so the names reaching the preloader are a closed set. A future typo is reported rather than failing the request. The follow-ups that use the new behaviour have their own flags.

References

Screenshots or screen recordings

No UI change.

How to set up and validate locally

Run the preloader spec and the specs of its callers:

bin/rspec spec/models/ci/preloaders/commit_status_preloader_spec.rb spec/services/ci/drop_pipeline_service_spec.rb spec/presenters/ci/stage_presenter_spec.rb spec/graphql/types/ci/stage_type_spec.rb spec/services/ci/cancel_pipeline_service_spec.rb spec/models/ci/stage_spec.rb

MR acceptance checklist

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

Edited by Hordur Freyr Yngvason

Merge request reports

Loading
Loading