From ace0a005b8bda05db224c21ac5ea691c3ffb6fb6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 29 Aug 2016 20:24:48 +0800
Subject: [PATCH 01/32] Smartly calculate real running time and pending time

---
 app/models/ci/pipeline.rb                     |  8 +-
 ...22117_add_pending_duration_to_pipelines.rb |  8 ++
 lib/gitlab/ci/pipeline_duration.rb            | 90 ++++++++++++++++++
 spec/lib/gitlab/ci/pipeline_duration_spec.rb  | 95 +++++++++++++++++++
 4 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
 create mode 100644 lib/gitlab/ci/pipeline_duration.rb
 create mode 100644 spec/lib/gitlab/ci/pipeline_duration_spec.rb

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 03812cd195f..e59c90e7e0c 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -258,7 +258,13 @@ module Ci
     end
 
     def update_duration
-      self.duration = calculate_duration
+      calculated_status = %w[success failed running canceled]
+      calculated_builds = builds.latest.where(status: calculated_status)
+      calculator = PipelineDuration.from_builds(calculated_builds)
+
+      self.duration = calculator.duration
+      self.pending_duration =
+        started_at - created_at + calculator.pending_duration
     end
 
     def execute_hooks
diff --git a/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb b/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
new file mode 100644
index 00000000000..f056f45c840
--- /dev/null
+++ b/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
@@ -0,0 +1,8 @@
+class AddPendingDurationToPipelines < ActiveRecord::Migration
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_commits, :pending_duration, :integer
+  end
+end
diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
new file mode 100644
index 00000000000..e4c0be3b640
--- /dev/null
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -0,0 +1,90 @@
+module Gitlab
+  module Ci
+    class PipelineDuration
+      SegmentStruct = Struct.new(:first, :last)
+      class Segment < SegmentStruct
+        def duration
+          last - first
+        end
+      end
+
+      def self.from_builds(builds)
+        now = Time.now
+
+        segments = builds.map do |b|
+          Segment.new(b.started_at, b.finished_at || now)
+        end
+
+        new(segments)
+      end
+
+      attr_reader :duration, :pending_duration
+
+      def initialize(segments)
+        process(segments.sort_by(&:first))
+      end
+
+      private
+
+      def process(segments)
+        merged = process_segments(segments)
+
+        @duration = process_duration(merged)
+        @pending_duration = process_pending_duration(merged, @duration)
+      end
+
+      def process_segments(segments)
+        if segments.empty?
+          segments
+        else
+          segments[1..-1].inject([segments.first]) do |current, target|
+            left, result = insert_segment(current, target)
+
+            if left # left is the latest one
+              result << left
+            else
+              result
+            end
+          end
+        end
+      end
+
+      def insert_segment(segments, init)
+        segments.inject([init, []]) do |target_result, member|
+          target, result = target_result
+
+          if target.nil? # done
+            result << member
+            [nil, result]
+          elsif merged = try_merge_segment(target, member) # overlapped
+            [merged, result] # merge and keep finding the hole
+          elsif target.last < member.first # found the hole
+            result << target << member
+            [nil, result]
+          else
+            result << member
+            target_result
+          end
+        end
+      end
+
+      def try_merge_segment(target, member)
+        if target.first <= member.last && target.last >= member.first
+          Segment.new([target.first, member.first].min,
+                      [target.last, member.last].max)
+        end
+      end
+
+      def process_duration(segments)
+        segments.inject(0) do |result, seg|
+          result + seg.duration
+        end
+      end
+
+      def process_pending_duration(segments, duration)
+        total = segments.last.last - segments.first.first
+        total - 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..7ac9a3bd6bc
--- /dev/null
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -0,0 +1,95 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::PipelineDuration do
+  let(:calculator) { create_calculator(data) }
+
+  shared_examples 'calculating duration' do
+    it do
+      expect(calculator.duration).to eq(duration)
+      expect(calculator.pending_duration).to eq(pending_duration)
+    end
+  end
+
+  context 'test sample A' do
+    let(:data) do
+      [[0, 1],
+       [1, 2],
+       [3, 4],
+       [5, 6]]
+    end
+
+    let(:duration) { 4 }
+    let(:pending_duration) { 2 }
+
+    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 }
+    let(:pending_duration) { 0 }
+
+    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 }
+    let(:pending_duration) { 1 }
+
+    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 }
+    let(:pending_duration) { 3 }
+
+    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 }
+    let(:pending_duration) { 2 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  def create_calculator(data)
+    segments = data.shuffle.map do |(first, last)|
+      Gitlab::Ci::PipelineDuration::Segment.new(first, last)
+    end
+
+    Gitlab::Ci::PipelineDuration.new(segments)
+  end
+end

From 1be5af31b086a79029b91d0bed5366b31fc6de1a Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 29 Aug 2016 20:41:00 +0800
Subject: [PATCH 02/32] I prefer to have empty lines around constants though

---
 db/migrate/20160829122117_add_pending_duration_to_pipelines.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb b/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
index f056f45c840..6f238a2f65c 100644
--- a/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
+++ b/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
@@ -1,5 +1,4 @@
 class AddPendingDurationToPipelines < ActiveRecord::Migration
-
   DOWNTIME = false
 
   def change

From e5d022c813d476383cc9889f8631d38ac8a85f62 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 29 Aug 2016 22:22:03 +0800
Subject: [PATCH 03/32] Fix constant name

---
 app/models/ci/pipeline.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index e59c90e7e0c..9db0f139162 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -260,7 +260,7 @@ module Ci
     def update_duration
       calculated_status = %w[success failed running canceled]
       calculated_builds = builds.latest.where(status: calculated_status)
-      calculator = PipelineDuration.from_builds(calculated_builds)
+      calculator = Gitlab::Ci::PipelineDuration.from_builds(calculated_builds)
 
       self.duration = calculator.duration
       self.pending_duration =

From f6051d71d62dd5b98daad44422867345685ed427 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 29 Aug 2016 23:37:15 +0800
Subject: [PATCH 04/32] no point to set duration while not started yet

---
 app/models/ci/pipeline.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 9db0f139162..9799c06c19f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -258,6 +258,8 @@ module Ci
     end
 
     def update_duration
+      return unless started_at
+
       calculated_status = %w[success failed running canceled]
       calculated_builds = builds.latest.where(status: calculated_status)
       calculator = Gitlab::Ci::PipelineDuration.from_builds(calculated_builds)

From e9e7c3788d1b2061e321032db4c72178f21ca6bd Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 29 Aug 2016 23:40:15 +0800
Subject: [PATCH 05/32] no builds no pending

---
 lib/gitlab/ci/pipeline_duration.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index e4c0be3b640..b9d0006182e 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -82,6 +82,8 @@ module Gitlab
       end
 
       def process_pending_duration(segments, duration)
+        return 0 if segments.empty?
+
         total = segments.last.last - segments.first.first
         total - duration
       end

From 7e32e2ef20feb9c2bbc1b1504cc56c9185621c86 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 02:53:53 +0800
Subject: [PATCH 06/32] build might not start yet

---
 lib/gitlab/ci/pipeline_duration.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index b9d0006182e..03bf7fabc99 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -12,7 +12,7 @@ module Gitlab
         now = Time.now
 
         segments = builds.map do |b|
-          Segment.new(b.started_at, b.finished_at || now)
+          Segment.new(b.started_at || now, b.finished_at || now)
         end
 
         new(segments)

From 031b162392c074b0bff1a5d4239acd8026592fdd Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 02:54:07 +0800
Subject: [PATCH 07/32] Fix test for Pipeline#duration

---
 spec/models/ci/pipeline_spec.rb | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 721b20e0cb2..a34b9cb4461 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -124,17 +124,22 @@ 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 - 60) }
