Skip to content

Optimize searching cherry-picked merge requests for linking deployments

Shinya Maeda requested to merge optimize-link-merge-request-worker into master

What does this MR do?

As issued in #321032 (closed), often LinkMergeRequestService raises an error by hitting the query statement timeout. This is because the index that introduced by !48864 (merged) is used in the query, but it's inefficient in Deployments should track picked merge requests context. This MR fixes the problem by re-writing the query.

Problem analysis

The problematic query looks like this:

SELECT
    "merge_requests".*
FROM
    "merge_requests"
    INNER JOIN "notes" ON "notes"."noteable_type" = 'MergeRequest'
    AND "notes"."noteable_id" = "merge_requests"."id"
WHERE
    "merge_requests"."target_project_id" = 2009901
    AND "notes"."commit_id" IN (
        'a5a7a4328de3fdf433ecdf618cac2758c2e31125',
        '0ab28fcec50f224dfd31c2fca2b978444679b49c',
        'f36ae30ec621a48b99e59060329d7c7fe9860339',
        'f434d2dbf2f0e407efe057f9086fe05c6bde7739',
        '6f68e2cb582b272336963761181d1b0759fb5b22',
        '54f0de751b48b23424640cd851a8222b5025f6bc',
        '9c08dc0188e1a00866c9c1361bfd67862bec01d7',
        '60f150c4a290b0959ccd3fb39882855238352816',
        '1ee57b6c23b4b4caf4d61807e24d0b8fddd66d3d',
        '80211dc8e19f57018ddce84c3a99c96fff170c51',
        '1c7e8238c479c4dc06562178035c607f8afc6f30',
        'bb8419abe39f407843b26a36e7ae98fa1c3de56f',
        'ac2834036613a1fb85e18c94ea9571034d742b49',
        'bc5a194af0681e0c0496577fb65710029353fef6',
        '4c00bb661515f3612417baa4beb00f150754a025',
        '187bb420e6f4cde52d504777b27c4b424fd1dc4c',
        '07bb218223541d86a0831f132d56afac155ce9a7',
        '6b2d1545ad4ced4f8bde23d571776b8c273228c5',
        'ac20457dee05af05ee4b4cbf914ee0156e335aaa',
        '819329452cbf4f586768611b450d9376fb5b5f60',
        'c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746',
        'b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132'
    )

