diff options
| author | Stan Hu <stanhu@gmail.com> | 2019-08-23 16:38:01 -0700 |
|---|---|---|
| committer | Stan Hu <stanhu@gmail.com> | 2019-08-23 21:26:50 -0700 |
| commit | 440635015fbea129cbfd7b266589ea2a33dda471 (patch) | |
| tree | 13c2379f7d9bb2647f6cd5ba06948f5ad2a3b6c7 | |
| parent | 20d38feda1b7085a2d1246a960ab575cd545da8f (diff) | |
| download | gitlab-ce-sh-fix-nplusone-issues.tar.gz | |
Fix N+1 Gitaly calls in /api/v4/projects/:id/issuessh-fix-nplusone-issues
This is a follow-up from
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31938.
In GitLab 9.0,
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9661 removed the
`subscribed` flag from the API when the user requested a list of issues
or merge requests since calculating this value triggers extensive
Markdown processing.
In GitLab 12.0 via a4fbf39e, we accidentally reintroduced this
performance regression by changing `IssueBasic` to `Issue` in
`entities.rb`. This showed up as a Gitaly N+1 issue since the Markdown
processing would attempt to extract a commit if it detected a regex that
matched a commit.
We restore the prior behavior by once again removing the `subscribed`
flag for the bulk list of issues and merge requests and add a test to
ensure they aren't reintroduced.
Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/66202
| -rw-r--r-- | changelogs/unreleased/sh-fix-nplusone-issues.yml | 5 | ||||
| -rw-r--r-- | doc/api/issues.md | 1 | ||||
| -rw-r--r-- | lib/api/issues.rb | 3 | ||||
| -rw-r--r-- | spec/requests/api/issues/get_project_issues_spec.rb | 8 |
4 files changed, 15 insertions, 2 deletions
diff --git a/changelogs/unreleased/sh-fix-nplusone-issues.yml b/changelogs/unreleased/sh-fix-nplusone-issues.yml new file mode 100644 index 00000000000..f749b5eeb40 --- /dev/null +++ b/changelogs/unreleased/sh-fix-nplusone-issues.yml @@ -0,0 +1,5 @@ +--- +title: Fix N+1 Gitaly calls in /api/v4/projects/:id/issues +merge_request: 32171 +author: +type: performance diff --git a/doc/api/issues.md b/doc/api/issues.md index 8313dd2c3bd..cadc9291489 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -284,7 +284,6 @@ Example response: "award_emoji":"http://example.com/api/v4/projects/4/issues/41/award_emoji", "project":"http://example.com/api/v4/projects/4" }, - "subscribed": false, "task_completion_status":{ "count":0, "completed_count":0 diff --git a/lib/api/issues.rb b/lib/api/issues.rb index e16eeef202c..215178478d0 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -163,7 +163,8 @@ module API with_labels_details: declared_params[:with_labels_details], current_user: current_user, project: user_project, - issuable_metadata: issuable_meta_data(issues, 'Issue', current_user) + issuable_metadata: issuable_meta_data(issues, 'Issue', current_user), + include_subscribed: false } present issues, options diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 521d6b88734..b7aa3f93451 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -446,6 +446,14 @@ describe API::Issues do expect_paginated_array_response([closed_issue.id, confidential_issue.id, issue.id]) end + it 'exposes known attributes' do + get api("#{base_url}/issues", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.last.keys).to include(*%w(id iid project_id title description)) + expect(json_response.last).not_to have_key('subscribed') + end + context 'issues_statistics' do context 'no state is treated as all state' do let(:params) { {} } |