+    let(:another_build) { create_build('build2', current + 20) }
 
     describe '#duration' do
       before do
-        travel_to(current - 120) do
-          pipeline.run
-        end
+        pipeline.run
 
         travel_to(current) do
-          pipeline.succeed
+          build.success
         end
+
+        travel_to(current + 80) do
+          another_build.drop
+        end
+
+        pipeline.drop
       end
 
       it 'matches sum of builds duration' do
@@ -169,6 +174,13 @@ describe Ci::Pipeline, models: true do
         expect(pipeline.reload.finished_at).to be_nil
       end
     end
+
+    def create_build(name, started_at = current)
+      create(:ci_build,
+             name: name,
+             pipeline: pipeline,
+             started_at: started_at)
+    end
   end
 
   describe '#branch?' do

From bd78e6af297c59d220090a037891003ec2571e22 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 23:02:39 +0800
Subject: [PATCH 08/32] Use algorithm from Kamil:

Excluding sorting, this is O(n) which should be much faster and
much simpler and easier to understand.
---
 lib/gitlab/ci/pipeline_duration.rb | 37 ++++++++----------------------
 1 file changed, 9 insertions(+), 28 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 03bf7fabc99..ff6dcdbfa31 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -37,41 +37,22 @@ module Gitlab
         if segments.empty?
           segments
         else
-          segments[1..-1].inject([segments.first]) do |current, target|
-            left, result = insert_segment(current, target)
+          segments.drop(1).inject([segments.first]) do |result, current|
+            merged = try_merge_segment(result.last, current)
 
-            if left # left is the latest one
-              result << left
-            else
+            if merged
+              result[-1] = merged
               result
+            else
+              result << current
             end
           end
         end
       end
 
-      def insert_segment(segments, init)
-        segments.inject([init, []]) do |target_result, member|
-          target, result = target_result
-
-          if target.nil? # done
-            result << member
-            [nil, result]
-          elsif merged = try_merge_segment(target, member) # overlapped
-            [merged, result] # merge and keep finding the hole
-          elsif target.last < member.first # found the hole
-            result << target << member
-            [nil, result]
-          else
-            result << member
-            target_result
-          end
-        end
-      end
-
-      def try_merge_segment(target, member)
-        if target.first <= member.last && target.last >= member.first
-          Segment.new([target.first, member.first].min,
-                      [target.last, member.last].max)
+      def try_merge_segment(previous, current)
+        if current.first <= previous.last
+          Segment.new(previous.first, [previous.last, current.last].max)
         end
       end
 

From 1f6efa352ce8cabc1d733cc54420daa86a7d22c0 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 23:06:29 +0800
Subject: [PATCH 09/32] Rename to periods since it's easier to understand

---
 lib/gitlab/ci/pipeline_duration.rb           | 44 ++++++++++----------
 spec/lib/gitlab/ci/pipeline_duration_spec.rb |  2 +-
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index ff6dcdbfa31..490e8f34cff 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -1,8 +1,8 @@
 module Gitlab
   module Ci
     class PipelineDuration
-      SegmentStruct = Struct.new(:first, :last)
-      class Segment < SegmentStruct
+      PeriodStruct = Struct.new(:first, :last)
+      class Period < SegmentStruct
         def duration
           last - first
         end
