Skip to content

Fix board issues graphql query performance regression

euko requested to merge fix-perf-regression-for-graphql-board-issues into master

What does this MR do and why?

Resolves #341793 (closed).

  1. Allow Types::BaseField's initializer to accept a new argument late_extensions. The argument will contain a list of extensions that will run after the default extensions have been run.

  2. Add a new custom connection extension Gitlab::Graphql::Board::IssuesConnectionExtension.

  • This extension will be used to call a method that checks and sets relative positions of the supplied issues for a board list.

  • The extension needs to be handed an ActiveRecord relation (issues) that's wrapped in the default pagination connections extension using late_extensions on the field issues of the type BoardList.

The below ideas have been scrapped! See !71164 (comment 696231028)

1. In BoardListIssuesResolver, pass the correct attribute of a pagination connections object (nodes) to the method handling/initializing relative positions.

2. Add a custom connections extension. A full discussion is in !71164 (comment 688541858).

More context

When visiting a board, a typical graphql query for board issues like this is requested:

{
  group(fullPath: "gitlab-org") {
    board(id: "gid://gitlab/Board/22") {
      list(id: "gid://gitlab/List/12") {
         issues(first: 10) {
           nodes {
             relativePosition
           }
         }
      }
    }
  }
}

If the requested issues have nil relative positions, we need to initialize them.

When we do this, the pagination connection class we use accidentally executes without the pagination argument first: 10 resulting in a db query like this:

SELECT issues.*, highest_priorities.label_priority as highest_priority
FROM issues JOIN LATERAL(SELECT MIN("label_priorities"."priority") AS label_priority FROM "labels" LEFT OUTER JOIN "label_priorities" ON "labels"."id" = "label_priorities"."label_id" INNER JOIN "label_links" ON "label_links"."label_id" = "labels"."id"
WHERE (label_priorities.project_id = issues.project_id) AND (label_links.target_id = issues.id) AND "label_links"."target_type" = 'Issue') as highest_priorities ON TRUE WHERE (NOT EXISTS (SELECT 1 FROM "banned_users" WHERE (issues.author_id = banned_users.user_id))) AND "issues"."project_id" = 278964 AND ("issues"."state_id" IN (1)) AND "issues"."issue_type" IN (0, 1) AND (NOT EXISTS (SELECT 1 FROM "issue_assignees" WHERE "issue_assignees"."user_id" IN (SELECT "lists"."user_id" FROM "lists" WHERE "lists"."board_id" = 1947445 AND "lists"."list_type" = 3 AND "lists"."user_id" IS NOT NULL) AND (issue_id = issues.id)))
GROUP BY "issues"."id", "highest_priorities"."label_priority"
ORDER BY issues.relative_position ASC NULLS LAST, highest_priorities.label_priority ASC NULLS LAST, "issues"."id" DESC

This MR fixes the regression by making the pagination connection to use the pagination argument so that it executes with LIMIT.

So the new query would have LIMIT 10:

SELECT issues.*, highest_priorities.label_priority as highest_priority
-- everything stays the same
LIMIT 10

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by euko

Merge request reports