From 7a240397afc56ab366b6c3504761fbf531b78ec1 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Jan 2016 17:27:47 +0100 Subject: Added an index on milestones.title Certain pages (e.g. the group wide issues page) filter miletones by their title. Without an index this will result in a sequence scan on a large dataset increasing the total loading time of a page. --- db/migrate/20160106162223_add_index_milestones_title.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 db/migrate/20160106162223_add_index_milestones_title.rb diff --git a/db/migrate/20160106162223_add_index_milestones_title.rb b/db/migrate/20160106162223_add_index_milestones_title.rb new file mode 100644 index 00000000000..767885e2aac --- /dev/null +++ b/db/migrate/20160106162223_add_index_milestones_title.rb @@ -0,0 +1,5 @@ +class AddIndexMilestonesTitle < ActiveRecord::Migration + def change + add_index :milestones, :title + end +end -- cgit v1.2.1 From 9dacc3bc568c6c8cfc4a1bc1af23eb96f9eae9b0 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Jan 2016 17:29:44 +0100 Subject: Sort by ID when sorting using "Recently created" Sorting by "id" has the same effect as sorting by created_at while performing far better and without the need of an extra index (in case one wanted to speed up sorting by "created_at"). Sorting by "Recently updated" still uses the physical "updated_at" column as there's no way to use the "id" column for this instead. --- app/controllers/application_controller.rb | 2 +- app/helpers/sorting_helper.rb | 4 ++-- app/models/concerns/sortable.rb | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d9a37a4d45f..81cb1367e2c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -286,7 +286,7 @@ class ApplicationController < ActionController::Base end def set_filters_params - params[:sort] ||= 'created_desc' + params[:sort] ||= 'id_desc' params[:scope] = 'all' if params[:scope].blank? params[:state] = 'opened' if params[:state].blank? diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index bb12d43f397..9a7c26d1ccf 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -63,11 +63,11 @@ module SortingHelper end def sort_value_oldest_created - 'created_asc' + 'id_asc' end def sort_value_recently_created - 'created_desc' + 'id_desc' end def sort_value_milestone_soon diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index 7391a77383c..8b47b9e0abd 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -11,6 +11,7 @@ module Sortable default_scope { order_id_desc } scope :order_id_desc, -> { reorder(id: :desc) } + scope :order_id_asc, -> { reorder(id: :asc) } scope :order_created_desc, -> { reorder(created_at: :desc) } scope :order_created_asc, -> { reorder(created_at: :asc) } scope :order_updated_desc, -> { reorder(updated_at: :desc) } @@ -28,6 +29,8 @@ module Sortable when 'updated_desc' then order_updated_desc when 'created_asc' then order_created_asc when 'created_desc' then order_created_desc + when 'id_desc' then order_id_desc + when 'id_asc' then order_id_asc else all end -- cgit v1.2.1 From 0d0049c0584be2d358048c3cc545406b64bf3826 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Jan 2016 17:32:25 +0100 Subject: Don't pluck IDs when getting issues/MRs per group This replaces plucking of IDs with a sub-query, saving the overhead of loading the data in Ruby and then mapping the rows to an Array of IDs. This also scales much better when dealing with a large amount of IDs that would be involved. --- app/models/issue.rb | 2 +- app/models/merge_request.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 80ecd15077f..21cc2105161 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -33,7 +33,7 @@ class Issue < ActiveRecord::Base belongs_to :project validates :project, presence: true - scope :of_group, ->(group) { where(project_id: group.project_ids) } + scope :of_group, ->(group) { where(project_id: group.projects.select(:id)) } scope :cared, ->(user) { where(assignee_id: user) } scope :open_for, ->(user) { opened.assigned_to(user) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 30d0c2b5961..3c41bebc807 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -131,7 +131,7 @@ class MergeRequest < ActiveRecord::Base validate :validate_branches validate :validate_fork - scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } + scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.projects.select(:id)) } scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } -- cgit v1.2.1 From fc443ea7bcdd7255745cd1811c8bc95546ae3b12 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Jan 2016 17:33:43 +0100 Subject: Drop projects order in IssuableFinder When grabbing the projects to filter issues by we don't care about the order they're returned in. By removing the ORDER BY the resulting query can be quite a bit faster. --- app/finders/issuable_finder.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 3d5e8b6fbe7..4d56b48e3f8 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -79,9 +79,9 @@ class IssuableFinder if project? @projects = project elsif current_user && params[:authorized_only].presence && !current_user_related? - @projects = current_user.authorized_projects + @projects = current_user.authorized_projects.reorder(nil) else - @projects = ProjectsFinder.new.execute(current_user) + @projects = ProjectsFinder.new.execute(current_user).reorder(nil) end end -- cgit v1.2.1 From 5dc708e11c912acebf739851df4e9e2e11188d02 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 11:19:10 +0100 Subject: Updated Gemfile.lock due to Bundler re-ordering Bundler keeps re-ordering this particular Gem every time something is executed using "bundle exec". --- Gemfile.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a1168ed3b7a..3c7cb6cf439 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -443,6 +443,10 @@ GEM omniauth (1.2.2) hashie (>= 1.2, < 4) rack (~> 1.0) + omniauth-azure-oauth2 (0.0.6) + jwt (~> 1.0) + omniauth (~> 1.0) + omniauth-oauth2 (~> 1.1) omniauth-bitbucket (0.0.2) multi_json (~> 1.7) omniauth (~> 1.1) @@ -488,10 +492,6 @@ GEM activesupport nokogiri (>= 1.4.4) omniauth (~> 1.0) - omniauth-azure-oauth2 (0.0.6) - jwt (~> 1.0) - omniauth (~> 1.0) - omniauth-oauth2 (~> 1.1) opennebula (4.14.2) json nokogiri @@ -920,6 +920,7 @@ DEPENDENCIES oauth2 (~> 1.0.0) octokit (~> 3.7.0) omniauth (~> 1.2.2) + omniauth-azure-oauth2 omniauth-bitbucket (~> 0.0.2) omniauth-cas3 (~> 1.1.2) omniauth-facebook (~> 3.0.0) @@ -931,7 +932,6 @@ DEPENDENCIES omniauth-shibboleth (~> 1.2.0) omniauth-twitter (~> 1.2.0) omniauth_crowd - omniauth-azure-oauth2 org-ruby (~> 0.9.12) paranoia (~> 2.0) pg (~> 0.18.2) -- cgit v1.2.1 From 9b0c360bd5f8dd0538ecb5ffcb39da9a618b0231 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 12:38:21 +0100 Subject: Fixed issue sorting specs for ID changes These specs assumed data was still sorted by timestamp, instead of by ID. --- spec/features/issues_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index a2fb3e4c75d..e844e681ebf 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -127,15 +127,15 @@ describe 'Issues', feature: true do it 'sorts by newest' do visit namespace_project_issues_path(project.namespace, project, sort: sort_value_recently_created) - expect(first_issue).to include('foo') - expect(last_issue).to include('baz') + expect(first_issue).to include('baz') + expect(last_issue).to include('foo') end it 'sorts by oldest' do visit namespace_project_issues_path(project.namespace, project, sort: sort_value_oldest_created) - expect(first_issue).to include('baz') - expect(last_issue).to include('foo') + expect(first_issue).to include('foo') + expect(last_issue).to include('baz') end it 'sorts by most recently updated' do @@ -190,8 +190,8 @@ describe 'Issues', feature: true do sort: sort_value_oldest_created, assignee_id: user2.id) - expect(first_issue).to include('bar') - expect(last_issue).to include('foo') + expect(first_issue).to include('foo') + expect(last_issue).to include('bar') expect(page).not_to have_content 'baz' end end -- cgit v1.2.1 From 19a0db30ba14cb07a8f542abec56bd9cb2fa417f Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 15:34:37 +0100 Subject: Removed ORDER BY in "of_group" scopes These scopes don't care about the order. Removing the explicit "ORDER BY" can speed up the queries by a little bit. --- app/models/issue.rb | 4 +++- app/models/merge_request.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index 21cc2105161..f52e47f3e62 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -33,7 +33,9 @@ class Issue < ActiveRecord::Base belongs_to :project validates :project, presence: true - scope :of_group, ->(group) { where(project_id: group.projects.select(:id)) } + scope :of_group, + ->(group) { where(project_id: group.projects.select(:id).reorder(nil)) } + scope :cared, ->(user) { where(assignee_id: user) } scope :open_for, ->(user) { opened.assigned_to(user) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 3c41bebc807..4e685ad3b7c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -131,7 +131,7 @@ class MergeRequest < ActiveRecord::Base validate :validate_branches validate :validate_fork - scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.projects.select(:id)) } + scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.projects.select(:id).reorder(nil)) } scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) } scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } -- cgit v1.2.1