Commit ba25e2f1 authored by Timothy Andrew's avatar Timothy Andrew
Browse files

Improve performance of the cycle analytics page.

1. These changes bring down page load time for 100 issues from more than
   a minute to about 1.5 seconds.

2. This entire commit is composed of these types of performance
   enhancements:

     - Cache relevant data in `IssueMetrics` wherever possible.
     - Cache relevant data in `MergeRequestMetrics` wherever possible.
     - Preload metrics

3. Given these improvements, we now only need to make 4 SQL calls:

    - Load all issues
    - Load all merge requests
    - Load all metrics for the issues
    - Load all metrics for the merge requests

4. A list of all the data points that are now being pre-calculated:

    a. The first time an issue is mentioned in a commit

      - In `GitPushService`, find all issues mentioned by the given commit
        using `ReferenceExtractor`. Set the `first_mentioned_in_commit_at`
        flag for each of them.

      - There seems to be a (pre-existing) bug here - files (and
        therefore commits) created using the Web CI don't have
        cross-references created, and issues are not closed even when
        the commit title is "Fixes #xx".

    b. The first time a merge request is deployed to production

      When a `Deployment` is created, find all merge requests that
      were merged in before the deployment, and set the
      `first_deployed_to_production_at` flag for each of them.

    c. The start / end time for a merge request pipeline

      Hook into the `Pipeline` state machine. When the `status` moves to
      `running`, find the merge requests whose tip commit matches the
      pipeline, and record the `latest_build_started_at` time for each
      of them. When the `status` moves to `success`, record the
      `latest_build_finished_at` time.

    d. The merge requests that close an issue

      - This was a big cause of the performance problems we were having
        with Cycle Analytics. We need to use `ReferenceExtractor` to make
        this calculation, which is slow when we have to run it on a large
        number of merge requests.

      - When a merge request is created, updated, or refreshed, find the
        issues it closes, and create an instance of
        `MergeRequestsClosingIssues`, which acts as a join model between
        merge requests and issues.

      - If a `MergeRequestsClosingIssues` instance links a merge request
        and an issue, that issue closes that merge request.

5. The `Queries` module was changed into a class, so we can cache the
   results of `issues` and `merge_requests_closing_issues` across
   various cycle analytics stages.

6. The code added in this commit is untested. Tests will be added in the
   next commit.
