Merge branch '31528-fix-pipelines-charts-timeout' into 'master'

Fix pipelines chart query timeout

See merge request gitlab-org/gitlab!47069

changelogs/unreleased/31528-fix-pipelines-charts-timeout.yml

+---
+title: Fix pipelines chart query timeout
+merge_request: 47069
+author:
+type: performance

db/migrate/20201106134139_add_pipelines_created_index.rb

+# frozen_string_literal: true
+
+class AddPipelinesCreatedIndex < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  INDEX_NAME = :index_ci_pipelines_on_project_id_and_status_and_created_at
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_pipelines, [:project_id, :status, :created_at], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_pipelines, INDEX_NAME
+  end
+end

db/schema_migrations/20201106134139

+9b1008df64741ad313ddf51969c18d609cd01a7255563fe0395d2bbf4d288e30
\ No newline at end of file

db/structure.sql

@@ -20401,6 +20401,8 @@ CREATE INDEX index_ci_pipelines_on_project_id_and_source ON ci_pipelines USING b
 
 CREATE INDEX index_ci_pipelines_on_project_id_and_status_and_config_source ON ci_pipelines USING btree (project_id, status, config_source);
 
+CREATE INDEX index_ci_pipelines_on_project_id_and_status_and_created_at ON ci_pipelines USING btree (project_id, status, created_at);
+
 CREATE INDEX index_ci_pipelines_on_project_id_and_status_and_updated_at ON ci_pipelines USING btree (project_id, status, updated_at);
 
 CREATE INDEX index_ci_pipelines_on_project_id_and_user_id_and_status_and_ref ON ci_pipelines USING btree (project_id, user_id, status, ref) WHERE (source <> 12);

lib/gitlab/ci/charts.rb

@@ -3,38 +3,8 @@
 module Gitlab
   module Ci
     module Charts
-      module DailyInterval
-        # rubocop: disable CodeReuse/ActiveRecord
-        def grouped_count(query)
-          query
-            .group("DATE(#{::Ci::Pipeline.table_name}.created_at)")
-            .count(:created_at)
-            .transform_keys { |date| date.strftime(@format) } # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        end
-        # rubocop: enable CodeReuse/ActiveRecord
-
-        def interval_step
-          @interval_step ||= 1.day
-        end
-      end
-
-      module MonthlyInterval
-        # rubocop: disable CodeReuse/ActiveRecord
-        def grouped_count(query)
-          query
-            .group("to_char(#{::Ci::Pipeline.table_name}.created_at, '01 Month YYYY')")
-            .count(:created_at)
-            .transform_keys(&:squish)
-        end
-        # rubocop: enable CodeReuse/ActiveRecord
-
-        def interval_step
-          @interval_step ||= 1.month
-        end
-      end
-
       class Chart
-        attr_reader :labels, :total, :success, :project, :pipeline_times
+        attr_reader :from, :to, :labels, :total, :success, :project, :pipeline_times
 
         def initialize(project)
           @labels = []
@@ -46,48 +16,59 @@ module Gitlab
           collect
         end
 
+        private
+
+        attr_reader :interval
+
         # rubocop: disable CodeReuse/ActiveRecord
         def collect
           query = project.all_pipelines
-            .where("? > #{::Ci::Pipeline.table_name}.created_at AND #{::Ci::Pipeline.table_name}.created_at > ?", @to, @from) # rubocop:disable GitlabSecurity/SqlInjection
+            .where(::Ci::Pipeline.arel_table['created_at'].gteq(@from))
+            .where(::Ci::Pipeline.arel_table['created_at'].lteq(@to))
 
           totals_count  = grouped_count(query)
           success_count = grouped_count(query.success)
 
           current = @from
-          while current < @to
-            label = current.strftime(@format)
-
-            @labels  << label
-            @total   << (totals_count[label] || 0)
-            @success << (success_count[label] || 0)
+          while current <= @to
+            @labels  << current.strftime(@format)
+            @total   << (totals_count[current] || 0)
+            @success << (success_count[current] || 0)
 
             current += interval_step
           end
         end
         # rubocop: enable CodeReuse/ActiveRecord
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def grouped_count(query)
+          query
+            .group("date_trunc('#{interval}', #{::Ci::Pipeline.table_name}.created_at)")
+            .count(:created_at)
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def interval_step
+          @interval_step ||= 1.public_send(interval) # rubocop: disable GitlabSecurity/PublicSend
+        end
       end
 
       class YearChart < Chart
-        include MonthlyInterval
-        attr_reader :to, :from
-
         def initialize(*)
           @to     = Date.today.end_of_month.end_of_day
-          @from   = @to.years_ago(1).beginning_of_month.beginning_of_day
-          @format = '%d %B %Y'
+          @from   = (@to - 1.year).beginning_of_month.beginning_of_day
+          @interval = :month
+          @format = '%B %Y'
 
           super
         end
       end
 
       class MonthChart < Chart
-        include DailyInterval
-        attr_reader :to, :from
-
         def initialize(*)
           @to     = Date.today.end_of_day
-          @from   = 1.month.ago.beginning_of_day
+          @from   = (@to - 1.month).beginning_of_day
+          @interval = :day
           @format = '%d %B'
 
           super
@@ -95,12 +76,10 @@ module Gitlab
       end
 
       class WeekChart < Chart
-        include DailyInterval
-        attr_reader :to, :from
-
         def initialize(*)
           @to     = Date.today.end_of_day
-          @from   = 1.week.ago.beginning_of_day
+          @from   = (@to - 1.week).beginning_of_day
+          @interval = :day
           @format = '%d %B'
 
           super

spec/lib/gitlab/ci/charts_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Charts do
-  context "yearchart" do
+  context 'yearchart' do
     let(:project) { create(:project) }
     let(:chart) { Gitlab::Ci::Charts::YearChart.new(project) }
 
@@ -16,9 +16,13 @@ RSpec.describe Gitlab::Ci::Charts do
     it 'starts at the beginning of the current year' do
       expect(chart.from).to eq(chart.to.years_ago(1).beginning_of_month.beginning_of_day)
     end
+
+    it 'uses %B %Y as labels format' do
+      expect(chart.labels).to include(chart.from.strftime('%B %Y'))
+    end
   end
 
-  context "monthchart" do
+  context 'monthchart' do
     let(:project) { create(:project) }
     let(:chart) { Gitlab::Ci::Charts::MonthChart.new(project) }
 
@@ -31,9 +35,13 @@ RSpec.describe Gitlab::Ci::Charts do
     it 'starts one month ago' do
       expect(chart.from).to eq(1.month.ago.beginning_of_day)
     end
+
+    it 'uses %d %B as labels format' do
+      expect(chart.labels).to include(chart.from.strftime('%d %B'))
+    end
   end
 
-  context "weekchart" do
+  context 'weekchart' do
     let(:project) { create(:project) }
     let(:chart) { Gitlab::Ci::Charts::WeekChart.new(project) }
 
@@ -46,9 +54,13 @@ RSpec.describe Gitlab::Ci::Charts do
     it 'starts one week ago' do
       expect(chart.from).to eq(1.week.ago.beginning_of_day)
     end
+
+    it 'uses %d %B as labels format' do
+      expect(chart.labels).to include(chart.from.strftime('%d %B'))
+    end
   end
 
-  context "pipeline_times" do
+  context 'pipeline_times' do
     let(:project) { create(:project) }
     let(:chart) { Gitlab::Ci::Charts::PipelineTime.new(project) }