Fix three N+1s in Releases API entity generation
What does this MR do?
Fixes three N+1 problems in the Releases API entity generation:
-
expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? }
Without preloading
author
, this adds another query for each release (CACHEd in my example because it was the author, but still an N+1):D, [2021-04-25T01:05:43.566702 #10489] DEBUG -- : CACHE User Load (0.1ms) SELECT "users".* FROM "users" WHERE "users"."id" = 1 LIMIT 1 [["id", 1], ["LIMIT", 1]] D, [2021-04-25T01:05:43.627533 #10489] DEBUG -- : CACHE User Load (0.2ms) SELECT "users".* FROM "users" WHERE "users"."id" = 1 LIMIT 1 [["id", 1], ["LIMIT", 1]]
-
This is actually assets_count, which is the sum of links + sources:
def assets_count(except: []) links_count = links.count sources_count = except.include?(:sources) ? 0 : sources.count links_count + sources_count end
Without the change from
.count
to.size
and preloading links, there's an extra count for each release, i.e.:D, [2021-04-25T00:25:11.341606 #5804] DEBUG -- : (0.6ms) /*application:console,correlation_id:d534f17c-956b-4c75-aaee-f180c1f9f56a,endpoint_id:/api/:version/projects/:id/releases*/ SELECT COUNT(*) FROM "release_links" WHERE "release_links"."release_id" = 1129 D, [2021-04-25T00:25:11.345848 #5804] DEBUG -- : ↳ app/models/release.rb:65:in `assets_count' D, [2021-04-25T00:25:11.345947 #5804] DEBUG -- : ↳ app/presenters/release_presenter.rb:66:in `assets_count' D, [2021-04-25T00:25:11.299277 #5804] DEBUG -- : (0.6ms) /*application:console,correlation_id:d534f17c-956b-4c75-aaee-f180c1f9f56a,endpoint_id:/api/:version/projects/:id/releases*/ SELECT COUNT(*) FROM "release_links" WHERE "release_links"."release_id" = 1130 D, [2021-04-25T00:25:11.301665 #5804] DEBUG -- : ↳ app/models/release.rb:65:in `assets_count' D, [2021-04-25T00:25:11.301742 #5804] DEBUG -- : ↳ app/presenters/release_presenter.rb:66:in `assets_count' [..]
-
expose :links, using: Entities::Releases::Link do |release, options| release.links.sorted end
A bit tricker, because
sorted
is a scope on theRelease::Link
model, so we need to add asorted_links
scope on the Release model and preload that to prevent sorting links for every release, i.e. the "before":D, [2021-04-25T00:25:11.423087 #5804] DEBUG -- : Releases::Link Load (3.1ms) /*application:console,correlation_id:d534f17c-956b-4c75-aaee-f180c1f9f56a,endpoint_id:/api/:version/projects/:id/releases*/ SELECT "release_links".* FROM "release_links" WHERE "release_links"."release_id" = 1128 ORDER BY "release_links"."created_at" DESC D, [2021-04-25T00:25:11.473897 #5804] DEBUG -- : Releases::Link Load (0.7ms) /*application:console,correlation_id:d534f17c-956b-4c75-aaee-f180c1f9f56a,endpoint_id:/api/:version/projects/:id/releases*/ SELECT "release_links".* FROM "release_links" WHERE "release_links"."release_id" = 1127 ORDER BY "release_links"."created_at" DESC [..]
Screenshots (strongly suggested)
No screenshots, but query count and timing data:
Before:
root@cat-gpt-10k-gitlab-rails-1:~# zcat /var/log/gitlab/gitlab-rails/api_json.log.1.gz | grep '^{' | jq -r 'select(."route" == "/api/:version/projects/:id/releases") | [.db_count, .db_duration_s] | @tsv' | head -n 5
83 0.11487
83 0.113
83 0.11209
81 0.11357
83 0.10504
After:
root@cat-gpt-10k-gitlab-rails-1:~# zcat /var/log/gitlab/gitlab-rails/api_json.log.1.gz | grep '^{' | jq -r 'select(."route" == "/api/:version/projects/:id/releases") | [.db_count, .db_duration_s] | @tsv' | tail -n 5
18 0.02614
18 0.02553
18 0.0265
18 0.02607
18 0.02612
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team