From 5010be7766d08a9adee51c7d16ba71c19ff7dede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 9 Aug 2016 12:20:09 +0200 Subject: [PATCH] Improve the performance of the GET /:sources/:id/{access_requests,members} API endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The performance was greatly improved by removing two N+1 queries issues for each endpoint. For comparison: 1. `GET /projects/:id/members` With two N+1 queries issues (one was already fxed in the following snippet): ``` ProjectMember Load (0.2ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL ORDER BY "members"."id" DESC [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (5, 22, 16, 13) ORDER BY "users"."id" DESC ActiveRecord::SchemaMigration Load (0.2ms) SELECT "schema_migrations".* FROM "schema_migrations" ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 5]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 22]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 16]] ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"], ["user_id", 13]] ``` Without the N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL ORDER BY "members"."id" DESC LIMIT 20 OFFSET 0 [["source_type", "Project"], ["source_id", 1], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (5, 22, 16, 13) ORDER BY "users"."id" DESC ``` 2. `GET /projects/:id/access_requests` With two N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) ORDER BY "members"."id" DESC [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"]] User Load (0.1ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" DESC LIMIT 1 [["id", 24]] User Load (0.1ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" DESC LIMIT 1 [["id", 23]] ActiveRecord::SchemaMigration Load (0.2ms) SELECT "schema_migrations".* FROM "schema_migrations" ProjectMember Load (0.1ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"], ["user_id", 24]] ProjectMember Load (0.2ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) AND "members"."user_id" = $4 ORDER BY "members"."id" DESC LIMIT 1 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"], ["user_id", 23]] ``` Without the N+1 queries issues: ``` ProjectMember Load (0.3ms) SELECT "members".* FROM "members" WHERE "members"."source_type" = $1 AND "members"."type" IN ('ProjectMember') AND "members"."source_id" = $2 AND "members"."source_type" = $3 AND "members"."type" IN ('ProjectMember') AND ("members"."requested_at" IS NOT NULL) ORDER BY "members"."id" DESC LIMIT 20 OFFSET 0 [["source_type", "Project"], ["source_id", 8], ["source_type", "Project"]] User Load (0.5ms) SELECT "users".* FROM "users" WHERE "users"."id" IN (24, 23) ORDER BY "users"."id" DESC ``` Signed-off-by: Rémy Coutable --- lib/api/access_requests.rb | 5 ++--- lib/api/entities.rb | 4 ++-- lib/api/members.rb | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb index 9c41d8aaa3e..d02b469dac8 100644 --- a/lib/api/access_requests.rb +++ b/lib/api/access_requests.rb @@ -18,10 +18,9 @@ module API source = find_source(source_type, params[:id]) authorize_admin_source!(source_type, source) - access_requesters = source.requesters - users = Kaminari.paginate_array(access_requesters.map(&:user)) + access_requesters = paginate(source.requesters.includes(:user)) - present paginate(users), with: Entities::AccessRequester, source: source + present access_requesters.map(&:user), with: Entities::AccessRequester, access_requesters: access_requesters end # Request access to the group/project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c5ff4557b4a..ae74d14a4bb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -93,14 +93,14 @@ module API class Member < UserBasic expose :access_level do |user, options| - member = options[:member] || options[:source].members.find_by(user_id: user.id) + member = options[:member] || options[:members].find { |m| m.user_id == user.id } member.access_level end end class AccessRequester < UserBasic expose :requested_at do |user, options| - access_requester = options[:access_requester] || options[:source].requesters.find_by(user_id: user.id) + access_requester = options[:access_requester] || options[:access_requesters].find { |m| m.user_id == user.id } access_requester.requested_at end end diff --git a/lib/api/members.rb b/lib/api/members.rb index ccb7618360d..2fae83f60b2 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -18,11 +18,11 @@ module API get ":id/members" do source = find_source(source_type, params[:id]) - members = source.members + members = source.members.includes(:user) members = members.joins(:user).merge(User.search(params[:query])) if params[:query] - users = Kaminari.paginate_array(members.map(&:user)) + members = paginate(members) - present paginate(users), with: Entities::Member, source: source + present members.map(&:user), with: Entities::Member, members: members end # Get a group/project member