Merge branch 'zj-faster-charts-page' into 'master'

Improve performance for pipeline charts

Closes #32407

See merge request !12378

9 files changed, 35 insertions, 33 deletions

diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 8effb792689..303e91a8dc0 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -135,7 +135,12 @@ class Projects::PipelinesController < Projects::ApplicationController
     @charts[:week] = Ci::Charts::WeekChart.new(project)
     @charts[:month] = Ci::Charts::MonthChart.new(project)
     @charts[:year] = Ci::Charts::YearChart.new(project)
-    @charts[:build_times] = Ci::Charts::BuildTime.new(project)
+    @charts[:pipeline_times] = Ci::Charts::PipelineTime.new(project)
+
+    @counts = {}
+    @counts[:total] = @project.pipelines.count(:all)
+    @counts[:success] = @project.pipelines.success.count(:all)
+    @counts[:failed] = @project.pipelines.failed.count(:all)
   end
 
   private
diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb
index c2ab80f2e0d..2e9b72e9613 100644
--- a/app/helpers/graph_helper.rb
+++ b/app/helpers/graph_helper.rb
@@ -17,13 +17,10 @@ module GraphHelper
     ids.zip(parent_spaces)
   end
 
-  def success_ratio(success_builds, failed_builds)
-    failed_builds = failed_builds.count(:all)
-    success_builds = success_builds.count(:all)
+  def success_ratio(counts)
+    return 100 if counts[:failed].zero?
 
-    return 100 if failed_builds.zero?
-
-    ratio = (success_builds.to_f / (success_builds + failed_builds)) * 100
+    ratio = (counts[:success].to_f / (counts[:success] + counts[:failed])) * 100
     ratio.to_i
   end
 end
diff --git a/app/views/projects/pipelines/charts.html.haml b/app/views/projects/pipelines/charts.html.haml
index 4a5043aac3c..8ffddfe6154 100644
--- a/app/views/projects/pipelines/charts.html.haml
+++ b/app/views/projects/pipelines/charts.html.haml
@@ -15,7 +15,7 @@
       .col-md-6
         = render 'projects/pipelines/charts/overall'
       .col-md-6
-        = render 'projects/pipelines/charts/build_times'
+        = render 'projects/pipelines/charts/pipeline_times'
 
     %hr
-    = render 'projects/pipelines/charts/builds'
+    = render 'projects/pipelines/charts/pipelines'
diff --git a/app/views/projects/pipelines/charts/_overall.haml b/app/views/projects/pipelines/charts/_overall.haml
index 0b7e3d22dd7..93083397d5b 100644
--- a/app/views/projects/pipelines/charts/_overall.haml
+++ b/app/views/projects/pipelines/charts/_overall.haml
@@ -2,18 +2,14 @@
 %ul
   %li
     Total:
-    %strong= pluralize @project.builds.count(:all), 'job'
+    %strong= pluralize @counts[:total], 'pipeline'
   %li
     Successful:
-    %strong= pluralize @project.builds.success.count(:all), 'job'
+    %strong= pluralize @counts[:success], 'pipeline'
   %li
     Failed:
-    %strong= pluralize @project.builds.failed.count(:all), 'job'
+    %strong= pluralize @counts[:failed], 'pipeline'
   %li
     Success ratio:
     %strong
-      #{success_ratio(@project.builds.success, @project.builds.failed)}%
-  %li
-    Commits covered:
-    %strong
-      = @project.pipelines.count(:all)
+      #{success_ratio(@counts)}%
diff --git a/app/views/projects/pipelines/charts/_build_times.haml b/app/views/projects/pipelines/charts/_pipeline_times.haml
index bb0975a9535..aee7c5492aa 100644
--- a/app/views/projects/pipelines/charts/_build_times.haml
+++ b/app/views/projects/pipelines/charts/_pipeline_times.haml
@@ -6,7 +6,7 @@
 
 :javascript
   var data = {
-    labels : #{@charts[:build_times].labels.to_json},
+    labels : #{@charts[:pipeline_times].labels.to_json},
     datasets : [
       {
         fillColor : "rgba(220,220,220,0.5)",
@@ -14,7 +14,7 @@
         barStrokeWidth: 1,
         barValueSpacing: 1,
         barDatasetSpacing: 1,
-        data : #{@charts[:build_times].build_times.to_json}
+        data : #{@charts[:pipeline_times].pipeline_times.to_json}
       }
     ]
   }