@@ -11,34 +11,34 @@ module Gitlab
       def self.from_builds(builds)
         now = Time.now
 
-        segments = builds.map do |b|
-          Segment.new(b.started_at || now, b.finished_at || now)
+        periods = builds.map do |b|
+          Period.new(b.started_at || now, b.finished_at || now)
         end
 
-        new(segments)
+        new(periods)
       end
 
       attr_reader :duration, :pending_duration
 
-      def initialize(segments)
-        process(segments.sort_by(&:first))
+      def initialize(periods)
+        process(periods.sort_by(&:first))
       end
 
       private
 
-      def process(segments)
-        merged = process_segments(segments)
+      def process(periods)
+        merged = process_periods(periods)
 
         @duration = process_duration(merged)
         @pending_duration = process_pending_duration(merged, @duration)
       end
 
-      def process_segments(segments)
-        if segments.empty?
-          segments
+      def process_periods(periods)
+        if periods.empty?
+          periods
         else
-          segments.drop(1).inject([segments.first]) do |result, current|
-            merged = try_merge_segment(result.last, current)
+          periods.drop(1).inject([periods.first]) do |result, current|
+            merged = try_merge_period(result.last, current)
 
             if merged
               result[-1] = merged
@@ -50,22 +50,22 @@ module Gitlab
         end
       end
 
-      def try_merge_segment(previous, current)
+      def try_merge_period(previous, current)
         if current.first <= previous.last
-          Segment.new(previous.first, [previous.last, current.last].max)
+          Period.new(previous.first, [previous.last, current.last].max)
         end
       end
 
-      def process_duration(segments)
-        segments.inject(0) do |result, seg|
-          result + seg.duration
+      def process_duration(periods)
+        periods.inject(0) do |result, per|
+          result + per.duration
         end
       end
 
-      def process_pending_duration(segments, duration)
-        return 0 if segments.empty?
+      def process_pending_duration(periods, duration)
+        return 0 if periods.empty?
 
-        total = segments.last.last - segments.first.first
+        total = periods.last.last - periods.first.first
         total - duration
       end
     end
diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index 7ac9a3bd6bc..5cd0727f46b 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -87,7 +87,7 @@ describe Gitlab::Ci::PipelineDuration do
 
   def create_calculator(data)
     segments = data.shuffle.map do |(first, last)|
-      Gitlab::Ci::PipelineDuration::Segment.new(first, last)
+      Gitlab::Ci::PipelineDuration::Period.new(first, last)
     end
 
     Gitlab::Ci::PipelineDuration.new(segments)

From 2af2b78e49c2089386919bc7a9d155fed244ec80 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 23:08:25 +0800
Subject: [PATCH 10/32] Avoid shadowing method name. Just use existing method

---
 lib/gitlab/ci/pipeline_duration.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 490e8f34cff..d8fb05724bc 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -30,7 +30,7 @@ module Gitlab
         merged = process_periods(periods)
 
         @duration = process_duration(merged)
-        @pending_duration = process_pending_duration(merged, @duration)
+        @pending_duration = process_pending_duration(merged)
       end
 
       def process_periods(periods)
@@ -62,7 +62,7 @@ module Gitlab
         end
       end
 
-      def process_pending_duration(periods, duration)
+      def process_pending_duration(periods)
         return 0 if periods.empty?
 
         total = periods.last.last - periods.first.first

From f10a1e331dc83fe3c973283d9f9a50f75c59d2d5 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 23:11:57 +0800
Subject: [PATCH 11/32] Fix renaming

---
 lib/gitlab/ci/pipeline_duration.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index d8fb05724bc..55d870e2e9d 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Ci
     class PipelineDuration
       PeriodStruct = Struct.new(:first, :last)
-      class Period < SegmentStruct
+      class Period < PeriodStruct
         def duration
           last - first
         end

From 4789adc565b08ed4979ef3b876b2fbd6224fa084 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 30 Aug 2016 23:12:55 +0800
Subject: [PATCH 12/32] Add test cases from:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14709761
---
 spec/lib/gitlab/ci/pipeline_duration_spec.rb | 28 ++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index 5cd0727f46b..21d5a5c96a2 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -85,6 +85,34 @@ describe Gitlab::Ci::PipelineDuration do
     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 }
+    let(:pending_duration) { 1 }
+
+    it_behaves_like 'calculating duration'
+  end
+
+  context 'test sample G' do
+    let(:data) do
+      [[1, 3],
+       [2, 4],
+       [6, 7]]
+    end
+
+    let(:duration) { 4 }
+    let(:pending_duration) { 2 }
+
+    it_behaves_like 'calculating duration'
+  end
+
   def create_calculator(data)
     segments = data.shuffle.map do |(first, last)|
       Gitlab::Ci::PipelineDuration::Period.new(first, last)

From 7dcc2446077084c0cbce92bc5b1279984cc75920 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 Aug 2016 00:49:10 +0800
Subject: [PATCH 13/32] It's renamed to periods

---
 spec/lib/gitlab/ci/pipeline_duration_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index 21d5a5c96a2..acb690581d6 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -114,10 +114,10 @@ describe Gitlab::Ci::PipelineDuration do
   end
 
   def create_calculator(data)
-    segments = data.shuffle.map do |(first, last)|
+    periods = data.shuffle.map do |(first, last)|
       Gitlab::Ci::PipelineDuration::Period.new(first, last)
     end
 
-    Gitlab::Ci::PipelineDuration.new(segments)
+    Gitlab::Ci::PipelineDuration.new(periods)
   end
 end

