Add index to todos the improve query performance
What does this MR do and why?
This MR adds an extra (async) index to support the query that updates the todo's status from pending to done. I found this query in the postgres checkup report: https://gitlab.com/gitlab-com/gl-infra/reliability/-/snippets/2401701
SELECT "todos"."id"
FROM "todos"
WHERE "todos"."user_id" = 3482158
AND ("todos"."state" IN ('pending'))
AND "todos"."project_id" = 278964
AND "todos"."target_id" = 174214523
AND "todos"."target_type" = 'MergeRequest'
AND "todos"."state" != 'done'
Query plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/11816/commands/42163
- Note 1: the
"todos"."state" != 'done'
part does not make much sense here. It's added by theTodo#batch_update
method. - Note 2: the query is mainly called from
UpdateMergeRequestsWorker
. The query is also called when a new comment is added (with mention).
Uses a BITMAP AND
with two sub-lookups:
- Reads all comments by
target_type
andtarget_id
: bad when MR/issue has many comment - Reads all pending comments by user: bad when user has many pending todos
The query is not particularly slow but it's called frequently on the primary (on each MR push, 11 calls/s). The only way I see to optimize this query is by adding an index which would reduce the uncached query time significantly. Caching/reducing the calls seems too complicated.
Improved query plan: https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/11816/commands/42244
Migration
Up:
main: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: migrating ==========
main: -- index_exists?(:todos, [:user_id, :project_id, :target_type, :target_id, :state, :id], {:name=>"index_on_todos_user_project_target_and_state", :algorithm=>:concurrently})
main: -> 0.0068s
main: -- add_index_options(:todos, [:user_id, :project_id, :target_type, :target_id, :state, :id], {:name=>"index_on_todos_user_project_target_and_state", :algorithm=>:concurrently})
main: -> 0.0002s
main: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: migrated (0.0231s) =
ci: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: migrating ==========
ci: -- index_exists?(:todos, [:user_id, :project_id, :target_type, :target_id, :state, :id], {:name=>"index_on_todos_user_project_target_and_state", :algorithm=>:concurrently})
ci: -> 0.0079s
ci: -- add_index_options(:todos, [:user_id, :project_id, :target_type, :target_id, :state, :id], {:name=>"index_on_todos_user_project_target_and_state", :algorithm=>:concurrently})
ci: -> 0.0002s
ci: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: migrated (0.0133s) =
Down
ci: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: reverting ==========
ci: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: reverted (0.0158s) =
main: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: reverting ==========
main: == 20220905112710 AddAsyncIndexToTodosToCoverPendingQuery: reverted (0.0167s) =
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.