diff --git a/app/views/projects/pipelines/charts/_builds.haml b/app/views/projects/pipelines/charts/_pipelines.haml
index b6f453b9736..b6f453b9736 100644
--- a/app/views/projects/pipelines/charts/_builds.haml
+++ b/app/views/projects/pipelines/charts/_pipelines.haml
diff --git a/changelogs/unreleased/zj-faster-charts-page.yml b/changelogs/unreleased/zj-faster-charts-page.yml
new file mode 100644
index 00000000000..9afcf111328
--- /dev/null
+++ b/changelogs/unreleased/zj-faster-charts-page.yml
@@ -0,0 +1,4 @@
+---
+title: Improve performance of the pipeline charts page
+merge_request: 12378
+author:
diff --git a/lib/ci/charts.rb b/lib/ci/charts.rb
index 6063d6f45e8..872e418c788 100644
--- a/lib/ci/charts.rb
+++ b/lib/ci/charts.rb
@@ -3,7 +3,7 @@ module Ci
     module DailyInterval
       def grouped_count(query)
         query
-          .group("DATE(#{Ci::Build.table_name}.created_at)")
+          .group("DATE(#{Ci::Pipeline.table_name}.created_at)")
           .count(:created_at)
           .transform_keys { |date| date.strftime(@format) }
       end
@@ -17,12 +17,12 @@ module Ci
       def grouped_count(query)
         if Gitlab::Database.postgresql?
           query
-            .group("to_char(#{Ci::Build.table_name}.created_at, '01 Month YYYY')")
+            .group("to_char(#{Ci::Pipeline.table_name}.created_at, '01 Month YYYY')")
             .count(:created_at)
             .transform_keys(&:squish)
         else
           query
-            .group("DATE_FORMAT(#{Ci::Build.table_name}.created_at, '01 %M %Y')")
+            .group("DATE_FORMAT(#{Ci::Pipeline.table_name}.created_at, '01 %M %Y')")
             .count(:created_at)
         end
       end
@@ -33,21 +33,21 @@ module Ci
     end
 
     class Chart
-      attr_reader :labels, :total, :success, :project, :build_times
+      attr_reader :labels, :total, :success, :project, :pipeline_times
 
       def initialize(project)
         @labels = []
         @total = []
         @success = []
-        @build_times = []
+        @pipeline_times = []
         @project = project
 
         collect
       end
 
       def collect
-        query = project.builds
-          .where("? > #{Ci::Build.table_name}.created_at AND #{Ci::Build.table_name}.created_at > ?", @to, @from)
+        query = project.pipelines
+          .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from)
 
         totals_count  = grouped_count(query)
         success_count = grouped_count(query.success)
@@ -101,14 +101,14 @@ module Ci
       end
     end
 
-    class BuildTime < Chart
+    class PipelineTime < Chart
       def collect
         commits = project.pipelines.last(30)
 
         commits.each do |commit|
           @labels << commit.short_sha
           duration = commit.duration || 0
-          @build_times << (duration / 60)
+          @pipeline_times << (duration / 60)
         end
       end
     end
diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb
index fb6cc398307..51cbfd2a848 100644
--- a/spec/lib/ci/charts_spec.rb
+++ b/spec/lib/ci/charts_spec.rb
@@ -1,21 +1,21 @@
 require 'spec_helper'
 
 describe Ci::Charts, lib: true do
-  context "build_times" do
+  context "pipeline_times" do
     let(:project) { create(:empty_project) }
-    let(:chart) { Ci::Charts::BuildTime.new(project) }
+    let(:chart) { Ci::Charts::PipelineTime.new(project) }
 
-    subject { chart.build_times }
+    subject { chart.pipeline_times }
 
     before do
       create(:ci_empty_pipeline, project: project, duration: 120)
     end
 
-    it 'returns build times in minutes' do
+    it 'returns pipeline times in minutes' do
       is_expected.to contain_exactly(2)
     end
 
-    it 'handles nil build times' do
+    it 'handles nil pipeline times' do
       create(:ci_empty_pipeline, project: project, duration: nil)
 
       is_expected.to contain_exactly(2, 0)