Skip to content

Speed up checking for approvers remaining

username-removed-443319 requested to merge improve-approvers-queries into master

Copying the commit message:

  1. 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.
  2. Use to_a explicitly when checking any? on a relation. Refactoring out the any? 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

Merge request reports