Improve the performance of MR analytics page (charts)
What does this MR do?
This change adds a feature flag (optimized_merge_request_count_with_merged_at_filter
) which alters the COUNT query to only
access the merge_request_metrics table.
How
We inspect the GraphQL query and detect if only the count
field is requested without any extra filters. Requesting only the count means that we don't need to deal with additional filtering (this is basically what happens when the page is visited for the first time).
Since merge_request_metrics
knows about the target_project_id
and we have a foreign key constraint on the table pointing to merge_requests
(has_one
), we can safely query the merge_request_metrics
table only. This saves I/O and a large JOIN
on the DB level.
Example GraphQL queries:
project(fullPath: "gitlab-org/gitlab") {
mergeRequests(mergedBefore: "2020-09-01", mergedAfter: "2020-08-01", first: 0) {
count
}
}
project(fullPath: "gitlab-org/gitlab") {
mergeRequests(mergedBefore: "2020-09-01", mergedAfter: "2020-08-01", first: 0) {
count
totalTimeToMerge
}
}
Database
Old query
SELECT COUNT(*)
FROM "merge_request_metrics"
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" >= '2020-09-01 00:00:00'
AND "merge_request_metrics"."merged_at" <= '2020-10-01 00:00:00'
New query
SELECT COUNT(*)
FROM merge_request_metrics
WHERE target_project_id = 278964
AND merged_at >= '2020-09-01 00:00:00'
AND merged_at <= '2020-10-01 00:00:00'
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
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
Related to #276386 (closed)