Fix Merge Request analytics query optimization
What does this MR do?
Note: the change is behind a feature flag, so there are no user facing changes.
Given the following query:
project(fullPath: "gitlab-org/gitlab") {
mergeRequests(mergedBefore: "2020-09-01", mergedAfter: "2020-08-01", first: 0) {
count
}
}
If the optimized_merge_request_count_with_merged_at_filter
feature flag is turned on, the backend will execute special DB queries for the count. The optimization works when specific fields are requested (count
, totalTimeToMerge
). The field check logic on the backend did not anticipate null
arguments, so the optimized code path was never reached when loading the MR analytics page.
Example:
project(fullPath: "gitlab-org/gitlab") {
mergeRequests(mergedBefore: "2020-09-01", mergedAfter: "2020-08-01", first: 0, labels: null) {
count
__typename
}
}
Fix:
-
.compact
the argument names to ignore empty (null) arguments - Ignore the
__typename
field. It seems like this is always requested by the FE.
How to test it:
- Make sure you're on ultimate
- Enable the FF:
Feature.enable(:optimized_merge_request_count_with_merged_at_filter)
- Go to any project: Analytics / Merge Request
- Inspect the graphql request with many
COUNT
queries. You should see that theCOUNT
queries are querying themerge_request_metrics
table.FROM "merge_request_metrics"
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
Resolves #276386 (closed)
Edited by Dan Jensen