From 09a53359e4847dfd146141cc3672d48f6b1df233 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 Aug 2016 00:51:24 +0800
Subject: [PATCH 14/32] Add CHANGELOG entry

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index d06fc24d40a..7aab8715801 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,6 +13,7 @@ v 8.12.0 (unreleased)
   - Remove Gitorious import
   - Add Sentry logging to API calls
   - 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
   - Fix groups sort dropdown alignment (ClemMakesApps)
   - Add horizontal scrolling to all sub-navs on mobile viewports (ClemMakesApps)
   - Fix markdown help references (ClemMakesApps)

From 1e49a8bc6cd593910577a0f09ea13f0c933de1e9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 Aug 2016 03:09:16 +0800
Subject: [PATCH 15/32] Introduction to PipelineDuration

---
 lib/gitlab/ci/pipeline_duration.rb | 98 ++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 55d870e2e9d..f97727e8548 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -1,5 +1,103 @@
 module Gitlab
   module Ci
+    # The problem this class 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` and `Period#last`. 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
+    #
+    # And the pending (queue) time would be (4, 6) like this: (marked as X)
+    #
+    #     0  1  2  3  4  5  6  7
+    #        AAAAAAA
+    #           BBBBBBB
+    #                       CCCC
+    #                  XXXXX
+    #
+    # Which could be calculated by having (1, 7) as total time, minus
+    # the running time we have above, 4. The full calculation would be:
+    #
+    #     total = (7 - 1)
+    #     duration = (4 - 1) + (7 - 6)
+    #     pending = total - duration # 6 - 4 => 2
+    #
+    # Which the answer to pending would be 2 in this example.
+    #
+    # 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, the rest is simple and
+    # described in the beginning. To summarise:
+    #
+    #     duration = (4 - 1) + (7 - 6)
+    #     total = (7 - 1)
+    #     pending = total - duration # 6 - 4 => 2
+    #
+    # Note that the pending time is actually not the final pending time
+    # for pipelines, because we still need to accumulate the pending time
+    # before the first job (A in this example) even started! That is:
+    #
+    #     total_pending = pipeline.started_at - pipeline.created_at + pending
+    #
+    # Would be the final answer. We deal with that in pipeline itself
+    # but not here because here we try not to be depending on pipeline
+    # and it's trivial enough to get that information.
     class PipelineDuration
       PeriodStruct = Struct.new(:first, :last)
       class Period < PeriodStruct

From d2cfcb3ec1327cd9dd901dcbe8c927e3c43cfb38 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 Aug 2016 16:48:29 +0800
Subject: [PATCH 16/32] Use guard clause, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14754681
---
 lib/gitlab/ci/pipeline_duration.rb | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index f97727e8548..e37ba19bca9 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -132,18 +132,16 @@ module Gitlab
       end
 
       def process_periods(periods)
-        if periods.empty?
-          periods
-        else
-          periods.drop(1).inject([periods.first]) do |result, current|
-            merged = try_merge_period(result.last, current)
+        return periods if periods.empty?
 
-            if merged
-              result[-1] = merged
-              result
-            else
-              result << current
-            end
+        periods.drop(1).inject([periods.first]) do |result, current|
+          merged = try_merge_period(result.last, current)
+
+          if merged
+            result[-1] = merged
+            result
+          else
+            result << current
           end
         end
       end

From 39c090fecbdce0031d789a2da5826d25a59c73a4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 19:15:42 +0800
Subject: [PATCH 17/32] 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
---
 app/models/ci/pipeline.rb                    |  9 ++-----
 lib/gitlab/ci/pipeline_duration.rb           | 28 ++++++++++----------
 spec/lib/gitlab/ci/pipeline_duration_spec.rb |  8 ------
 spec/models/ci/pipeline_spec.rb              | 27 ++++++++++++-------
 4 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 9799c06c19f..b7d4b492a75 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -260,13 +260,8 @@ module Ci
     def update_duration
       return unless started_at
 
-      calculated_status = %w[success failed running canceled]
-      calculated_builds = builds.latest.where(status: calculated_status)
-      calculator = Gitlab::Ci::PipelineDuration.from_builds(calculated_builds)
-
-      self.duration = calculator.duration
-      self.pending_duration =
-        started_at - created_at + calculator.pending_duration
+      self.duration, self.pending_duration =
+        Gitlab::Ci::PipelineDuration.from_pipeline(self)
     end
 
     def execute_hooks
diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index e37ba19bca9..baf84954b7e 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -106,17 +106,27 @@ module Gitlab
         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
 
         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
 
         new(periods)
       end
 
-      attr_reader :duration, :pending_duration
+      attr_reader :duration
 
       def initialize(periods)
         process(periods.sort_by(&:first))
@@ -125,10 +135,7 @@ module Gitlab
       private
 
       def process(periods)
-        merged = process_periods(periods)
-
-        @duration = process_duration(merged)
-        @pending_duration = process_pending_duration(merged)
+        @duration = process_duration(process_periods(periods))
       end
 
       def process_periods(periods)
@@ -157,13 +164,6 @@ module Gitlab
           result + per.duration
         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
diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index acb690581d6..a160c0a6e39 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -6,7 +6,6 @@ describe Gitlab::Ci::PipelineDuration do
   shared_examples 'calculating duration' do
     it do
       expect(calculator.duration).to eq(duration)
-      expect(calculator.pending_duration).to eq(pending_duration)
     end
   end
 
