Speed up checking for approvers remaining
Copying the commit message:
- Use the ProjectAuthorization model to find users with developer access to this project. This is dramatically faster than the old version, with the complicated ORs, on staging: ~10ms vs ~800ms.
- Use
to_a
explicitly when checkingany?
on a relation. Refactoring out theany?
checks would be painful, but as we often use the results anyway,to_a
lets us save a query.
Adding this up gets us the below (for a MR on the gitlab-ce project in staging).
Before:
Project Load (2.3ms) SELECT "projects".* FROM "projects" WHERE
"projects"."pending_delete" = 'f' AND "projects"."id" = 13083 ORDER BY
"projects"."id" DESC LIMIT 1 [["pending_delete", false], ["id", 13083]]
Approver Exists (1.2ms) SELECT 1 AS one FROM "approvers" WHERE
"approvers"."target_id" = 2294769 AND "approvers"."target_type" =
'MergeRequest' LIMIT 1 [["target_id", 2294769], ["target_type",
"MergeRequest"]]
ApproverGroup Exists (0.8ms) SELECT 1 AS one FROM "approver_groups" WHERE
"approver_groups"."target_id" = 2294769 AND "approver_groups"."target_type"
= 'MergeRequest' LIMIT 1 [["target_id", 2294769], ["target_type",
"MergeRequest"]]
User Load (2.7ms) SELECT "users".* FROM "users" WHERE "users"."id" = 840794
ORDER BY "users"."id" DESC LIMIT 1 [["id", 840794]]
Approver Load (0.9ms) SELECT "approvers".* FROM "approvers" WHERE
"approvers"."target_id" = 13083 AND "approvers"."target_type" = 'Project'
AND ("approvers"."user_id" != 840794) [["target_id", 13083], ["target_type",
"Project"], ["user_id", 840794]]
Approver Exists (0.6ms) SELECT 1 AS one FROM "approvers" WHERE
"approvers"."target_id" = 2294769 AND "approvers"."target_type" =
'MergeRequest' LIMIT 1 [["target_id", 2294769], ["target_type",
"MergeRequest"]]
ApproverGroup Exists (0.6ms) SELECT 1 AS one FROM "approver_groups" WHERE
"approver_groups"."target_id" = 2294769 AND "approver_groups"."target_type"
= 'MergeRequest' LIMIT 1 [["target_id", 2294769], ["target_type",
"MergeRequest"]]
ApproverGroup Load (0.7ms) SELECT "approver_groups".* FROM "approver_groups"
WHERE "approver_groups"."target_id" = 13083 AND
"approver_groups"."target_type" = 'Project' [["target_id", 13083],
["target_type", "Project"]]
Group Load (1.3ms) SELECT "namespaces".* FROM "namespaces" WHERE
"namespaces"."deleted_at" IS NULL AND "namespaces"."type" IN ('Group') AND
"namespaces"."id" = 9970 AND "namespaces"."type" IN ('Group') AND
"namespaces"."type" = 'Group' ORDER BY "namespaces"."id" DESC LIMIT 1
[["id", 9970], ["type", "Group"]]
(1222.4ms) SELECT COUNT(*) FROM "users" WHERE ("users"."state"
IN ('active')) AND ("users"."ghost" = 'f' OR "users"."ghost" IS NULL)
AND ("users"."support_bot" = 'f' OR "users"."support_bot" IS NULL) AND ((id
IN (SELECT "members"."user_id" FROM "members" WHERE "members"."source_type"
= 'Project' AND "members"."type" IN ('ProjectMember') AND
"members"."source_id" = 13083 AND "members"."source_type" = 'Project' AND
"members"."type" IN ('ProjectMember') AND "members"."requested_at" IS NULL
AND (access_level > 20) ORDER BY "members"."id" DESC) OR id IN (SELECT
"members"."user_id" FROM "members" WHERE "members"."source_type" =
'Namespace' AND "members"."type" IN ('GroupMember') AND
"members"."source_id" = 9970 AND "members"."source_type" = 'Namespace' AND
"members"."type" IN ('GroupMember') AND "members"."requested_at" IS NULL
AND (access_level > 20) ORDER BY "members"."id" DESC)) AND id NOT IN (SELECT
"approvals"."user_id" FROM "approvals" WHERE "approvals"."merge_request_id"
= 2294769)) AND ("users"."id" != 840794) [["id", 840794]]
After:
Project Load (2.5ms) SELECT "projects".* FROM "projects" WHERE
"projects"."pending_delete" = 'f' AND "projects"."id" = 13083 ORDER BY
"projects"."id" DESC LIMIT 1 [["pending_delete", false], ["id", 13083]]
Approver Load (1.3ms) SELECT "approvers".* FROM "approvers" WHERE
"approvers"."target_id" = 2294769 AND "approvers"."target_type" =
'MergeRequest' [["target_id", 2294769], ["target_type", "MergeRequest"]]
ApproverGroup Load (1.5ms) SELECT "approver_groups".* FROM "approver_groups"
WHERE "approver_groups"."target_id" = 2294769 AND
"approver_groups"."target_type" = 'MergeRequest' [["target_id", 2294769],
["target_type", "MergeRequest"]]
User Load (2.9ms) SELECT "users".* FROM "users" WHERE "users"."id" = 840794
ORDER BY "users"."id" DESC LIMIT 1 [["id", 840794]]
Approver Load (0.9ms) SELECT "approvers".* FROM "approvers" WHERE
"approvers"."target_id" = 13083 AND "approvers"."target_type" = 'Project'
AND ("approvers"."user_id" != 840794) [["target_id", 13083], ["target_type",
"Project"], ["user_id", 840794]]
ApproverGroup Load (0.7ms) SELECT "approver_groups".* FROM "approver_groups"
WHERE "approver_groups"."target_id" = 13083 AND
"approver_groups"."target_type" = 'Project' [["target_id", 13083],
["target_type", "Project"]]
(6.2ms) SELECT COUNT(*) FROM "users" WHERE ("users"."state" IN ('active'))
AND ("users"."ghost" = 'f' OR "users"."ghost" IS NULL)
AND ("users"."support_bot" = 'f' OR "users"."support_bot" IS NULL) AND ((id
IN (SELECT "project_authorizations"."user_id" FROM "project_authorizations"
WHERE (project_id = 13083 AND access_level > 20))) AND id NOT IN (SELECT
"approvals"."user_id" FROM "approvals" WHERE "approvals"."merge_request_id"
= 2294769)) AND ("users"."id" != 840794) [["id", 840794]]
Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/1843 and https://gitlab.com/gitlab-org/gitlab-ee/issues/2448.
Edited by username-removed-443319