Here is the plan on production replica:

                                                    QUERY PLAN                                                                                                                                                     
                                                                                                                                                                                                                   
                                                                                                                   
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------
 Nested Loop  (cost=1.14..14578.12 rows=1 width=760)
   ->  Index Scan using index_on_merge_requests_for_latest_diffs on merge_requests  (cost=0.57..302.13 rows=287 width=760)
         Index Cond: (target_project_id = 2009901)
   ->  Index Scan using index_notes_on_noteable_id_and_noteable_type_and_system on notes  (cost=0.57..49.73 rows=1 width=4)
         Index Cond: ((noteable_id = merge_requests.id) AND ((noteable_type)::text = 'MergeRequest'::text))
         Filter: ((commit_id)::text = ANY ('{a5a7a4328de3fdf433ecdf618cac2758c2e31125,0ab28fcec50f224dfd31c2fca2b978444679b49c,f36ae30ec621a48b99e59060329d7c7fe9860339,f434d2dbf2f0e407efe057f9086fe05c6bde7739,6f
68e2cb582b272336963761181d1b0759fb5b22,54f0de751b48b23424640cd851a8222b5025f6bc,9c08dc0188e1a00866c9c1361bfd67862bec01d7,60f150c4a290b0959ccd3fb39882855238352816,1ee57b6c23b4b4caf4d61807e24d0b8fddd66d3d,80211dc8
e19f57018ddce84c3a99c96fff170c51,1c7e8238c479c4dc06562178035c607f8afc6f30,bb8419abe39f407843b26a36e7ae98fa1c3de56f,ac2834036613a1fb85e18c94ea9571034d742b49,bc5a194af0681e0c0496577fb65710029353fef6,4c00bb661515f3
612417baa4beb00f150754a025,187bb420e6f4cde52d504777b27c4b424fd1dc4c,07bb218223541d86a0831f132d56afac155ce9a7,6b2d1545ad4ced4f8bde23d571776b8c273228c5,ac20457dee05af05ee4b4cbf914ee0156e335aaa,819329452cbf4f586768
611b450d9376fb5b5f60,c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746,b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132}'::text[]))
(6 rows)

Notice that index_notes_on_noteable_id_and_noteable_type_and_system is used for index scan and (commit_id)::text = ANY ... is used for sequence scan. Given that notes table is a big table, this filtering would be very expensive. Maybe it takes hours or days to complete.

Here is the correct planning, that was able to be captured in db-lab:

 Hash Join  (cost=5293.50..24886.55 rows=1 width=760) (actual time=289.043..289.047 rows=0 loops=1)
   Hash Cond: (notes.noteable_id = merge_requests.id)
   Buffers: shared hit=28 read=63
   I/O Timings: read=285.603
   ->  Index Scan using index_notes_on_commit_id on public.notes  (cost=0.57..19571.27 rows=8513 width=4) (actual time=289.042..289.043 rows=0 loops=1)
         Index Cond: ((notes.commit_id)::text = ANY ('{a5a7a4328de3fdf433ecdf618cac2758c2e31125,0ab28fcec50f224dfd31c2fca2b978444679b49c,f36ae30ec621a48b99e59060329d7c7fe9860339,f434d2dbf2f0e407efe057f9086fe05c6bde7739,6f68e2cb582b272336963761181d1b0759fb5b22,54f0de751b48b23424640cd851a8222b5025f6bc,9c08dc0188e1a00866c9c1361bfd67862bec01d7,60f150c4a290b0959ccd3fb39882855238352816,1ee57b6c23b4b4caf4d61807e24d0b8fddd66d3d,80211dc8e19f57018ddce84c3a99c96fff170c51,1c7e8238c479c4dc06562178035c607f8afc6f30,bb8419abe39f407843b26a36e7ae98fa1c3de56f,ac2834036613a1fb85e18c94ea9571034d742b49,bc5a194af0681e0c0496577fb65710029353fef6,4c00bb661515f3612417baa4beb00f150754a025,187bb420e6f4cde52d504777b27c4b424fd1dc4c,07bb218223541d86a0831f132d56afac155ce9a7,6b2d1545ad4ced4f8bde23d571776b8c273228c5,ac20457dee05af05ee4b4cbf914ee0156e335aaa,819329452cbf4f586768611b450d9376fb5b5f60,c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746,b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132}'::text[]))
         Filter: ((notes.noteable_type)::text = 'MergeRequest'::text)
         Rows Removed by Filter: 0
         Buffers: shared hit=28 read=63
         I/O Timings: read=285.603
   ->  Hash  (cost=5230.44..5230.44 rows=4999 width=760) (actual time=0.000..0.000 rows=0 loops=0)
         ->  Index Scan using index_on_merge_requests_for_latest_diffs on public.merge_requests  (cost=0.57..5230.44 rows=4999 width=760) (actual time=0.000..0.000 rows=0 loops=0)
               Index Cond: (merge_requests.target_project_id = 2009901)

When it comes to filtering by SHAs, index_notes_on_commit_id can expect much faster timing, because SHAs filters out most of the rows efficiently.

Approach

In this MR, we resolve this problem by re-writing the query to fetch the related MRs through notes table without cross-joining.

explain analyze SELECT
    "merge_requests".*
FROM
    "merge_requests"
WHERE
    "merge_requests"."target_project_id" = 2009901
    AND "merge_requests"."id" IN (
        SELECT
            "notes"."noteable_id"
        FROM
            "notes"
        WHERE
            "notes"."project_id" = 2009901
            AND "notes"."noteable_type" = 'MergeRequest'
            AND "notes"."commit_id" IN (
                'a5a7a4328de3fdf433ecdf618cac2758c2e31125',
                '0ab28fcec50f224dfd31c2fca2b978444679b49c',
                'f36ae30ec621a48b99e59060329d7c7fe9860339',
                'f434d2dbf2f0e407efe057f9086fe05c6bde7739',
                '6f68e2cb582b272336963761181d1b0759fb5b22',
                '54f0de751b48b23424640cd851a8222b5025f6bc',
                '9c08dc0188e1a00866c9c1361bfd67862bec01d7',
                '60f150c4a290b0959ccd3fb39882855238352816',
                '1ee57b6c23b4b4caf4d61807e24d0b8fddd66d3d',
                '80211dc8e19f57018ddce84c3a99c96fff170c51',
                '1c7e8238c479c4dc06562178035c607f8afc6f30',
                'bb8419abe39f407843b26a36e7ae98fa1c3de56f',
                'ac2834036613a1fb85e18c94ea9571034d742b49',
                'bc5a194af0681e0c0496577fb65710029353fef6',
                '4c00bb661515f3612417baa4beb00f150754a025',
                '187bb420e6f4cde52d504777b27c4b424fd1dc4c',
                '07bb218223541d86a0831f132d56afac155ce9a7',
                '6b2d1545ad4ced4f8bde23d571776b8c273228c5',
                'ac20457dee05af05ee4b4cbf914ee0156e335aaa',
                '819329452cbf4f586768611b450d9376fb5b5f60',
                'c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746',
                'b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132'
            )
    );

Query plan on production replica:

                                                                                                     QUERY PLAN                                                                                                    
                                                                                                                                                                                                                   
                                                                                                                                                                                                                   
  
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--
 Nested Loop  (cost=1695.48..1702.14 rows=1 width=757) (actual time=0.280..0.280 rows=0 loops=1)
   ->  Unique  (cost=1694.91..1694.92 rows=2 width=4) (actual time=0.280..0.280 rows=0 loops=1)
         ->  Sort  (cost=1694.91..1694.91 rows=2 width=4) (actual time=0.279..0.280 rows=0 loops=1)
               Sort Key: notes.noteable_id
               Sort Method: quicksort  Memory: 25kB
               ->  Bitmap Heap Scan on notes  (cost=1691.81..1694.90 rows=2 width=4) (actual time=0.277..0.277 rows=0 loops=1)
                     Recheck Cond: (((commit_id)::text = ANY ('{a5a7a4328de3fdf433ecdf618cac2758c2e31125,0ab28fcec50f224dfd31c2fca2b978444679b49c,f36ae30ec621a48b99e59060329d7c7fe9860339,f434d2dbf2f0e407efe057f9
086fe05c6bde7739,6f68e2cb582b272336963761181d1b0759fb5b22,54f0de751b48b23424640cd851a8222b5025f6bc,9c08dc0188e1a00866c9c1361bfd67862bec01d7,60f150c4a290b0959ccd3fb39882855238352816,1ee57b6c23b4b4caf4d61807e24d0b
8fddd66d3d,80211dc8e19f57018ddce84c3a99c96fff170c51,1c7e8238c479c4dc06562178035c607f8afc6f30,bb8419abe39f407843b26a36e7ae98fa1c3de56f,ac2834036613a1fb85e18c94ea9571034d742b49,bc5a194af0681e0c0496577fb65710029353
fef6,4c00bb661515f3612417baa4beb00f150754a025,187bb420e6f4cde52d504777b27c4b424fd1dc4c,07bb218223541d86a0831f132d56afac155ce9a7,6b2d1545ad4ced4f8bde23d571776b8c273228c5,ac20457dee05af05ee4b4cbf914ee0156e335aaa,8
19329452cbf4f586768611b450d9376fb5b5f60,c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746,b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132}'::text[])) AND (project_id = 2009901) AND ((noteable_type)::text = 'MergeRequest'::text)
)
                     ->  BitmapAnd  (cost=1691.81..1691.81 rows=2 width=0) (actual time=0.276..0.276 rows=0 loops=1)
                           ->  Bitmap Index Scan on index_notes_on_commit_id  (cost=0.00..214.06 rows=13658 width=0) (actual time=0.275..0.275 rows=0 loops=1)
                                 Index Cond: ((commit_id)::text = ANY ('{a5a7a4328de3fdf433ecdf618cac2758c2e31125,0ab28fcec50f224dfd31c2fca2b978444679b49c,f36ae30ec621a48b99e59060329d7c7fe9860339,f434d2dbf2f0e40
7efe057f9086fe05c6bde7739,6f68e2cb582b272336963761181d1b0759fb5b22,54f0de751b48b23424640cd851a8222b5025f6bc,9c08dc0188e1a00866c9c1361bfd67862bec01d7,60f150c4a290b0959ccd3fb39882855238352816,1ee57b6c23b4b4caf4d61
807e24d0b8fddd66d3d,80211dc8e19f57018ddce84c3a99c96fff170c51,1c7e8238c479c4dc06562178035c607f8afc6f30,bb8419abe39f407843b26a36e7ae98fa1c3de56f,ac2834036613a1fb85e18c94ea9571034d742b49,bc5a194af0681e0c0496577fb65
710029353fef6,4c00bb661515f3612417baa4beb00f150754a025,187bb420e6f4cde52d504777b27c4b424fd1dc4c,07bb218223541d86a0831f132d56afac155ce9a7,6b2d1545ad4ced4f8bde23d571776b8c273228c5,ac20457dee05af05ee4b4cbf914ee0156
e335aaa,819329452cbf4f586768611b450d9376fb5b5f60,c5229cc2d1c24f49620ca65b4d2d2b6b9eb68746,b86a2ba3e29e0243f25f1b82f30e8e9ed7f85132}'::text[]))
                           ->  Bitmap Index Scan on index_notes_on_project_id_and_noteable_type  (cost=0.00..1477.50 rows=78093 width=0) (never executed)
                                 Index Cond: ((project_id = 2009901) AND ((noteable_type)::text = 'MergeRequest'::text))
   ->  Index Scan using merge_requests_pkey on merge_requests  (cost=0.57..3.59 rows=1 width=757) (never executed)
         Index Cond: (id = notes.noteable_id)
         Filter: (target_project_id = 2009901)
 Planning Time: 0.733 ms
 Execution Time: 0.389 ms
(17 rows)

Please see #321032 (comment 544642962) for more details. Also note that this problem might not occur depending on the query planner. For example, we can reproduce the statement timeout on production PostgreSQL, however, can't reproduce in db-lab.

Closes #321032 (closed)

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by Michael Kozono

Merge request reports