Merge branch '271408_reschedule_latest_pipeline_id_population_with_logging' into 'master'

Re-schedule `latest_pipeline_id` population with logger

See merge request gitlab-org/gitlab!66050

db/post_migrate/20210706142819_re_schedule_latest_pipeline_id_population.rb

 # frozen_string_literal: true
 
 class ReScheduleLatestPipelineIdPopulation < ActiveRecord::Migration[6.1]
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-  DELAY_INTERVAL = 2.minutes.to_i
-  BATCH_SIZE = 100
-  MIGRATION = 'PopulateLatestPipelineIds'
-
-  disable_ddl_transaction!
-
-  def up
-    return unless Gitlab.ee?
-
-    queue_background_migration_jobs_by_range_at_intervals(
-      Gitlab::BackgroundMigration::PopulateLatestPipelineIds::ProjectSetting.has_vulnerabilities_without_latest_pipeline_set,
-      MIGRATION,
-      DELAY_INTERVAL,
-      batch_size: BATCH_SIZE,
-      primary_column_name: 'project_id'
-    )
-  end
-
-  def down
-    # no-op
+  def change
+    # no-op: This migration has been marked as no-op and replaced by
+    # `ReScheduleLatestPipelineIdPopulationWithLogging` as we've found some problems.
+    # For more information: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66050
   end
 end

db/post_migrate/20210713075117_re_schedule_latest_pipeline_id_population_with_logging.rb

+# frozen_string_literal: true
+
+class ReScheduleLatestPipelineIdPopulationWithLogging < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  DELAY_INTERVAL = 2.minutes.to_i
+  BATCH_SIZE = 100
+  MIGRATION = 'PopulateLatestPipelineIds'
+
+  disable_ddl_transaction!
+
+  def up
+    return unless Gitlab.ee?
+
+    Gitlab::BackgroundMigration.steal(MIGRATION)
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      Gitlab::BackgroundMigration::PopulateLatestPipelineIds::ProjectSetting.has_vulnerabilities_without_latest_pipeline_set,
+      MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      primary_column_name: 'project_id'
+    )
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20210713075117

+9a8cbcf6ddbdd4379320ed747faed9beb0c2104eb89e61b349432b1f0346a4b5
\ No newline at end of file

ee/lib/ee/gitlab/background_migration/populate_latest_pipeline_ids.rb

@@ -6,6 +6,14 @@ module EE
       module PopulateLatestPipelineIds
         extend ::Gitlab::Utils::Override
 
+        module LogUtils
+          MIGRATOR = 'PopulateLatestPipelineIds'
+
+          def log_info(log_attributes)
+            ::Gitlab::BackgroundMigration::Logger.info(log_attributes.merge(migrator: MIGRATOR))
+          end
+        end
+
         module Routable
           extend ActiveSupport::Concern
 
@@ -26,30 +34,16 @@ module EE
           end
         end
 
-        module Visibility
-          PUBLIC_LEVEL = 20
-
-          def public?
-            visibility_level == PUBLIC_LEVEL
-          end
-        end
-
         class Namespace < ActiveRecord::Base
           include Routable
-          include Visibility
 
           self.table_name = 'namespaces'
+          self.inheritance_column = :_type_disabled
 
           belongs_to :parent, class_name: '::EE::Gitlab::BackgroundMigration::PopulateLatestPipelineIds::Namespace'
 
-          def self.find_sti_class(type_name)
-            super("EE::Gitlab::BackgroundMigration::PopulateLatestPipelineIds::#{type_name}")
-          end
-        end
-
-        class Group < Namespace
           def self.polymorphic_name
-            'Group'
+            'Namespace'
           end
         end
 
@@ -57,10 +51,9 @@ module EE
           self.table_name = 'routes'
         end
 
-        class Project < ActiveRecord::Base
+        class Project < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
           include Routable
-          include Visibility
-          include ::Gitlab::Utils::StrongMemoize
+          include LogUtils
 
           self.table_name = 'projects'
 
@@ -114,7 +107,7 @@ module EE
             LIMIT 1
           SQL
 
-          belongs_to :namespace
+          belongs_to :namespace, class_name: '::EE::Gitlab::BackgroundMigration::PopulateLatestPipelineIds::Namespace'
           alias_method :parent, :namespace
 
           has_many :all_pipelines, class_name: '::EE::Gitlab::BackgroundMigration::PopulateLatestPipelineIds::Pipeline'
@@ -132,9 +125,15 @@ module EE
           end
 
           def stats_tuple
-            return unless latest_pipeline_id
+            unless latest_pipeline_id
+              log_info(message: 'No latest_pipeline_id found', project_id: id)
+
+              return
+            end
 
