Merge branch 'smart-pipeline-duration' into 'master'

Smartly calculate real running time and pending time

## What does this MR do?

Try to smartly calculate the running time and pending time for pipelines, instead of just use wall clock time from start to end. The algorithm is based on:

> Suppose we have A, B, and C jobs:

> * A: from 1 to 3
> * B: from 2 to 4
> * C: from 6 to 7

> The processing time should be accumulated from 1 to 4, and 6 to 7, totally 4, excluding retires, and calculate on `%w[success failed running canceled]` jobs (if a job is not finished yet, assume it's `Time.now`)

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

I would actually like to test `Gitlab::Ci::PipelineDuration#process_segments`, but it's a private method right now and it's not very convenient to test it. Is there a way to test it without changing the original code too much? Note that I would like to avoid saving merged segments because it's not used and should be garbage collected.

## Screenshots:

![Screen_Shot_2016-09-05_at_6.45.32_PM](/uploads/a82bfaf316661091e383b743a2f11334/Screen_Shot_2016-09-05_at_6.45.32_PM.png)

## Does this MR meet the acceptance criteria?

- [x] [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)
- Tests
  - [x] Added for this feature/bug

## What are the relevant issue numbers?

Closes #18260, #19804

See merge request !6084

6 files changed, 300 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cc87ff3ecb7..eaa1e007ca9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -46,6 +46,8 @@ v 8.12.0 (unreleased)
   - Use 'git update-ref' for safer web commits !6130
   - Sort pipelines requested through the API
   - Automatically expand hidden discussions when accessed by a permalink !5585 (Mike Greiling)
+  - Change pipeline duration to be jobs running time instead of simple wall time from start to end !6084
+  - Show queued time when showing a pipeline !6084
   - Remove unused mixins (ClemMakesApps)
   - Add search to all issue board lists
   - Fix groups sort dropdown alignment (ClemMakesApps)
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index bd1737c7587..0b1df9f4294 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -257,8 +257,17 @@ module Ci
       ]
     end
 
+    def queued_duration
+      return unless started_at
+
+      seconds = (started_at - created_at).to_i
+      seconds unless seconds.zero?
+    end
+
     def update_duration
-      self.duration = calculate_duration
+      return unless started_at
+
+      self.duration = Gitlab::Ci::PipelineDuration.from_pipeline(self)
     end
 
     def execute_hooks
diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 063e83a407a..5800ef7de48 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -10,6 +10,8 @@
   - if @pipeline.duration
     in
     = time_interval_in_words(@pipeline.duration)
+  - if @pipeline.queued_duration
+    = "(queued for #{time_interval_in_words(@pipeline.queued_duration)})"
 
   .pull-right
     = link_to namespace_project_pipeline_path(@project.namespace, @project, @pipeline), class: "ci-status ci-#{@pipeline.status}" do
diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
new file mode 100644
index 00000000000..a210e76acaa
--- /dev/null
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -0,0 +1,141 @@
+module Gitlab
+  module Ci
+    # # Introduction - total running time
+    #
+    # The problem this module is trying to solve is finding the total running
+    # time amongst all the jobs, excluding retries and pending (queue) time.
+    # We could reduce this problem down to finding the union of periods.
+    #
+    # So each job would be represented as a `Period`, which consists of
+    # `Period#first` as when the job started and `Period#last` as when the
+    # job was finished. A simple example here would be:
+    #
+    # * A (1, 3)
+    # * B (2, 4)
+    # * C (6, 7)
+    #
+    # Here A begins from 1, and ends to 3. B begins from 2, and ends to 4.
+    # C begins from 6, and ends to 7. Visually it could be viewed as:
+    #
+    #     0  1  2  3  4  5  6  7
+    #        AAAAAAA
+    #           BBBBBBB
+    #                       CCCC
+    #
+    # The union of A, B, and C would be (1, 4) and (6, 7), therefore the
+    # total running time should be:
+    #
+    #     (4 - 1) + (7 - 6) => 4
+    #
+    # # The Algorithm
+    #
+    # The algorithm used here for union would be described as follow.
+    # First we make sure that all periods are sorted by `Period#first`.
+    # Then we try to merge periods by iterating through the first period
+    # to the last period. The goal would be merging all overlapped periods
+    # so that in the end all the periods are discrete. When all periods
+    # are discrete, we're free to just sum all the periods to get real
+    # running time.
+    #
+    # Here we begin from A, and compare it to B. We could find that
+    # before A ends, B already started. That is `B.first <= A.last`
+    # that is `2 <= 3` which means A and B are overlapping!
+    #
+    # When we found that two periods are overlapping, we would need to merge
+    # them into a new period and disregard the old periods. To make a new
+    # period, we take `A.first` as the new first because remember? we sorted
+    # them, so `A.first` must be smaller or equal to `B.first`. And we take
+    # `[A.last, B.last].max` as the new last because we want whoever ended
+    # later. This could be broken into two cases:
+    #
+    #     0  1  2  3  4
+    #        AAAAAAA
+    #           BBBBBBB
+    #
+    # Or:
+    #
+    #     0  1  2  3  4
+    #        AAAAAAAAAA
+    #           BBBB
+    #
+    # So that we need to take whoever ends later. Back to our example,
+    # after merging and discard A and B it could be visually viewed as:
+    #
+    #     0  1  2  3  4  5  6  7
+    #        DDDDDDDDDD
+    #                       CCCC
+    #
+    # Now we could go on and compare the newly created D and the old C.
+    # We could figure out that D and C are not overlapping by checking
+    # `C.first <= D.last` is `false`. Therefore we need to keep both C
+    # and D. The example would end here because there are no more jobs.
+    #
+    # After having the union of all periods, we just need to sum the length
+    # of all periods to get total time.
+    #
+    #     (4 - 1) + (7 - 6) => 4
+    #
+    # That is 4 is the answer in the example.
+    module PipelineDuration
+      extend self
+
+      Period = Struct.new(:first, :last) do
+        def duration
+          last - first
+        end
+      end
+
+      def from_pipeline(pipeline)
+        status = %w[success failed running canceled]
+        builds = pipeline.builds.latest.
+          where(status: status).where.not(started_at: nil).order(:started_at)
+
+        from_builds(builds)
+      end
+
+      def from_builds(builds)
+        now = Time.now
+
+        periods = builds.map do |b|
+          Period.new(b.started_at, b.finished_at || now)
+        end
+
+        from_periods(periods)
+      end
+
+      # periods should be sorted by `first`
+      def from_periods(periods)
+        process_duration(process_periods(periods))
+      end
+
+      private
+
+      def process_periods(periods)
+        return periods if periods.empty?
+
+        periods.drop(1).inject([periods.first]) do |result, current|
+          previous = result.last
+
+          if overlap?(previous, current)
+            result[-1] = merge(previous, current)
+            result
+          else
+            result << current
+          end
+        end
+      end
+
+      def overlap?(previous, current)
+        current.first <= previous.last
+      end
+
+      def merge(previous, current)
+        Period.new(previous.first, [previous.last, current.last].max)
+      end
+
+      def process_duration(periods)
+        periods.sum(&:duration)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
new file mode 100644
index 00000000000..b26728a843c
--- /dev/null
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -0,0 +1,115 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::PipelineDuration do
+  let(:calculated_duration) { calculate(data) }
+
+  shared_examples 'calculating duration' do
+    it do
+      expect(calculated_duration).to eq(duration)
+    end
+  end
+
+  context 'test sample A' do
+    let(:data) do
+      [[0, 1],
+       [1, 2],
+       [3, 4],
+       [5, 6]]
+    end
+
+    let(:duration) { 4 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample B' do
+    let(:data) do
+      [[0, 1],
+       [1, 2],
+       [2, 3],
+       [3, 4],
+       [0, 4]]
+    end
+
+    let(:duration) { 4 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample C' do
+    let(:data) do
+      [[0, 4],
+       [2, 6],
+       [5, 7],
+       [8, 9]]
+    end
+
+    let(:duration) { 8 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample D' do
+    let(:data) do
+      [[0, 1],
+       [2, 3],
+       [4, 5],
+       [6, 7]]
+    end
+
+    let(:duration) { 4 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample E' do
+    let(:data) do
+      [[0, 1],
+       [3, 9],
+       [3, 4],
+       [3, 5],
+       [3, 8],
+       [4, 5],
+       [4, 7],
+       [5, 8]]
+    end
+
+    let(:duration) { 7 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample F' do
+    let(:data) do
+      [[1, 3],
+       [2, 4],
+       [2, 4],
+       [2, 4],
+       [5, 8]]
+    end
+
+    let(:duration) { 6 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample G' do
+    let(:data) do
+      [[1, 3],
+       [2, 4],
+       [6, 7]]
+    end
+
+    let(:duration) { 4 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  def calculate(data)
+    periods = data.shuffle.map do |(first, last)|
+      Gitlab::Ci::PipelineDuration::Period.new(first, last)
+    end
+
+    Gitlab::Ci::PipelineDuration.from_periods(periods.sort_by(&:first))
+  end
+end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 598df576001..fbf945c757c 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -124,21 +124,38 @@ describe Ci::Pipeline, models: true do
 
   describe 'state machine' do
     let(:current) { Time.now.change(usec: 0) }
-    let(:build) { create :ci_build, name: 'build1', pipeline: pipeline }
+    let(:build) { create_build('build1', current, 10) }
+    let(:build_b) { create_build('build2', current, 20) }
+    let(:build_c) { create_build('build3', current + 50, 10) }
 
     describe '#duration' do
       before do
-        travel_to(current - 120) do
+        pipeline.update(created_at: current)
+
+        travel_to(current + 5) do
           pipeline.run
+          pipeline.save
+        end
+
+        travel_to(current + 30) do
+          build.success
+        end
+
+        travel_to(current + 40) do
+          build_b.drop
         end
 
-        travel_to(current) do
-          pipeline.succeed
+        travel_to(current + 70) do
+          build_c.success
         end
+
+        pipeline.drop
       end
 
       it 'matches sum of builds duration' do
-        expect(pipeline.reload.duration).to eq(120)
+        pipeline.reload
+
+        expect(pipeline.duration).to eq(40)
       end
     end
 
@@ -169,6 +186,14 @@ describe Ci::Pipeline, models: true do
         expect(pipeline.reload.finished_at).to be_nil
       end
     end
+
+    def create_build(name, queued_at = current, started_from = 0)
+      create(:ci_build,
+             name: name,
+             pipeline: pipeline,
+             queued_at: queued_at,
+             started_at: queued_at + started_from)
+    end
   end
 
   describe '#branch?' do