diff options
author | Stan Hu <stanhu@gmail.com> | 2019-03-02 20:31:36 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-03-06 18:03:46 +0300 |
commit | 062efe4f7a83fb2b6d951b314692cca9ee8731cd (patch) | |
tree | 2907359acdf497130b38c75555056226189af829 /lib/api | |
parent | a592a78072bb44fed1a25c25f2cabdc4cf4bc0bd (diff) |
Significantly reduce N+1 queries in /api/v4/todos endpoint
By preloading associations and batching issuable metadata lookups,
we can significantly cut the number of SQL queries needed to load
the Todos API endpoint.
On GitLab.com, my own tests showed my user's SQL queries went
from 365 to under 60 SQL queries.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/40378
Diffstat (limited to 'lib/api')
-rw-r--r-- | lib/api/entities.rb | 3 | ||||
-rw-r--r-- | lib/api/issues.rb | 2 | ||||
-rw-r--r-- | lib/api/merge_requests.rb | 2 | ||||
-rw-r--r-- | lib/api/todos.rb | 32 |
4 files changed, 35 insertions, 4 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 5176e9713c1..2cd0d93b205 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -883,7 +883,8 @@ module API expose :target_type expose :target do |todo, options| - todo_target_class(todo.target_type).represent(todo.target, options) + todo_options = options.fetch(todo.target_type, {}) + todo_target_class(todo.target_type).represent(todo.target, todo_options) end expose :target_url do |todo, options| diff --git a/lib/api/issues.rb b/lib/api/issues.rb index f43f4d961d6..7b8c070ad57 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -28,7 +28,7 @@ module API args[:scope] = args[:scope].underscore if args[:scope] issues = IssuesFinder.new(current_user, args).execute - .preload(:assignees, :labels, :notes, :timelogs, :project, :author, :closed_by) + .with_api_entity_associations issues.reorder(order_options_with_tie_breaker) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 44f1e81caf2..28072bdf586 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -45,7 +45,7 @@ module API return merge_requests if args[:view] == 'simple' merge_requests - .preload(:notes, :author, :assignee, :milestone, :latest_merge_request_diff, :labels, :timelogs, metrics: [:latest_closed_by, :merged_by]) + .with_api_entity_associations end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 64ac8ece56c..d2196f05173 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -6,6 +6,8 @@ module API before { authenticate! } + helpers ::Gitlab::IssuableMetadata + ISSUABLE_TYPES = { 'merge_requests' => ->(iid) { find_merge_request_with_access(iid) }, 'issues' => ->(iid) { find_project_issue(iid) } @@ -42,6 +44,30 @@ module API def find_todos TodosFinder.new(current_user, params).execute end + + def issuable_and_awardable?(type) + obj_type = Object.const_get(type) + + (obj_type < Issuable) && (obj_type < Awardable) + rescue NameError + false + end + + def batch_load_issuable_metadata(todos, options) + # This should be paginated and will cause Rails to SELECT for all the Todos + todos_by_type = todos.group_by(&:target_type) + + todos_by_type.keys.each do |type| + next unless issuable_and_awardable?(type) + + collection = todos_by_type[type] + + next unless collection + + targets = collection.map(&:target) + options[type] = { issuable_metadata: issuable_meta_data(targets, type) } + end + end end desc 'Get a todo list' do @@ -51,7 +77,11 @@ module API use :pagination end get do - present paginate(find_todos), with: Entities::Todo, current_user: current_user + todos = paginate(find_todos.with_api_entity_associations) + options = { with: Entities::Todo, current_user: current_user } + batch_load_issuable_metadata(todos, options) + + present todos, options end desc 'Mark a todo as done' do |