Commit 3de4a150 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'add-pipeline-metrics-worker' into 'master'

Add Pipeline metrics worker

## What does this MR do?
Creating a new Merge Request metrics is time consuming, because we need to access repository.

This makes this operation asynchronous, thus non-blocking for updating pipeline.

## Are there points in the code the reviewer needs to double check?

## Why was this MR needed?

## Screenshots (if relevant)

## Does this MR meet the acceptance criteria?

- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [ ] Added for this feature/bug
  - [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

See merge request !6896
parents 8a5d3ce1 a4978030
...@@ -21,6 +21,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -21,6 +21,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Change user & group landing page routing from /u/:username to /:username - Change user & group landing page routing from /u/:username to /:username
- Prevent running GfmAutocomplete setup for each diff note !6569 - Prevent running GfmAutocomplete setup for each diff note !6569
- Added documentation for .gitattributes files - Added documentation for .gitattributes files
- Move Pipeline Metrics to separate worker
- AbstractReferenceFilter caches project_refs on RequestStore when active - AbstractReferenceFilter caches project_refs on RequestStore when active
- Replaced the check sign to arrow in the show build view. !6501 - Replaced the check sign to arrow in the show build view. !6501
- Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar) - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar)
......
...@@ -49,6 +49,10 @@ module Ci ...@@ -49,6 +49,10 @@ module Ci
transition any => :canceled transition any => :canceled
end end
# IMPORTANT
# Do not add any operations to this state_machine
# Create a separate worker for each new operation
before_transition [:created, :pending] => :running do |pipeline| before_transition [:created, :pending] => :running do |pipeline|
pipeline.started_at = Time.now pipeline.started_at = Time.now
end end
...@@ -62,13 +66,11 @@ module Ci ...@@ -62,13 +66,11 @@ module Ci
end end
after_transition [:created, :pending] => :running do |pipeline| after_transition [:created, :pending] => :running do |pipeline|
MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)). pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) }
update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil)
end end
after_transition any => [:success] do |pipeline| after_transition any => [:success] do |pipeline|
MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)). pipeline.run_after_commit { PipelineMetricsWorker.perform_async(id) }
update_all(latest_build_finished_at: pipeline.finished_at)
end end
after_transition [:created, :pending, :running] => :success do |pipeline| after_transition [:created, :pending, :running] => :success do |pipeline|
......
class PipelineMetricsWorker
include Sidekiq::Worker
sidekiq_options queue: :default
def perform(pipeline_id)
Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline|
update_metrics_for_active_pipeline(pipeline) if pipeline.active?
update_metrics_for_succeeded_pipeline(pipeline) if pipeline.success?
end
end
private
def update_metrics_for_active_pipeline(pipeline)
metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil)
end
def update_metrics_for_succeeded_pipeline(pipeline)
metrics(pipeline).update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: pipeline.finished_at)
end
def metrics(pipeline)
MergeRequest::Metrics.where(merge_request_id: merge_requests(pipeline))
end
def merge_requests(pipeline)
pipeline.merge_requests.map(&:id)
end
end
...@@ -187,33 +187,24 @@ describe Ci::Pipeline, models: true do ...@@ -187,33 +187,24 @@ describe Ci::Pipeline, models: true do
end end
end end
describe "merge request metrics" do describe 'merge request metrics' do
let(:project) { FactoryGirl.create :project } let(:project) { FactoryGirl.create :project }
let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) } let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) }
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) }
context 'when transitioning to running' do before do
it 'records the build start time' do expect(PipelineMetricsWorker).to receive(:perform_async).with(pipeline.id)
time = Time.now
Timecop.freeze(time) { build.run }
expect(merge_request.reload.metrics.latest_build_started_at).to be_within(1.second).of(time)
end end
it 'clears the build end time' do context 'when transitioning to running' do
build.run it 'schedules metrics workers' do
pipeline.run
expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil
end end
end end
context 'when transitioning to success' do context 'when transitioning to success' do
it 'records the build end time' do it 'schedules metrics workers' do
build.run pipeline.succeed
time = Time.now
Timecop.freeze(time) { build.success }
expect(merge_request.reload.metrics.latest_build_finished_at).to be_within(1.second).of(time)
end end
end end
end end
......
require 'spec_helper'
describe PipelineMetricsWorker do
let(:project) { create(:project) }
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) }
let(:pipeline) do
create(:ci_empty_pipeline,
status: status,
project: project,
ref: 'master',
sha: project.repository.commit('master').id,
started_at: 1.hour.ago,
finished_at: Time.now)
end
describe '#perform' do
subject { described_class.new.perform(pipeline.id) }
context 'when pipeline is running' do
let(:status) { 'running' }
it 'records the build start time' do
subject
expect(merge_request.reload.metrics.latest_build_started_at).to eq(pipeline.started_at)
end
it 'clears the build end time' do
subject
expect(merge_request.reload.metrics.latest_build_finished_at).to be_nil
end
end
context 'when pipeline succeeded' do
let(:status) { 'success' }
it 'records the build end time' do
subject
expect(merge_request.reload.metrics.latest_build_finished_at).to eq(pipeline.finished_at)
end
end
end
end
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