-            [id, DEFAULT_LETTER_GRADE, latest_pipeline_id, quoted_time, quoted_time].join(', ').then { |s| "(#{s})" }
+            log_info(message: 'latest_pipeline_id found', project_id: id)
+
+            [id, DEFAULT_LETTER_GRADE, latest_pipeline_id, current_time, current_time].join(', ').then { |s| "(#{s})" }
           rescue StandardError => e
             ::Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
 
@@ -145,12 +144,12 @@ module EE
 
           delegate :connection, to: :'self.class', private: true
 
-          def quoted_time
-            @quoted_time ||= connection.quote(Time.zone.now)
+          def current_time
+            @current_time ||= connection.quote(Time.zone.now)
           end
 
           def latest_pipeline_id
-            strong_memoize(:latest_pipeline_id) { pipeline_with_reports&.fetch('id') }
+            @latest_pipeline_id ||= pipeline_with_reports&.fetch('id')
           end
 
           def pipeline_with_reports
@@ -158,10 +157,11 @@ module EE
           end
 
           def pipeline_with_reports_sql
+            log_info(message: 'Pipeline with reports SQL requested', project_id: id, ref: default_branch)
+
             format(LATEST_PIPELINE_WITH_REPORTS_SQL, project_id: id, ref: connection.quote(default_branch), file_types: FILE_TYPES.join(', '))
           end
 
-          ### Default branch related logic
           def default_branch
             @default_branch ||= repository.root_ref || default_branch_from_preferences
           end
@@ -277,6 +277,8 @@ module EE
         end
 
         class VulnerabilityStatistic < ActiveRecord::Base
+          extend LogUtils
+
           self.table_name = 'vulnerability_statistics'
 
           UPSERT_SQL = <<~SQL
@@ -294,7 +296,9 @@ module EE
             def update_latest_pipeline_ids_for(projects)
               upsert_tuples = projects.map(&:stats_tuple).compact
 
-              run_upsert(upsert_tuples) if upsert_tuples.present?
+              return log_info(message: 'No projects to update') unless upsert_tuples.present?
+
+              run_upsert(upsert_tuples)
             end
 
             private
@@ -303,13 +307,21 @@ module EE
               upsert_sql = format(UPSERT_SQL, insert_tuples: tuples.join(', '))
 
               connection.execute(upsert_sql)
+              log_info(message: 'Update query has been executed')
             end
           end
         end
 
+        include LogUtils
+
+        override :perform
         def perform(start_id, end_id)
+          log_info(message: 'Migration started', start_id: start_id, end_id: end_id)
+
           projects = Project.by_range(start_id, end_id)
 
+          log_info(message: 'Projects fetched', count: projects.length)
+
           VulnerabilityStatistic.update_latest_pipeline_ids_for(projects)
         end
       end

ee/spec/lib/ee/gitlab/background_migration/populate_latest_pipeline_ids_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe Gitlab::BackgroundMigration::PopulateLatestPipelineIds do
     }
   end
 
-  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
+  let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org', type: 'Group') }
   let!(:project_1) { projects.create!(namespace_id: namespace.id, name: 'Foo 1') }
   let!(:project_2) { projects.create!(namespace_id: namespace.id, name: 'Foo 2') }
   let!(:project_3) { projects.create!(namespace_id: namespace.id, name: 'Foo 3') }
@@ -70,7 +70,8 @@ RSpec.describe Gitlab::BackgroundMigration::PopulateLatestPipelineIds do
     subject(:populate_latest_pipeline_ids) { migrator.perform(project_1.id, project_6.id) }
 
     before do
-      allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+      allow(::Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+      allow(::Gitlab::BackgroundMigration::Logger).to receive(:info)
 
       # Raise a RuntimeError while retreiving the `pipeline_with_reports` for the `project_6`
       allow_next_found_instance_of(described_class::Project) do |project_instance|
@@ -89,7 +90,8 @@ RSpec.describe Gitlab::BackgroundMigration::PopulateLatestPipelineIds do
                                              .and change { vulnerability_statistics.find_by(project_id: project_1.id)&.latest_pipeline_id }.from(nil).to(project_1_latest_pipeline.id)
                                              .and not_change { project_2_stats.reload.latest_pipeline_id }.from(project_2_pipeline.id)
 
-      expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception).once
+      expect(::Gitlab::ErrorTracking).to have_received(:track_and_raise_for_dev_exception).once
+      expect(::Gitlab::BackgroundMigration::Logger).to have_received(:info).exactly(9).times
     end
   end
 

spec/migrations/re_schedule_latest_pipeline_id_population_spec.rb → spec/migrations/re_schedule_latest_pipeline_id_population_with_logging_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 require_migration!
 
-RSpec.describe ReScheduleLatestPipelineIdPopulation do
+RSpec.describe ReScheduleLatestPipelineIdPopulationWithLogging do
   let(:namespaces) { table(:namespaces) }
   let(:pipelines) { table(:ci_pipelines) }
   let(:projects) { table(:projects) }