Skip to content

Use a UNION ALL for getting merge request notes

yorickpeterse-staging requested to merge merge-request-notes-performance into master

This MR changes MergeRequest#related_notes so that it uses a UNION ALL instead of an OR, resulting in a better query plan.

This fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/38508 since the new query consistently runs in about 2-3 ms.

TODO

  • Deploy this somehow to GitLab.com (temporarily) so we can get better insight on the impact across the board
  • Test this new query with a lot of commit IDs to see how this affects performance
  • Re-run EXPLAIN ANALYZE but with BUFFERS enabled

SQL Info

Per https://gitlab.com/gitlab-org/gitlab-ce/issues/38508#note_41779221 the new SQL query is:

SELECT notes.*
FROM (
    SELECT notes.*
    FROM notes
    WHERE notes.project_id = 13083
    AND notes.noteable_id = 3985770
    AND notes.noteable_type = 'MergeRequest'

    UNION ALL

    SELECT notes.*
    FROM notes
    WHERE notes.project_id IN (13083, 13083)
    AND notes.noteable_type = 'Commit'
    AND notes.commit_id IN ('aac1de46c9be659b74da12f704412f38292974db', '0395c47193b3bbf6b4f060f28c9f632580313a35')
) notes;

Which on GitLab.com produces the following plan:

 Append  (cost=0.56..2566.66 rows=6 width=2390) (actual time=0.021..0.990 rows=312 loops=1)
   ->  Index Scan using index_notes_on_noteable_id_and_noteable_type on notes  (cost=0.56..38.23 rows=1 width=2390) (actual time=0.020..0.791 rows=310 loops=1)
         Index Cond: ((noteable_id = 3985770) AND ((noteable_type)::text = 'MergeRequest'::text))
         Filter: (project_id = 13083)
   ->  Index Scan using index_notes_on_commit_id on notes notes_1  (cost=0.56..2528.38 rows=5 width=2390) (actual time=0.021..0.047 rows=2 loops=1)
         Index Cond: ((commit_id)::text = ANY ('{aac1de46c9be659b74da12f704412f38292974db,0395c47193b3bbf6b4f060f28c9f632580313a35}'::text[]))
         Filter: ((project_id = ANY ('{13083,13083}'::integer[])) AND ((noteable_type)::text = 'Commit'::text))
 Planning time: 0.477 ms
 Execution time: 1.120 ms

Compared to the old plan:

 Bitmap Heap Scan on notes  (cost=1570.26..1616.70 rows=3 width=2390) (actual time=2.368..6.096 rows=312 loops=1)
   Recheck Cond: (((noteable_id = 3985770) AND ((noteable_type)::text = 'MergeRequest'::text)) OR (((commit_id)::text = ANY ('{aac1de46c9be659b74da12f704412f38292974db,0395c47193b3bbf6b4f060f28c9f632580313a35}'::text[])) AND (project_id = 13083) AND ((noteable_type)::text = 'Commit'::text)))
   Filter: (project_id = 13083)
   Heap Blocks: exact=277
   ->  BitmapOr  (cost=1570.26..1570.26 rows=23 width=0) (actual time=2.310..2.310 rows=0 loops=1)
         ->  Bitmap Index Scan on index_notes_on_noteable_id_and_noteable_type  (cost=0.00..2.76 rows=20 width=0) (actual time=0.176..0.176 rows=310 loops=1)
               Index Cond: ((noteable_id = 3985770) AND ((noteable_type)::text = 'MergeRequest'::text))
         ->  BitmapAnd  (cost=1567.25..1567.25 rows=2 width=0) (actual time=2.132..2.132 rows=0 loops=1)
               ->  Bitmap Index Scan on index_notes_on_commit_id  (cost=0.00..30.55 rows=1257 width=0) (actual time=0.095..0.095 rows=2 loops=1)
                     Index Cond: ((commit_id)::text = ANY ('{aac1de46c9be659b74da12f704412f38292974db,0395c47193b3bbf6b4f060f28c9f632580313a35}'::text[]))
               ->  Bitmap Index Scan on index_notes_on_project_id_and_noteable_type  (cost=0.00..1536.45 rows=61189 width=0) (actual time=1.987..1.987 rows=6554 loops=1)
                     Index Cond: ((project_id = 13083) AND ((noteable_type)::text = 'Commit'::text))
 Planning time: 2.210 ms
 Execution time: 6.257 ms

Database Checklist

When adding or modifying queries to improve performance:

  • Included the raw SQL queries of the relevant queries
  • Included the output of EXPLAIN ANALYZE and execution timings of the relevant queries
  • Added tests for the relevant changes

General Checklist

Edited by yorickpeterse-staging

Merge request reports