Optimize searching cherry-picked merge requests for linking deployments
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
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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