Add NULLS LAST to index on merge request metrics (!47300) · Merge requests · GitLab.org / GitLab

What does this MR do?

This MR adds NULLS LAST to index on merge_request_metrics table in order to further improve the sort performance on the merge request analytics page: https://gitlab.com/gitlab-org/gitlab/-/analytics/merge_request_analytics

Database review:

Note: this index will not improve the performance of the current query. We have plans to rewrite the query to benefit from the index in #276387 (closed)

We decided to separate the index creation and the query modification because the modification requires more work: we might need to make significant modifications in the finders.

Current query:

SELECT "merge_requests".*,
       "merge_request_metrics"."merged_at" AS "merge_request_metrics.merged_at"
FROM "merge_requests"
INNER JOIN "merge_request_metrics" ON "merge_request_metrics"."merge_request_id" = "merge_requests"."id"
WHERE "merge_requests"."target_project_id" = 278964
  AND "merge_requests"."target_project_id" = "merge_request_metrics"."target_project_id"
  AND "merge_request_metrics"."merged_at" >= '2019-11-12 00:00:00'
  AND "merge_request_metrics"."merged_at" <= '2020-11-11 00:00:00'
  AND "merge_requests"."target_project_id" = "merge_request_metrics"."target_project_id"
ORDER BY merge_request_metrics.merged_at DESC NULLS LAST,
                                              "merge_requests"."id" DESC
LIMIT 20

Plan before index change: https://explain.depesz.com/s/Of5b
Plan after index change: https://explain.depesz.com/s/6t7F

Planned query change (not part of this MR):

SELECT "merge_requests".*,
       "merge_request_metrics"."merged_at" AS "merge_request_metrics.merged_at"
FROM "merge_requests"
INNER JOIN "merge_request_metrics" ON "merge_request_metrics"."merge_request_id" = "merge_requests"."id" AND "merge_request_metrics"."target_project_id" = "merge_requests"."target_project_id"
WHERE "merge_requests"."target_project_id" = 278964
  AND "merge_request_metrics"."merged_at" >= '2019-11-12 00:00:00'
  AND "merge_request_metrics"."merged_at" <= '2020-11-11 00:00:00'
ORDER BY merge_request_metrics.merged_at DESC NULLS LAST, merge_request_metrics.id DESC
LIMIT 20

Plan: https://explain.depesz.com/s/Dhxe

Index creation took 2 minutes.

Up:

== 20201110133629 ChangeIndexMrMetricsTargetProjectId: migrating ==============
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_metrics, [:target_project_id, :merged_at, :id], {:order=>{:merged_at=>"DESC NULLS LAST", :id=>"DESC"}, :name=>"index_mr_metrics_on_target_project_id_merged_at_nulls_last", :algorithm=>:concurrently})
   -> 0.0039s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- add_index(:merge_request_metrics, [:target_project_id, :merged_at, :id], {:order=>{:merged_at=>"DESC NULLS LAST", :id=>"DESC"}, :name=>"index_mr_metrics_on_target_project_id_merged_at_nulls_last", :algorithm=>:concurrently})
   -> 0.0089s
-- execute("RESET ALL")
   -> 0.0002s
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_metrics)
   -> 0.0040s
-- remove_index(:merge_request_metrics, {:algorithm=>:concurrently, :name=>"index_merge_request_metrics_on_target_project_id_merged_at"})
   -> 0.0011s
== 20201110133629 ChangeIndexMrMetricsTargetProjectId: migrated (0.0191s) =====

Down:

== 20201110133629 ChangeIndexMrMetricsTargetProjectId: reverting ==============
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:merge_request_metrics, [:target_project_id, :merged_at], {:name=>"index_merge_request_metrics_on_target_project_id_merged_at", :algorithm=>:concurrently})
   -> 0.0033s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- add_index(:merge_request_metrics, [:target_project_id, :merged_at], {:name=>"index_merge_request_metrics_on_target_project_id_merged_at", :algorithm=>:concurrently})
   -> 0.0114s
-- execute("RESET ALL")
   -> 0.0017s
-- transaction_open?()
   -> 0.0000s
-- indexes(:merge_request_metrics)
   -> 0.0170s
-- remove_index(:merge_request_metrics, {:algorithm=>:concurrently, :name=>"index_mr_metrics_on_target_project_id_merged_at_nulls_last"})
   -> 0.0055s
== 20201110133629 ChangeIndexMrMetricsTargetProjectId: reverted (0.0424s) =====

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

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

Add NULLS LAST to index on merge request metrics