Skip to content

Improve query performance of attention requests count

Patrick Bajao requested to merge 357385-improve-count-query into master

What does this MR do and why?

In !74800 (merged), 2 indexes were added to merge_request_assignees and merge_request_reviewers tables to improve the performance of the finder query for attention requests.

In !80732 (merged), the finder query was used to show the count of attention requests.

While investigating a bug related to the count, found out that the count query isn't performant enough. Based on this query plan (https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/9497/commands/33700), it takes around 447ms.

It's noticeable that the query plan isn't using the index added on the first MR when querying the merge_request_assignees and merge_request_reviewers tables.

To fix this, narrower indexes are added on both tables for user_id and state columns. This also deletes the state indexes that were added before. The query timing dropped to 14ms from 447ms.

Query plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/9497/commands/33717

Migrations

db:migrate 
== 20220401044858 AddUserIdAndStateIndexToMergeRequestAssignees: migrating ====
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_assignees, [:user_id, :state], {:where=>"state = 2", :name=>"index_on_merge_request_assignees_user_id_and_state", :algorithm=>:concurrently})
   -> 0.0044s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index(:merge_request_assignees, [:user_id, :state], {:where=>"state = 2", :name=>"index_on_merge_request_assignees_user_id_and_state", :algorithm=>:concurrently})
   -> 0.0033s
-- execute("RESET statement_timeout")
   -> 0.0004s
== 20220401044858 AddUserIdAndStateIndexToMergeRequestAssignees: migrated (0.0173s) 

== 20220401045116 AddUserIdAndStateIndexToMergeRequestReviewers: migrating ====
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_reviewers, [:user_id, :state], {:where=>"state = 2", :name=>"index_on_merge_request_reviewers_user_id_and_state", :algorithm=>:concurrently})
   -> 0.0023s
-- add_index(:merge_request_reviewers, [:user_id, :state], {:where=>"state = 2", :name=>"index_on_merge_request_reviewers_user_id_and_state", :algorithm=>:concurrently})
   -> 0.0023s
== 20220401045116 AddUserIdAndStateIndexToMergeRequestReviewers: migrated (0.0069s) 

== 20220401045621 RemoveStateIndexOnMergeRequestAssignees: migrating ==========
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_assignees)
   -> 0.0022s
-- remove_index(:merge_request_assignees, {:algorithm=>:concurrently, :name=>"index_on_merge_request_assignees_state"})
   -> 0.0036s
== 20220401045621 RemoveStateIndexOnMergeRequestAssignees: migrated (0.0078s) =

== 20220401045642 RemoveStateIndexOnMergeRequestReviewers: migrating ==========
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_reviewers)
   -> 0.0020s
-- remove_index(:merge_request_reviewers, {:algorithm=>:concurrently, :name=>"index_on_merge_request_reviewers_state"})
   -> 0.0022s
== 20220401045642 RemoveStateIndexOnMergeRequestReviewers: migrated (0.0060s) =
db:rollback 
== 20220401045642 RemoveStateIndexOnMergeRequestReviewers: reverting ==========
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_reviewers, :state, {:where=>"state = 2", :name=>"index_on_merge_request_reviewers_state", :algorithm=>:concurrently})
   -> 0.0033s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index(:merge_request_reviewers, :state, {:where=>"state = 2", :name=>"index_on_merge_request_reviewers_state", :algorithm=>:concurrently})
   -> 0.0047s
-- execute("RESET statement_timeout")
   -> 0.0005s
== 20220401045642 RemoveStateIndexOnMergeRequestReviewers: reverted (0.0143s) =

== 20220401045621 RemoveStateIndexOnMergeRequestAssignees: reverting ==========
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_assignees, :state, {:where=>"state = 2", :name=>"index_on_merge_request_assignees_state", :algorithm=>:concurrently})
   -> 0.0033s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- add_index(:merge_request_assignees, :state, {:where=>"state = 2", :name=>"index_on_merge_request_assignees_state", :algorithm=>:concurrently})
   -> 0.0039s
-- execute("RESET statement_timeout")
   -> 0.0004s
== 20220401045621 RemoveStateIndexOnMergeRequestAssignees: reverted (0.0128s) =

== 20220401045116 AddUserIdAndStateIndexToMergeRequestReviewers: reverting ====
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_reviewers)
   -> 0.0032s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- remove_index(:merge_request_reviewers, {:algorithm=>:concurrently, :name=>"index_on_merge_request_reviewers_user_id_and_state"})
   -> 0.0041s
-- execute("RESET statement_timeout")
   -> 0.0004s
== 20220401045116 AddUserIdAndStateIndexToMergeRequestReviewers: reverted (0.0129s)

== 20220401044858 AddUserIdAndStateIndexToMergeRequestAssignees: reverting ====
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_assignees)
   -> 0.0034s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- remove_index(:merge_request_assignees, {:algorithm=>:concurrently, :name=>"index_on_merge_request_assignees_user_id_and_state"})
   -> 0.0027s
-- execute("RESET statement_timeout")
   -> 0.0004s
== 20220401044858 AddUserIdAndStateIndexToMergeRequestAssignees: reverted (0.0113s)

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 Patrick Bajao

Merge request reports