@@ -19,7 +18,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 4 }
-    let(:pending_duration) { 2 }
 
     it_behaves_like 'calculating duration'
   end
@@ -34,7 +32,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 4 }
-    let(:pending_duration) { 0 }
 
     it_behaves_like 'calculating duration'
   end
@@ -48,7 +45,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 8 }
-    let(:pending_duration) { 1 }
 
     it_behaves_like 'calculating duration'
   end
@@ -62,7 +58,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 4 }
-    let(:pending_duration) { 3 }
 
     it_behaves_like 'calculating duration'
   end
@@ -80,7 +75,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 7 }
-    let(:pending_duration) { 2 }
 
     it_behaves_like 'calculating duration'
   end
@@ -95,7 +89,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 6 }
-    let(:pending_duration) { 1 }
 
     it_behaves_like 'calculating duration'
   end
@@ -108,7 +101,6 @@ describe Gitlab::Ci::PipelineDuration do
     end
 
     let(:duration) { 4 }
-    let(:pending_duration) { 2 }
 
     it_behaves_like 'calculating duration'
   end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index a34b9cb4461..2d239caab64 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -124,18 +124,23 @@ describe Ci::Pipeline, models: true do
 
   describe 'state machine' do
     let(:current) { Time.now.change(usec: 0) }
-    let(:build) { create_build('build1', current - 60) }
-    let(:another_build) { create_build('build2', current + 20) }
+    let(:build) { create_build('build1', current, 10) }
+    let(:another_build) { create_build('build2', current + 80, 5) }
 
-    describe '#duration' do
+    describe '#duration and #pending_duration' do
       before do
-        pipeline.run
+        pipeline.update(created_at: current)
 
-        travel_to(current) do
+        travel_to(current + 5) do
+          pipeline.run
+          pipeline.save
+        end
+
+        travel_to(current + 70) do
           build.success
         end
 
-        travel_to(current + 80) do
+        travel_to(current + 145) do
           another_build.drop
         end
 
@@ -143,7 +148,10 @@ describe Ci::Pipeline, models: true do
       end
 
       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
 
@@ -175,11 +183,12 @@ describe Ci::Pipeline, models: true do
       end
     end
 
-    def create_build(name, started_at = current)
+    def create_build(name, queued_at = current, started_from = 0)
       create(:ci_build,
              name: name,
              pipeline: pipeline,
-             started_at: started_at)
+             queued_at: queued_at,
+             started_at: queued_at + started_from)
     end
   end
 

From 1ccb578ec89df5d69c997920fd512a1db44e309d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 13:00:56 +0000
Subject: [PATCH 18/32] Update schema for pending_duration

---
 db/schema.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/db/schema.rb b/db/schema.rb
index af6e74a4e25..f57c4149e6c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -210,6 +210,7 @@ ActiveRecord::Schema.define(version: 20160831223750) do
     t.datetime "finished_at"
     t.integer  "duration"
     t.integer  "user_id"
+    t.integer  "pending_duration"
   end
 
   add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree

From defd8899056c26593939243b05c365605ac06fdc Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 21:08:26 +0800
Subject: [PATCH 19/32] Actually we still need to use total - running to get:

