Commit 39c090fe authored by Lin Jen-Shin's avatar Lin Jen-Shin

Calculate real queueing time to exclude gaps from:

retries and probably also manual actions!

Feedback:

* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14735478
* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14737804
parent d2cfcb3e
...@@ -260,13 +260,8 @@ module Ci ...@@ -260,13 +260,8 @@ module Ci
def update_duration def update_duration
return unless started_at return unless started_at
calculated_status = %w[success failed running canceled] self.duration, self.pending_duration =
calculated_builds = builds.latest.where(status: calculated_status) Gitlab::Ci::PipelineDuration.from_pipeline(self)
calculator = Gitlab::Ci::PipelineDuration.from_builds(calculated_builds)
self.duration = calculator.duration
self.pending_duration =
started_at - created_at + calculator.pending_duration
end end
def execute_hooks def execute_hooks
......
...@@ -106,17 +106,27 @@ module Gitlab ...@@ -106,17 +106,27 @@ module Gitlab
end end
end end
def self.from_builds(builds) def self.from_pipeline(pipeline)
status = %w[success failed running canceled]
builds = pipeline.builds.latest.where(status: status)
duration = from_builds(builds, :started_at, :finished_at).duration
queued = from_builds(builds, :queued_at, :started_at).duration
[duration, pipeline.started_at - pipeline.created_at + queued]
end
def self.from_builds(builds, from, to)
now = Time.now now = Time.now
periods = builds.map do |b| periods = builds.map do |b|
Period.new(b.started_at || now, b.finished_at || now) Period.new(b.public_send(from) || now, b.public_send(to) || now)
end end
new(periods) new(periods)
end end
attr_reader :duration, :pending_duration attr_reader :duration
def initialize(periods) def initialize(periods)
process(periods.sort_by(&:first)) process(periods.sort_by(&:first))
...@@ -125,10 +135,7 @@ module Gitlab ...@@ -125,10 +135,7 @@ module Gitlab
private private
def process(periods) def process(periods)
merged = process_periods(periods) @duration = process_duration(process_periods(periods))
@duration = process_duration(merged)
@pending_duration = process_pending_duration(merged)
end end
def process_periods(periods) def process_periods(periods)
...@@ -157,13 +164,6 @@ module Gitlab ...@@ -157,13 +164,6 @@ module Gitlab
result + per.duration result + per.duration
end end
end end
def process_pending_duration(periods)
return 0 if periods.empty?
total = periods.last.last - periods.first.first
total - duration
end
end end
end end
end end
...@@ -6,7 +6,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -6,7 +6,6 @@ describe Gitlab::Ci::PipelineDuration do
shared_examples 'calculating duration' do shared_examples 'calculating duration' do
it do it do
expect(calculator.duration).to eq(duration) expect(calculator.duration).to eq(duration)
expect(calculator.pending_duration).to eq(pending_duration)
end end
end end
...@@ -19,7 +18,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -19,7 +18,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 4 } let(:duration) { 4 }
let(:pending_duration) { 2 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -34,7 +32,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -34,7 +32,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 4 } let(:duration) { 4 }
let(:pending_duration) { 0 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -48,7 +45,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -48,7 +45,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 8 } let(:duration) { 8 }
let(:pending_duration) { 1 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -62,7 +58,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -62,7 +58,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 4 } let(:duration) { 4 }
let(:pending_duration) { 3 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -80,7 +75,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -80,7 +75,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 7 } let(:duration) { 7 }
let(:pending_duration) { 2 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -95,7 +89,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -95,7 +89,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 6 } let(:duration) { 6 }
let(:pending_duration) { 1 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
...@@ -108,7 +101,6 @@ describe Gitlab::Ci::PipelineDuration do ...@@ -108,7 +101,6 @@ describe Gitlab::Ci::PipelineDuration do
end end
let(:duration) { 4 } let(:duration) { 4 }
let(:pending_duration) { 2 }
it_behaves_like 'calculating duration' it_behaves_like 'calculating duration'
end end
......
...@@ -124,18 +124,23 @@ describe Ci::Pipeline, models: true do ...@@ -124,18 +124,23 @@ describe Ci::Pipeline, models: true do
describe 'state machine' do describe 'state machine' do
let(:current) { Time.now.change(usec: 0) } let(:current) { Time.now.change(usec: 0) }
let(:build) { create_build('build1', current - 60) } let(:build) { create_build('build1', current, 10) }
let(:another_build) { create_build('build2', current + 20) } let(:another_build) { create_build('build2', current + 80, 5) }
describe '#duration' do describe '#duration and #pending_duration' do
before do before do
pipeline.update(created_at: current)
travel_to(current + 5) do
pipeline.run pipeline.run
pipeline.save
end
travel_to(current) do travel_to(current + 70) do
build.success build.success
end end
travel_to(current + 80) do travel_to(current + 145) do
another_build.drop another_build.drop
end end
...@@ -143,7 +148,10 @@ describe Ci::Pipeline, models: true do ...@@ -143,7 +148,10 @@ describe Ci::Pipeline, models: true do
end end
it 'matches sum of builds duration' do it 'matches sum of builds duration' do
expect(pipeline.reload.duration).to eq(120) pipeline.reload
expect(pipeline.duration).to eq(70 - 10 + 145 - 85)
expect(pipeline.pending_duration).to eq(5 + 10 + 5)
end end
end end
...@@ -175,11 +183,12 @@ describe Ci::Pipeline, models: true do ...@@ -175,11 +183,12 @@ describe Ci::Pipeline, models: true do
end end
end end
def create_build(name, started_at = current) def create_build(name, queued_at = current, started_from = 0)
create(:ci_build, create(:ci_build,
name: name, name: name,
pipeline: pipeline, pipeline: pipeline,
started_at: started_at) queued_at: queued_at,
started_at: queued_at + started_from)
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