summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2016-01-19 13:48:07 +0100
committerYorick Peterse <yorickpeterse@gmail.com>2016-01-19 14:03:20 +0100
commitb4ee6f57b9322401d1439eb21f9160ae2cb91d14 (patch)
treec767d7130fddc78aa8a320fa47b7a9545dd2e171
parent2becc6fae9b82479b644c0ca10b758cf8447bc19 (diff)
downloadgitlab-ce-b4ee6f57b9322401d1439eb21f9160ae2cb91d14.tar.gz
Greatly improve external_issue_tracker performanceimprove-project-external-issue-trackers
This greatly improves the performance of Project#external_issue_tracker by moving most of the fields queried in Ruby to the database and letting the database handle all logic. Prior to this change the process of finding an external issue tracker was along the lines of the following: 1. Load all project services into memory. 2. Reduce the list to only services where "issue_tracker?" returns true 3. Reduce the list from step 2 to service where "default?" returns false 4. Find the first service where "activated?" returns true This has to two big problems: 1. Loading all services into memory only to reduce the list down to a single item later on is a waste of memory (and slow timing wise). 2. Calling Array#select followed by Array#reject followed by Array#find allocates extra objects when this really isn't needed. To work around this the following service fields have been moved to the database (instead of being hardcoded): * category * default This in turn means we can get the external issue tracker using the following query: SELECT * FROM services WHERE active IS TRUE AND default IS FALSE AND category = 'issue_tracker' AND project_id = XXX LIMIT 1 This coupled with memoizing the result (just as before this commit) greatly reduces the time it takes for Project#external_issue_tracker to complete. The exact reduction depends on one's environment, but locally the execution time is reduced from roughly 230 ms to only 2 ms (= a reduction of almost 180x). Fixes gitlab-org/gitlab-ce#10771
-rw-r--r--app/models/project.rb7
-rw-r--r--app/models/project_services/ci_service.rb8
-rw-r--r--app/models/project_services/gitlab_issue_tracker_service.rb4
-rw-r--r--app/models/project_services/issue_tracker_service.rb6
-rw-r--r--app/models/service.rb11
-rw-r--r--db/migrate/20160119111158_add_services_category.rb39
-rw-r--r--db/migrate/20160119112418_add_services_default.rb20
-rw-r--r--db/schema.rb30
8 files changed, 92 insertions, 33 deletions
diff --git a/app/models/project.rb b/app/models/project.rb
index 7e131151513..cb668e0c2f7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -468,12 +468,9 @@ class Project < ActiveRecord::Base
!external_issue_tracker
end
- def external_issues_trackers
- services.select(&:issue_tracker?).reject(&:default?)
- end
-
def external_issue_tracker
- @external_issues_tracker ||= external_issues_trackers.find(&:activated?)
+ @external_issue_tracker ||=
+ services.issue_trackers.active.without_defaults.first
end
def can_have_issues_tracker_id?
diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb
index c3f70d1f972..e10b5529b42 100644
--- a/app/models/project_services/ci_service.rb
+++ b/app/models/project_services/ci_service.rb
@@ -23,14 +23,12 @@
# List methods you need to implement to get your CI service
# working with GitLab Merge Requests
class CiService < Service
- def category
- :ci
- end
-
+ default_value_for :category, 'ci'
+
def valid_token?(token)
self.respond_to?(:token) && self.token.present? && self.token == token
end
-
+
def supported_events
%w(push)
end
diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb
index 7aa04309f54..05436cd0f79 100644
--- a/app/models/project_services/gitlab_issue_tracker_service.rb
+++ b/app/models/project_services/gitlab_issue_tracker_service.rb
@@ -24,9 +24,7 @@ class GitlabIssueTrackerService < IssueTrackerService
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
- def default?
- true
- end
+ default_value_for :default, true
def to_param
'gitlab'
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index ed201979d39..25045224ce5 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -23,12 +23,10 @@ class IssueTrackerService < Service
validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated?
- def category
- :issue_tracker
- end
+ default_value_for :category, 'issue_tracker'
def default?
- false
+ default
end
def issue_url(iid)
diff --git a/app/models/service.rb b/app/models/service.rb
index 24f4bf7646e..721273250ea 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -43,6 +43,9 @@ class Service < ActiveRecord::Base
validates :project_id, presence: true, unless: Proc.new { |service| service.template? }
scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) }
+ scope :issue_trackers, -> { where(category: 'issue_tracker') }
+ scope :active, -> { where(active: true) }
+ scope :without_defaults, -> { where(default: false) }
scope :push_hooks, -> { where(push_events: true, active: true) }
scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
@@ -51,6 +54,8 @@ class Service < ActiveRecord::Base
scope :note_hooks, -> { where(note_events: true, active: true) }
scope :build_hooks, -> { where(build_events: true, active: true) }
+ default_value_for :category, 'common'
+
def activated?
active
end
@@ -60,7 +65,7 @@ class Service < ActiveRecord::Base
end
def category
- :common
+ read_attribute(:category).to_sym
end
def initialize_properties
@@ -153,7 +158,7 @@ class Service < ActiveRecord::Base
# Returns a hash of the properties that have been assigned a new value since last save,
# indicating their original values (attr => original value).
- # ActiveRecord does not provide a mechanism to track changes in serialized keys,
+ # ActiveRecord does not provide a mechanism to track changes in serialized keys,
# so we need a specific implementation for service properties.
# This allows to track changes to properties set with the accessor methods,
# but not direct manipulation of properties hash.
@@ -164,7 +169,7 @@ class Service < ActiveRecord::Base
def reset_updated_properties
@updated_properties = nil
end
-
+
def async_execute(data)
return unless supported_events.include?(data[:object_kind])
diff --git a/db/migrate/20160119111158_add_services_category.rb b/db/migrate/20160119111158_add_services_category.rb
new file mode 100644
index 00000000000..a9110a8418b
--- /dev/null
+++ b/db/migrate/20160119111158_add_services_category.rb
@@ -0,0 +1,39 @@
+class AddServicesCategory < ActiveRecord::Migration
+ def up
+ add_column :services, :category, :string, default: 'common', null: false
+
+ category = quote_column_name('category')
+ type = quote_column_name('type')
+
+ execute <<-EOF
+UPDATE services
+SET #{category} = 'issue_tracker'
+WHERE #{type} IN (
+ 'CustomIssueTrackerService',
+ 'GitlabIssueTrackerService',
+ 'IssueTrackerService',
+ 'JiraService',
+ 'RedmineService'
+);
+EOF
+
+ execute <<-EOF
+UPDATE services
+SET #{category} = 'ci'
+WHERE #{type} IN (
+ 'BambooService',
+ 'BuildkiteService',
+ 'CiService',
+ 'DroneCiService',
+ 'GitlabCiService',
+ 'TeamcityService'
+);
+ EOF
+
+ add_index :services, :category
+ end
+
+ def down
+ remove_column :services, :category
+ end
+end
diff --git a/db/migrate/20160119112418_add_services_default.rb b/db/migrate/20160119112418_add_services_default.rb
new file mode 100644
index 00000000000..69a42d7b873
--- /dev/null
+++ b/db/migrate/20160119112418_add_services_default.rb
@@ -0,0 +1,20 @@
+class AddServicesDefault < ActiveRecord::Migration
+ def up
+ add_column :services, :default, :boolean, default: false
+
+ default = quote_column_name('default')
+ type = quote_column_name('type')
+
+ execute <<-EOF
+UPDATE services
+SET #{default} = true
+WHERE #{type} = 'GitlabIssueTrackerService'
+EOF
+
+ add_index :services, :default
+ end
+
+ def down
+ remove_column :services, :default
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9045135dd9a..a08181b910f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20160113111034) do
+ActiveRecord::Schema.define(version: 20160119112418) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -725,20 +725,24 @@ ActiveRecord::Schema.define(version: 20160113111034) do
t.string "type"
t.string "title"
t.integer "project_id"
- t.datetime "created_at"
- t.datetime "updated_at"
- t.boolean "active", default: false, null: false
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.boolean "active", null: false
t.text "properties"
- t.boolean "template", default: false
- t.boolean "push_events", default: true
- t.boolean "issues_events", default: true
- t.boolean "merge_requests_events", default: true
- t.boolean "tag_push_events", default: true
- t.boolean "note_events", default: true, null: false
- t.boolean "build_events", default: false, null: false
- end
-
+ t.boolean "template", default: false
+ t.boolean "push_events", default: true
+ t.boolean "issues_events", default: true
+ t.boolean "merge_requests_events", default: true
+ t.boolean "tag_push_events", default: true
+ t.boolean "note_events", default: true, null: false
+ t.boolean "build_events", default: false, null: false
+ t.string "category", default: "common", null: false
+ t.boolean "default", default: false
+ end
+
+ add_index "services", ["category"], name: "index_services_on_category", using: :btree
add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree
+ add_index "services", ["default"], name: "index_services_on_default", using: :btree
add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
add_index "services", ["template"], name: "index_services_on_template", using: :btree