real pending time, because that's actually by definition,
(the time that it's not running!)
or we could end up with awfully complicated algorithm :(
---
 lib/gitlab/ci/pipeline_duration.rb |  7 ++++---
 spec/models/ci/pipeline_spec.rb    | 17 +++++++++++------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index baf84954b7e..0c7c38e2b67 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -110,10 +110,11 @@ module Gitlab
         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
+        running = from_builds(builds, :started_at, :finished_at).duration
+        total = from_builds(builds, :queued_at, :finished_at).duration
+        pending = pipeline.started_at - pipeline.created_at
 
-        [duration, pipeline.started_at - pipeline.created_at + queued]
+        [running, pending + total - running]
       end
 
       def self.from_builds(builds, from, to)
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 68a88217afe..4615daa6551 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -125,7 +125,8 @@ describe Ci::Pipeline, models: true do
   describe 'state machine' do
     let(:current) { Time.now.change(usec: 0) }
     let(:build) { create_build('build1', current, 10) }
-    let(:another_build) { create_build('build2', current + 80, 5) }
+    let(:build_b) { create_build('build2', current, 20) }
+    let(:build_c) { create_build('build3', current + 40, 20) }
 
     describe '#duration and #pending_duration' do
       before do
@@ -136,12 +137,16 @@ describe Ci::Pipeline, models: true do
           pipeline.save
         end
 
-        travel_to(current + 70) do
+        travel_to(current + 30) do
           build.success
         end
 
-        travel_to(current + 145) do
-          another_build.drop
+        travel_to(current + 40) do
+          build_b.drop
+        end
+
+        travel_to(current + 70) do
+          build_c.success
         end
 
         pipeline.drop
@@ -150,8 +155,8 @@ describe Ci::Pipeline, models: true do
       it 'matches sum of builds duration' do
         pipeline.reload
 
-        expect(pipeline.duration).to eq(70 - 10 + 145 - 85)
-        expect(pipeline.pending_duration).to eq(5 + 10 + 5)
+        expect(pipeline.duration).to eq(40)
+        expect(pipeline.pending_duration).to eq(35)
       end
     end
 

From 8d96062f2386a4e9c30631c35cb41f827cc25beb Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 21:16:40 +0800
Subject: [PATCH 20/32] Make sure the algorithm did exclude gaps

---
 spec/models/ci/pipeline_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 4615daa6551..2c04ef298bc 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -126,7 +126,7 @@ describe Ci::Pipeline, models: true do
     let(:current) { Time.now.change(usec: 0) }
     let(:build) { create_build('build1', current, 10) }
     let(:build_b) { create_build('build2', current, 20) }
-    let(:build_c) { create_build('build3', current + 40, 20) }
+    let(:build_c) { create_build('build3', current + 50, 10) }
 
     describe '#duration and #pending_duration' do
       before do
@@ -156,7 +156,7 @@ describe Ci::Pipeline, models: true do
         pipeline.reload
 
         expect(pipeline.duration).to eq(40)
-        expect(pipeline.pending_duration).to eq(35)
+        expect(pipeline.pending_duration).to eq(25)
       end
     end
 

From 7aaed299eb744e524a1597ceed923c14b556ef20 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 2 Sep 2016 23:43:10 +0800
Subject: [PATCH 21/32] Just sum all the queuing time, indication for needing
 more runners

---
 lib/gitlab/ci/pipeline_duration.rb | 13 +++++++------
 spec/models/ci/pipeline_spec.rb    |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 0c7c38e2b67..ac68fd0272f 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -107,19 +107,20 @@ module Gitlab
       end
 
       def self.from_pipeline(pipeline)
+        now = Time.now
         status = %w[success failed running canceled]
         builds = pipeline.builds.latest.where(status: status)
 
-        running = from_builds(builds, :started_at, :finished_at).duration
-        total = from_builds(builds, :queued_at, :finished_at).duration
+        running = from_builds(builds, :started_at, :finished_at, now).duration
         pending = pipeline.started_at - pipeline.created_at
+        queuing = builds.inject(0) do |result, job|
+          result + ((job.started_at || now) - (job.queued_at || now))
+        end
 
-        [running, pending + total - running]
+        [running, pending + queuing]
       end
 
-      def self.from_builds(builds, from, to)
-        now = Time.now
-
+      def self.from_builds(builds, from, to, now = Time.now)
         periods = builds.map do |b|
           Period.new(b.public_send(from) || now, b.public_send(to) || now)
         end
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 2c04ef298bc..194c3fc01f1 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -156,7 +156,7 @@ describe Ci::Pipeline, models: true do
         pipeline.reload
 
         expect(pipeline.duration).to eq(40)
-        expect(pipeline.pending_duration).to eq(25)
+        expect(pipeline.pending_duration).to eq(45)
       end
     end
 

From d071f61b0de12a57012ed1c77634b54fa83615a7 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 5 Sep 2016 17:55:30 +0800
Subject: [PATCH 22/32] Forget about pending duration for now, need more
 discussion

---
 app/models/ci/pipeline.rb                     |  3 +-
 ...22117_add_pending_duration_to_pipelines.rb |  7 ---
 db/schema.rb                                  |  1 -
 lib/gitlab/ci/pipeline_duration.rb            | 49 ++++---------------
 4 files changed, 11 insertions(+), 49 deletions(-)
 delete mode 100644 db/migrate/20160829122117_add_pending_duration_to_pipelines.rb

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index de7709c36c4..4092205578e 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -260,8 +260,7 @@ module Ci
     def update_duration
       return unless started_at
 
-      self.duration, self.pending_duration =
-        Gitlab::Ci::PipelineDuration.from_pipeline(self)
+      self.duration = Gitlab::Ci::PipelineDuration.from_pipeline(self)
     end
 
     def execute_hooks
diff --git a/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb b/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
deleted file mode 100644
index 6f238a2f65c..00000000000
--- a/db/migrate/20160829122117_add_pending_duration_to_pipelines.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-class AddPendingDurationToPipelines < ActiveRecord::Migration
-  DOWNTIME = false
-
-  def change
-    add_column :ci_commits, :pending_duration, :integer
-  end
-end
diff --git a/db/schema.rb b/db/schema.rb
index f57c4149e6c..af6e74a4e25 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -210,7 +210,6 @@ ActiveRecord::Schema.define(version: 20160831223750) do
     t.datetime "finished_at"
     t.integer  "duration"
     t.integer  "user_id"
-    t.integer  "pending_duration"
   end
 
   add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree
diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index ac68fd0272f..cd782934302 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -1,11 +1,14 @@
 module Gitlab
   module Ci
+    # # Introduction - total running time
+    #
     # The problem this class 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` and `Period#last`. A simple example here would be:
+    # `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)
@@ -24,22 +27,7 @@ module Gitlab
     #
     #     (4 - 1) + (7 - 6) => 4
     #
-    # And the pending (queue) time would be (4, 6) like this: (marked as X)
-    #
-    #     0  1  2  3  4  5  6  7
-    #        AAAAAAA
-    #           BBBBBBB
-    #                       CCCC
-    #                  XXXXX
-    #
-    # Which could be calculated by having (1, 7) as total time, minus
-    # the running time we have above, 4. The full calculation would be:
-    #
-    #     total = (7 - 1)
-    #     duration = (4 - 1) + (7 - 6)
-    #     pending = total - duration # 6 - 4 => 2
-    #
-    # Which the answer to pending would be 2 in this example.
+    # # 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`.
@@ -82,22 +70,12 @@ module Gitlab
     # `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, the rest is simple and
-    # described in the beginning. To summarise:
+    # After having the union of all periods, we just need to sum the length
+    # of all periods to get total time.
     #
-    #     duration = (4 - 1) + (7 - 6)
-    #     total = (7 - 1)
-    #     pending = total - duration # 6 - 4 => 2
+    #     (4 - 1) + (7 - 6) => 4
     #
-    # Note that the pending time is actually not the final pending time
-    # for pipelines, because we still need to accumulate the pending time
-    # before the first job (A in this example) even started! That is:
-    #
-    #     total_pending = pipeline.started_at - pipeline.created_at + pending
-    #
-    # Would be the final answer. We deal with that in pipeline itself
-    # but not here because here we try not to be depending on pipeline
-    # and it's trivial enough to get that information.
+    # That is 4 is the answer in the example.
     class PipelineDuration
       PeriodStruct = Struct.new(:first, :last)
       class Period < PeriodStruct
@@ -107,17 +85,10 @@ module Gitlab
       end
 
       def self.from_pipeline(pipeline)
-        now = Time.now
         status = %w[success failed running canceled]
         builds = pipeline.builds.latest.where(status: status)
 
-        running = from_builds(builds, :started_at, :finished_at, now).duration
-        pending = pipeline.started_at - pipeline.created_at
-        queuing = builds.inject(0) do |result, job|
-          result + ((job.started_at || now) - (job.queued_at || now))
-        end
-
-        [running, pending + queuing]
+        from_builds(builds, :started_at, :finished_at).duration
       end
 
       def self.from_builds(builds, from, to, now = Time.now)

From 6e1a06d8cd761692596696ab4e17ccaf0c68519d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 5 Sep 2016 18:49:27 +0800
Subject: [PATCH 23/32] Show how long this pipeline was queued

Closes #19804
---
 app/models/ci/pipeline.rb                    | 7 +++++++
 app/views/projects/pipelines/_info.html.haml | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 4092205578e..0b1df9f4294 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -257,6 +257,13 @@ 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
       return unless started_at
 
diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 063e83a407a..03f5f55e336 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -10,6 +10,9 @@
   - 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

From d354d185ff8e6355d03457d61fd8424b4494e5cb Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 5 Sep 2016 19:18:14 +0800
Subject: [PATCH 24/32] Remove tests for pending_duration

---
 spec/models/ci/pipeline_spec.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 194c3fc01f1..fbf945c757c 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -128,7 +128,7 @@ describe Ci::Pipeline, models: true do
     let(:build_b) { create_build('build2', current, 20) }
     let(:build_c) { create_build('build3', current + 50, 10) }
 
-    describe '#duration and #pending_duration' do
+    describe '#duration' do
       before do
         pipeline.update(created_at: current)
 
@@ -156,7 +156,6 @@ describe Ci::Pipeline, models: true do
         pipeline.reload
 
         expect(pipeline.duration).to eq(40)
-        expect(pipeline.pending_duration).to eq(45)
       end
     end
 

From 413516081b91f3e64305fa3f4dffd84a564596a7 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 5 Sep 2016 21:13:13 +0800
Subject: [PATCH 25/32] Also add an entry for showing queued time [ci skip]

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index 0e7972c2704..1e286cca135 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -29,6 +29,7 @@ v 8.12.0 (unreleased)
   - Add BroadcastMessage 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)

From b92c75ab982e123e2e1efc189e35b84c0ffd1c27 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Sep 2016 18:12:46 +0800
Subject: [PATCH 26/32] Use sum, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14991392
---
 lib/gitlab/ci/pipeline_duration.rb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index cd782934302..9fe4996fc09 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -133,9 +133,7 @@ module Gitlab
       end
 
       def process_duration(periods)
-        periods.inject(0) do |result, per|
-          result + per.duration
-        end
+        periods.sum(&:duration)
       end
     end
   end

From 3a68c98973c2c17e0c6e57184c4b2605a2dd274e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Sep 2016 19:14:28 +0800
Subject: [PATCH 27/32] Just use module because there's nothing to save,
 feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14992064
---
 lib/gitlab/ci/pipeline_duration.rb           | 24 ++++++++------------
 spec/lib/gitlab/ci/pipeline_duration_spec.rb |  8 +++----
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 9fe4996fc09..0333807263f 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -2,7 +2,7 @@ module Gitlab
   module Ci
     # # Introduction - total running time
     #
-    # The problem this class is trying to solve is finding the total running
+    # 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.
     #
@@ -76,7 +76,9 @@ module Gitlab
     #     (4 - 1) + (7 - 6) => 4
     #
     # That is 4 is the answer in the example.
-    class PipelineDuration
+    module PipelineDuration
+      extend self
+
       PeriodStruct = Struct.new(:first, :last)
       class Period < PeriodStruct
         def duration
@@ -84,33 +86,27 @@ module Gitlab
         end
       end
 
-      def self.from_pipeline(pipeline)
+      def from_pipeline(pipeline)
         status = %w[success failed running canceled]
         builds = pipeline.builds.latest.where(status: status)
 
-        from_builds(builds, :started_at, :finished_at).duration
+        from_builds(builds, :started_at, :finished_at)
       end
 
-      def self.from_builds(builds, from, to, now = Time.now)
+      def from_builds(builds, from, to, now = Time.now)
         periods = builds.map do |b|
           Period.new(b.public_send(from) || now, b.public_send(to) || now)
         end
 
-        new(periods)
+        from_periods(periods)
       end
 
-      attr_reader :duration
-
-      def initialize(periods)
-        process(periods.sort_by(&:first))
+      def from_periods(periods)
+        process_duration(process_periods(periods.sort_by(&:first)))
       end
 
       private
 
-      def process(periods)
-        @duration = process_duration(process_periods(periods))
-      end
-
       def process_periods(periods)
         return periods if periods.empty?
 
diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index a160c0a6e39..580af97bea7 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::PipelineDuration do
-  let(:calculator) { create_calculator(data) }
+  let(:calculated_duration) { calculate(data) }
 
   shared_examples 'calculating duration' do
     it do
-      expect(calculator.duration).to eq(duration)
+      expect(calculated_duration).to eq(duration)
     end
   end
 
@@ -105,11 +105,11 @@ describe Gitlab::Ci::PipelineDuration do
     it_behaves_like 'calculating duration'
   end
 
-  def create_calculator(data)
+  def calculate(data)
     periods = data.shuffle.map do |(first, last)|
       Gitlab::Ci::PipelineDuration::Period.new(first, last)
     end
 
-    Gitlab::Ci::PipelineDuration.new(periods)
+    Gitlab::Ci::PipelineDuration.from_periods(periods)
   end
 end

From 7351c269deae2c28c63e793df11155e0f6c75d04 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Sep 2016 19:36:07 +0800
Subject: [PATCH 28/32] Sort by database, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14991226
and
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14994233
---
 lib/gitlab/ci/pipeline_duration.rb           | 6 ++++--
 spec/lib/gitlab/ci/pipeline_duration_spec.rb | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 0333807263f..711f911346c 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -88,7 +88,8 @@ module Gitlab
 
       def from_pipeline(pipeline)
         status = %w[success failed running canceled]
-        builds = pipeline.builds.latest.where(status: status)
+        builds = pipeline.builds.latest.
+          where(status: status).where.not(started_at: nil).order(:started_at)
 
         from_builds(builds, :started_at, :finished_at)
       end
@@ -101,8 +102,9 @@ module Gitlab
         from_periods(periods)
       end
 
+      # periods should be sorted by `first`
       def from_periods(periods)
-        process_duration(process_periods(periods.sort_by(&:first)))
+        process_duration(process_periods(periods))
       end
 
       private
diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
index 580af97bea7..b26728a843c 100644
--- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline_duration_spec.rb
@@ -110,6 +110,6 @@ describe Gitlab::Ci::PipelineDuration do
       Gitlab::Ci::PipelineDuration::Period.new(first, last)
     end
 
-    Gitlab::Ci::PipelineDuration.from_periods(periods)
+    Gitlab::Ci::PipelineDuration.from_periods(periods.sort_by(&:first))
   end
 end

From 61bc90af0ea247c5f561d8b71348ab028566033d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 6 Sep 2016 19:57:51 +0800
Subject: [PATCH 29/32] Be more specific since it's not needed to be generic
 now,

feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_14995016
---
 lib/gitlab/ci/pipeline_duration.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 711f911346c..10ad70f14fa 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -91,12 +91,14 @@ module Gitlab
         builds = pipeline.builds.latest.
           where(status: status).where.not(started_at: nil).order(:started_at)
 
-        from_builds(builds, :started_at, :finished_at)
+        from_builds(builds)
       end
 
-      def from_builds(builds, from, to, now = Time.now)
+      def from_builds(builds)
+        now = Time.now
+
         periods = builds.map do |b|
-          Period.new(b.public_send(from) || now, b.public_send(to) || now)
+          Period.new(b.started_at, b.finished_at || now)
         end
 
         from_periods(periods)

From 7b75476610cd4ac9f1c8b83b721e6bc524b781a1 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 7 Sep 2016 17:14:35 +0800
Subject: [PATCH 30/32] Just use string interpolation, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_15026132
---
 app/views/projects/pipelines/_info.html.haml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 03f5f55e336..5800ef7de48 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -11,8 +11,7 @@
     in
     = time_interval_in_words(@pipeline.duration)
   - if @pipeline.queued_duration
-    (queued for
-    = "#{time_interval_in_words(@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

From 27a3f1182ae2664f27150bd91f74a4f732cfd4af Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 8 Sep 2016 21:47:19 +0800
Subject: [PATCH 31/32] Split try_merge_period into overlap? and merge:

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_15083507
---
 lib/gitlab/ci/pipeline_duration.rb | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 10ad70f14fa..311f5a7f8d8 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -115,10 +115,10 @@ module Gitlab
         return periods if periods.empty?
 
         periods.drop(1).inject([periods.first]) do |result, current|
-          merged = try_merge_period(result.last, current)
+          previous = result.last
 
-          if merged
-            result[-1] = merged
+          if overlap?(previous, current)
+            result[-1] = merge(previous, current)
             result
           else
             result << current
@@ -126,10 +126,12 @@ module Gitlab
         end
       end
 
-      def try_merge_period(previous, current)
-        if current.first <= previous.last
-          Period.new(previous.first, [previous.last, current.last].max)
-        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)

From 822efd5c3b08dbd51ae4b468863475fa9d0ebc43 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 8 Sep 2016 23:55:07 +0800
Subject: [PATCH 32/32] Struct.new could take a block for defining methods,
 feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6084#note_15091858
---
 lib/gitlab/ci/pipeline_duration.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb
index 311f5a7f8d8..a210e76acaa 100644
--- a/lib/gitlab/ci/pipeline_duration.rb
+++ b/lib/gitlab/ci/pipeline_duration.rb
@@ -79,8 +79,7 @@ module Gitlab
     module PipelineDuration
       extend self
 
-      PeriodStruct = Struct.new(:first, :last)
-      class Period < PeriodStruct
+      Period = Struct.new(:first, :last) do
         def duration
           last - first
         end