parent 0fa32d5d0953
......@@ -55,6 +55,18 @@ class Pipeline < ActiveRecord::Base
pipeline.finished_at = Time.now
end
after_transition [:created, :pending] => :running do |pipeline|
pipeline.merge_requests.each do |merge_request|
merge_request.metrics.record_latest_build_start_time!(pipeline.started_at) if merge_request.metrics.present?
end
end
after_transition any => [:success] do |pipeline|
pipeline.merge_requests.each do |merge_request|
merge_request.metrics.record_latest_build_finish_time!(pipeline.finished_at) if merge_request.metrics.present?
end
end
before_transition do |pipeline|
pipeline.update_duration
end
......@@ -267,6 +279,13 @@ def execute_hooks
project.execute_services(data, :pipeline_hooks)
end
# Merge requests for which the current pipeline is running against
# the merge request's latest commit.
def merge_requests
project.merge_requests.where(source_branch: self.ref).
select { |merge_request| merge_request.pipeline.id == self.id }
end
private
def pipeline_data
......
......@@ -2,8 +2,8 @@ class CycleAnalytics
attr_reader :from
def initialize(project, from:)
@project = project
@from = from
@queries = Queries.new(project)
end
def as_json(options = {})
......@@ -14,45 +14,45 @@ def as_json(options = {})
end
def issue
calculate_metric(Queries::issues(@project, created_after: @from),
calculate_metric(@queries.issues(created_after: @from),
-> (data_point) { data_point[:issue].created_at },
[Queries::issue_first_associated_with_milestone_at, Queries::issue_first_added_to_list_label_at])
[@queries.issue_first_associated_with_milestone_at, @queries.issue_first_added_to_list_label_at])
end
def plan
calculate_metric(Queries::issues(@project, created_after: @from),
[Queries::issue_first_associated_with_milestone_at, Queries::issue_first_added_to_list_label_at],
Queries::issue_first_mentioned_in_commit_at)
calculate_metric(@queries.issues(created_after: @from),
[@queries.issue_first_associated_with_milestone_at, @queries.issue_first_added_to_list_label_at],
@queries.issue_first_mentioned_in_commit_at)
end
def code
calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from),
Queries::issue_first_mentioned_in_commit_at,
calculate_metric(@queries.merge_requests_closing_issues(created_after: @from),
@queries.issue_first_mentioned_in_commit_at,
-> (data_point) { data_point[:merge_request].created_at })
end
def test
calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from),
Queries::merge_request_build_started_at,
Queries::merge_request_build_finished_at)
calculate_metric(@queries.merge_requests_closing_issues(created_after: @from),
@queries.merge_request_build_started_at,
@queries.merge_request_build_finished_at)
end
def review
calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from),
calculate_metric(@queries.merge_requests_closing_issues(created_after: @from),
-> (data_point) { data_point[:merge_request].created_at },
Queries::merge_request_merged_at)
@queries.merge_request_merged_at)
end
def staging
calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from),
Queries::merge_request_merged_at,
Queries::merge_request_deployed_to_production_at)
calculate_metric(@queries.merge_requests_closing_issues(created_after: @from),
@queries.merge_request_merged_at,
@queries.merge_request_deployed_to_production_at)
end
def production
calculate_metric(Queries::merge_requests_closing_issues(@project, created_after: @from),
calculate_metric(@queries.merge_requests_closing_issues(created_after: @from),
-> (data_point) { data_point[:issue].created_at },
Queries::merge_request_deployed_to_production_at)
@queries.merge_request_deployed_to_production_at)
end
private
......
class CycleAnalytics
module Queries
class << self
def issues(project, created_after:)
project.issues.where("created_at >= ?", created_after).preload(:metrics, :system_notes).map { |issue| { issue: issue } }
end
def merge_requests_closing_issues(project, options = {})
issues(project, options).map do |data_point|
merge_requests = data_point[:issue].closed_by_merge_requests(nil, check_if_open: false)
merge_requests.map { |merge_request| { issue: data_point[:issue], merge_request: merge_request } }
end.flatten
end
class Queries
def initialize(project)
@project = project
end
def issue_first_associated_with_milestone_at
lambda do |data_point|
issue = data_point[:issue]
issue.metrics.first_associated_with_milestone_at if issue.metrics.present?
def issues(options = {})
@issues_data ||=
begin
issues_query(options).preload(:metrics).map { |issue| { issue: issue } }
end
end
end
def issue_first_added_to_list_label_at
lambda do |data_point|
issue = data_point[:issue]
issue.metrics.first_added_to_board_at if issue.metrics.present?
end
end
def merge_requests_closing_issues(options = {})
@merge_requests_closing_issues_data ||=
begin
merge_requests_closing_issues = MergeRequestsClosingIssues.where(issue: issues_query(options)).preload(issue: [:metrics], merge_request: [:metrics])
def issue_first_mentioned_in_commit_at
lambda do |data_point|
issue = data_point[:issue]
commits_mentioning_issue = issue.system_notes.map { |note| note.all_references.commits }.flatten
commits_mentioning_issue.map(&:committed_date).min if commits_mentioning_issue.present?
merge_requests_closing_issues.map do |record|
{ issue: record.issue, merge_request: record.merge_request }
end
end
end
end
def merge_request_first_closed_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.first_closed_at if merge_request.metrics.present?
end
def issue_first_associated_with_milestone_at
lambda do |data_point|
issue = data_point[:issue]
issue.metrics.first_associated_with_milestone_at if issue.metrics.present?
end
end
def merge_request_merged_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.merged_at if merge_request.metrics.present?
end
def issue_first_added_to_list_label_at
lambda do |data_point|
issue = data_point[:issue]
issue.metrics.first_added_to_board_at if issue.metrics.present?
end
end
def merge_request_build_started_at
lambda do |data_point|
merge_request = data_point[:merge_request]
tip = merge_request.commits.first
return unless tip
pipeline = Ci::Pipeline.success.find_by_sha(tip.sha)
pipeline.started_at if pipeline
end
def issue_first_mentioned_in_commit_at
lambda do |data_point|
issue = data_point[:issue]
issue.metrics.first_mentioned_in_commit_at if issue.metrics.present?
end
end
def merge_request_build_finished_at
lambda do |data_point|
merge_request = data_point[:merge_request]
tip = merge_request.commits.first
return unless tip
pipeline = Ci::Pipeline.success.find_by_sha(tip.sha)
pipeline.finished_at if pipeline
end
def merge_request_merged_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.merged_at if merge_request.metrics.present?
end
end
def merge_request_deployed_to_any_environment_at
lambda do |data_point|
merge_request = data_point[:merge_request]
if merge_request.metrics.present?
deployments = Deployment.where(ref: merge_request.target_branch).where("created_at > ?", merge_request.metrics.merged_at)
deployment = deployments.order(:created_at).first
deployment.created_at if deployment
end
end
def merge_request_build_started_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.latest_build_started_at if merge_request.metrics.present?
end
end
def merge_request_deployed_to_production_at
lambda do |data_point|
merge_request = data_point[:merge_request]
if merge_request.metrics.present?
# The first production deploy to the target branch that occurs after the merge request has been merged in.
# TODO: Does this need to account for reverts?
deployments = Deployment.joins(:environment).where(ref: merge_request.target_branch, "environments.name" => "production").
where("deployments.created_at > ?", merge_request.metrics.merged_at)
deployment = deployments.order(:created_at).first
deployment.created_at if deployment
end
end
def merge_request_build_finished_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.latest_build_finished_at if merge_request.metrics.present?
end
end
def issue_closing_merge_request_opened_at
lambda do |data_point|
issue = data_point[:issue]
merge_requests = issue.closed_by_merge_requests(nil, check_if_open: false)
merge_requests.map(&:created_at).min if merge_requests.present?
end
def merge_request_deployed_to_production_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.first_deployed_to_production_at if merge_request.metrics.present?
end
end
def merge_request_wip_flag_first_removed_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.wip_flag_first_removed_at if merge_request.metrics.present?
end
end
private
def merge_request_first_assigned_to_user_other_than_author_at
lambda do |data_point|
merge_request = data_point[:merge_request]
merge_request.metrics.first_assigned_to_user_other_than_author if merge_request.metrics.present?
end
end
def issues_query(created_after:)
@project.issues.where("created_at >= ?", created_after)
end
end
end
......@@ -25,6 +25,9 @@ class Issue < ActiveRecord::Base
has_one :metrics, dependent: :destroy
has_many :merge_requests_closing_issues, class_name: MergeRequestsClosingIssues
has_many :closed_by_merge_requests, through: :merge_requests_closing_issues, source: :merge_request
validates :project, presence: true
scope :cared, ->(user) { where(assignee_id: user) }
......
......@@ -13,6 +13,10 @@ def record!
self.save if self.changed?
end
def record_commit_mention!(commit_time)
self.update(first_mentioned_in_commit_at: commit_time) if self.first_mentioned_in_commit_at.blank?
end
private
def issue_assigned_to_list_label?
......
......@@ -17,6 +17,9 @@ class MergeRequest < ActiveRecord::Base
has_many :events, as: :target, dependent: :destroy
has_many :merge_requests_closing_issues, class_name: MergeRequestsClosingIssues
has_many :issues_closed, through: :merge_requests_closing_issues, source: :issue
serialize :merge_params, Hash
after_create :ensure_merge_request_diff, unless: :importing?
......@@ -494,6 +497,18 @@ def project
target_project
end
# If the merge request closes any issues, save this information in the
# `MergeRequestsClosingIssues` model. This is a performance optimization.
# Calculating this information for a number of merge requests requires
# running `ReferenceExtractor` on each of them separately.
def cache_merge_request_closes_issues!(current_user = self.author)
transaction do
closes_issues(current_user).each do |issue|
MergeRequestsClosingIssues.create!(merge_request: self, issue: issue)
end
end
end
def closes_issue?(issue)
closes_issues.include?(issue)
end
......
......@@ -20,4 +20,16 @@ def record!
self.save if self.changed?
end
def record_production_deploy!(deploy_time)
self.update(first_deployed_to_production_at: deploy_time) if self.first_deployed_to_production_at.blank?
end
def record_latest_build_start_time!(start_time)
self.update(latest_build_started_at: start_time, latest_build_finished_at: nil)
end
def record_latest_build_finish_time!(finish_time)
self.update(latest_build_finished_at: finish_time)
end
end
class MergeRequestsClosingIssues < ActiveRecord::Base
belongs_to :merge_request
belongs_to :issue
end
......@@ -6,7 +6,7 @@ def execute(deployable = nil)
name: params[:environment]
)
project.deployments.create(
deployment = project.deployments.create(
environment: environment,
ref: params[:ref],
tag: params[:tag],
......@@ -14,5 +14,26 @@ def execute(deployable = nil)
user: current_user,
deployable: deployable
)
update_merge_request_metrics(deployment, environment)
deployment
end
private
def update_merge_request_metrics(deployment, environment)
# TODO: Test cases:
# 1. Merge request with metrics available and `first_deployed_to_production_at` is nil
# 2. Merge request with metrics available and `first_deployed_to_production_at` is set
# 3. Merge request with metrics unavailable
# 4. Only applies to merge requests merged AFTER the previous production deploy to this branch
if environment.name == "production"
merge_requests_deployed_to_production_for_first_time = project.merge_requests.joins("LEFT OUTER JOIN merge_request_metrics ON merge_request_metrics.merge_request_id = merge_requests.id").
where(target_branch: params[:ref], "merge_request_metrics.first_deployed_to_production_at" => nil).
where("merge_request_metrics.merged_at < ?", deployment.created_at)
merge_requests_deployed_to_production_for_first_time.each { |merge_request| merge_request.metrics.record_production_deploy!(deployment.created_at) }
end
end
end
......@@ -134,6 +134,7 @@ def process_commit_messages
end
commit.create_cross_references!(authors[commit], closed_issues)
update_issue_metrics(commit, authors)
end
end
......@@ -186,4 +187,10 @@ def commit_user(commit)
def branch_name
@branch_name ||= Gitlab::Git.ref_name(params[:ref])
end
# TODO: What about commits created using the Web UI? Cross references are not created there.
def update_issue_metrics(commit, authors)
mentioned_issues = commit.all_references(authors[commit]).issues
mentioned_issues.each { |issue| issue.metrics.record_commit_mention!(commit.committed_date) if issue.metrics.present? }
end
end
......@@ -35,6 +35,18 @@ def execute_hooks(merge_request, action = 'open', oldrev = nil)
end
end
def create(merge_request)
merge_request = super(merge_request)
merge_request.cache_merge_request_closes_issues!(current_user)
merge_request
end
def update(merge_request)
merge_request = super(merge_request)
merge_request.cache_merge_request_closes_issues!(current_user)
merge_request
end
private
def filter_params
......
......@@ -13,6 +13,7 @@ def execute(oldrev, newrev, ref)
reload_merge_requests
reset_merge_when_build_succeeds
mark_pending_todos_done
cache_merge_requests_closing_issues
# Leave a system note if a branch was deleted/added
if branch_added? || branch_removed?
......@@ -141,6 +142,14 @@ def execute_mr_web_hooks
end
end
# If the merge requests closes any issues, save this information in the
# `MergeRequestsClosingIssues` model (as a performance optimization).
def cache_merge_requests_closing_issues
merge_requests_for_source_branch.each do |merge_request|
merge_request.cache_merge_request_closes_issues!(@current_user)
end
end
def filter_merge_requests(merge_requests)
merge_requests.uniq.select(&:source_project)
end
......
......@@ -24,30 +24,37 @@ def run_hook(name)
def seed!
Sidekiq::Testing.inline! do
issues = create_issues(@project)
print '.'
# Stage 1
Timecop.travel 5.days.from_now
add_milestones_and_list_labels(issues)
print '.'
# Stage 2
Timecop.travel 5.days.from_now
branches = mention_in_commits(issues)
print '.'
# Stage 3
Timecop.travel 5.days.from_now
merge_requests = create_merge_requests_closing_issues(issues, branches)
print '.'
# Stage 4
Timecop.travel 5.days.from_now
run_builds(merge_requests)
print '.'
# Stage 5
Timecop.travel 5.days.from_now
merge_merge_requests(merge_requests)
print '.'
# Stage 6 / 7
Timecop.travel 5.days.from_now
deploy_to_production(merge_requests)
print '.'
end
print '.'
......@@ -101,8 +108,14 @@ def mention_in_commits(issues)
}
commit_sha = Gitlab::Git::Blob.commit(issue.project.repository, options)
commit = issue.project.repository.commit(commit_sha)
commit.create_cross_references!
issue.project.repository.commit(commit_sha)
GitPushService.new(issue.project,
@user,
oldrev: issue.project.repository.commit("master").sha,
newrev: commit_sha,
ref: 'refs/heads/master').execute
branch_name
end
......@@ -162,10 +175,12 @@ def deploy_to_production(merge_requests)
Gitlab::Seeder.quiet do
if ENV['SEED_CYCLE_ANALYTICS']
seeder = Gitlab::Seeder::CycleAnalytics.new(Project.find(1))
Project.all.each do |project|
seeder = Gitlab::Seeder::CycleAnalytics.new(project)
seeder.seed!
end
elsif ENV['CYCLE_ANALYTICS_PERF_TEST']
seeder = Gitlab::Seeder::CycleAnalytics.new(Project.first, perf: true)
seeder = Gitlab::Seeder::CycleAnalytics.new(Project.order(:id).first, perf: true)
seeder.seed!
else
puts "Not running the cycle analytics seed file. Use the `SEED_CYCLE_ANALYTICS` environment variable to enable it."
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class CreateMergeRequestsClosingIssues < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = true
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
DOWNTIME_REASON = 'Adding foreign keys'
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
create_table :merge_requests_closing_issues do |t|
t.references :merge_request, foreign_key: true, null: false
t.references :issue, foreign_key: true, null: false
t.timestamps null: false
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddProductionDeployTimeToMergeRequestMetrics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
add_column :merge_request_metrics, :first_deployed_to_production_at, :datetime
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddFirstMentionedInCommitTimeToIssueMetrics < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers