Commit da61b8b3 authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch 'improve-project-external-issue-trackers' into 'master'

Greatly improve external_issue_tracker performance

See 3d41133d48f6522b8755bb91b804864e8e520871 for all the details. As an aside, this commit contains a set of migrations that will introduce downtime as they add a column with a default value which in turn locks the entire table (at least on PostgreSQL).

See merge request !2498
parent 208008ee
...@@ -468,12 +468,9 @@ class Project < ActiveRecord::Base ...@@ -468,12 +468,9 @@ class Project < ActiveRecord::Base
!external_issue_tracker !external_issue_tracker
end end
def external_issues_trackers
services.select(&:issue_tracker?).reject(&:default?)
end
def external_issue_tracker def external_issue_tracker
@external_issues_tracker ||= external_issues_trackers.find(&:activated?) @external_issue_tracker ||=
services.issue_trackers.active.without_defaults.first
end end
def can_have_issues_tracker_id? def can_have_issues_tracker_id?
......
...@@ -23,9 +23,7 @@ ...@@ -23,9 +23,7 @@
# List methods you need to implement to get your CI service # List methods you need to implement to get your CI service
# working with GitLab Merge Requests # working with GitLab Merge Requests
class CiService < Service class CiService < Service
def category default_value_for :category, 'ci'
:ci
end
def valid_token?(token) def valid_token?(token)
self.respond_to?(:token) && self.token.present? && self.token == token self.respond_to?(:token) && self.token.present? && self.token == token
......
...@@ -24,9 +24,7 @@ class GitlabIssueTrackerService < IssueTrackerService ...@@ -24,9 +24,7 @@ class GitlabIssueTrackerService < IssueTrackerService
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
def default? default_value_for :default, true
true
end
def to_param def to_param
'gitlab' 'gitlab'
......
...@@ -23,12 +23,10 @@ class IssueTrackerService < Service ...@@ -23,12 +23,10 @@ class IssueTrackerService < Service
validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated? validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated?
def category default_value_for :category, 'issue_tracker'
:issue_tracker
end
def default? def default?
false default
end end
def issue_url(iid) def issue_url(iid)
......
...@@ -43,6 +43,9 @@ class Service < ActiveRecord::Base ...@@ -43,6 +43,9 @@ class Service < ActiveRecord::Base
validates :project_id, presence: true, unless: Proc.new { |service| service.template? } validates :project_id, presence: true, unless: Proc.new { |service| service.template? }
scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } 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 :push_hooks, -> { where(push_events: true, active: true) }
scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) } scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
...@@ -51,6 +54,8 @@ class Service < ActiveRecord::Base ...@@ -51,6 +54,8 @@ class Service < ActiveRecord::Base
scope :note_hooks, -> { where(note_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) }
scope :build_hooks, -> { where(build_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) }
default_value_for :category, 'common'
def activated? def activated?
active active
end end
...@@ -60,7 +65,7 @@ class Service < ActiveRecord::Base ...@@ -60,7 +65,7 @@ class Service < ActiveRecord::Base
end end
def category def category
:common read_attribute(:category).to_sym
end end
def initialize_properties def initialize_properties
......
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
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
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -725,9 +725,9 @@ ActiveRecord::Schema.define(version: 20160113111034) do ...@@ -725,9 +725,9 @@ ActiveRecord::Schema.define(version: 20160113111034) do
t.string "type" t.string "type"
t.string "title" t.string "title"
t.integer "project_id" t.integer "project_id"
t.datetime "created_at" t.datetime "created_at", null: false
t.datetime "updated_at" t.datetime "updated_at", null: false
t.boolean "active", default: false, null: false t.boolean "active", null: false
t.text "properties" t.text "properties"
t.boolean "template", default: false t.boolean "template", default: false
t.boolean "push_events", default: true t.boolean "push_events", default: true
...@@ -736,9 +736,13 @@ ActiveRecord::Schema.define(version: 20160113111034) do ...@@ -736,9 +736,13 @@ ActiveRecord::Schema.define(version: 20160113111034) do
t.boolean "tag_push_events", default: true t.boolean "tag_push_events", default: true
t.boolean "note_events", default: true, null: false t.boolean "note_events", default: true, null: false
t.boolean "build_events", default: false, null: false t.boolean "build_events", default: false, null: false
t.string "category", default: "common", null: false
t.boolean "default", default: false
end 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", ["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", ["project_id"], name: "index_services_on_project_id", using: :btree
add_index "services", ["template"], name: "index_services_on_template", using: :btree add_index "services", ["template"], name: "index_services_on_template", using: :btree
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment