summaryrefslogtreecommitdiff
path: root/lib/api
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-08-22 00:19:44 -0700
committerStan Hu <stanhu@gmail.com>2019-08-22 22:28:47 -0700
commite24b9c2502613cc0df5b2a676236d1c36c02bca4 (patch)
tree8384f8dbb4fa2ca6ebebed3cfd774e81371a859f /lib/api
parentba67965ac5386164a38acda32ff4e547e61b0376 (diff)
downloadgitlab-ce-e24b9c2502613cc0df5b2a676236d1c36c02bca4.tar.gz
Eliminate Gitaly N+1 queries with notes APIsh-eliminate-gitaly-nplus-one-notes
Similar to https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31834, we see that in https://gitlab.com/gitlab-org/gitlab-ce/issues/65957 there can be hundreds, even thousands, of Gitaly requests in the `/api/:version/projects/:id/merge_requests/:noteable_id/notes` endpoint. Previously, the API to retrieve notes generated hundreds of Gitaly calls to determine whether a system note should be shown to the user. It did this by: 1. Rendering the Markdown 2. Extracting cross-references from the Markdown 3. Issuing a Gitaly `FindCommit` RPC for every reference to validate that the commit exists. The last step is unnecessary because we don't need to display a commit if the user doesn't have access to the project in the first place. `RendersNotes#prepare_notes_for_rendering` is already used in `MergeRequestsController`, which is why we don't see N+1 Gitaly calls there. We use it here to optimize the note redaction process.
Diffstat (limited to 'lib/api')
-rw-r--r--lib/api/helpers/notes_helpers.rb2
-rw-r--r--lib/api/notes.rb13
2 files changed, 9 insertions, 6 deletions
diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb
index 6bf9057fad7..b2bf6bf7417 100644
--- a/lib/api/helpers/notes_helpers.rb
+++ b/lib/api/helpers/notes_helpers.rb
@@ -3,6 +3,8 @@
module API
module Helpers
module NotesHelpers
+ include ::RendersNotes
+
def self.noteable_types
# This is a method instead of a constant, allowing EE to more easily
# extend it.
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 9381f045144..84563d66ee8 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -36,12 +36,13 @@ module API
# page can have less elements than :per_page even if
# there's more than one page.
raw_notes = noteable.notes.with_metadata.reorder(order_options_with_tie_breaker)
- notes =
- # paginate() only works with a relation. This could lead to a
- # mismatch between the pagination headers info and the actual notes
- # array returned, but this is really a edge-case.
- paginate(raw_notes)
- .reject { |n| n.cross_reference_not_visible_for?(current_user) }
+
+ # paginate() only works with a relation. This could lead to a
+ # mismatch between the pagination headers info and the actual notes
+ # array returned, but this is really a edge-case.
+ notes = paginate(raw_notes)
+ notes = prepare_notes_for_rendering(notes)
+ notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
present notes, with: Entities::Note
end
# rubocop: enable CodeReuse/ActiveRecord