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:metadataand:tags, which no longer exist on these classes, so they are removed. executeaccepts an optionalscope: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::CancelPipelineServiceandCi::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
- Related to #629310
- Part of splitting !255978 (merged)
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.rbMR 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.