Merge branch '218563-rename-stuck-import-jobs' into 'master'

Rename StuckImportJobsWorker to StuckProjectImportJobsWorker

Closes #218563

See merge request gitlab-org/gitlab!32965

app/models/concerns/import_state/sidekiq_job_tracker.rb

@@ -11,7 +11,7 @@ module ImportState
       # Refreshes the expiration time of the associated import job ID.
       #
       # This method can be used by asynchronous importers to refresh the status,
-      # preventing the StuckImportJobsWorker from marking the import as failed.
+      # preventing the Gitlab::Import::StuckProjectImportJobsWorker from marking the import as failed.
       def refresh_jid_expiration
         return unless jid
 

app/workers/all_queues.yml

@@ -147,6 +147,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: cronjob:import_stuck_project_import_jobs
+  :feature_category: :importers
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :cpu
+  :weight: 1
+  :idempotent: 
+  :tags: []
 - :name: cronjob:issue_due_scheduler
   :feature_category: :issue_tracking
   :has_external_dependencies: 

app/workers/gitlab/import/stuck_project_import_jobs_worker.rb

+# frozen_string_literal: true
+module Gitlab
+  module Import
+    class StuckProjectImportJobsWorker # rubocop:disable Scalability/IdempotentWorker
+      include Gitlab::Import::StuckImportJob
+
+      private
+
+      def track_metrics(with_jid_count, without_jid_count)
+        Gitlab::Metrics.add_event(
+          :stuck_import_jobs,
+          projects_without_jid_count: without_jid_count,
+          projects_with_jid_count: with_jid_count
+        )
+      end
+
+      def enqueued_import_states
+        ProjectImportState.with_status([:scheduled, :started])
+      end
+    end
+  end
+end

app/workers/gitlab/phabricator_import/base_worker.rb

@@ -13,7 +13,7 @@
 # - It keeps track of the jobs so we know how many jobs are running for the
 #   project
 # - It refreshes the import jid, so it doesn't get cleaned up by the
-#   `StuckImportJobsWorker`
+#   `Gitlab::Import::StuckProjectImportJobsWorker`
 # - It marks the import as failed if a job failed to many times
 # - It marks the import as finished when all remaining jobs are done
 module Gitlab

config/initializers/1_settings.rb

@@ -451,9 +451,9 @@ Settings.cron_jobs['trending_projects_worker']['job_class'] = 'TrendingProjectsW
 Settings.cron_jobs['remove_unreferenced_lfs_objects_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['cron'] ||= '20 0 * * *'
 Settings.cron_jobs['remove_unreferenced_lfs_objects_worker']['job_class'] = 'RemoveUnreferencedLfsObjectsWorker'
-Settings.cron_jobs['stuck_import_jobs_worker'] ||= Settingslogic.new({})
-Settings.cron_jobs['stuck_import_jobs_worker']['cron'] ||= '15 * * * *'
-Settings.cron_jobs['stuck_import_jobs_worker']['job_class'] = 'StuckImportJobsWorker'
+Settings.cron_jobs['import_stuck_project_import_jobs'] ||= Settingslogic.new({})
+Settings.cron_jobs['import_stuck_project_import_jobs']['cron'] ||= '15 * * * *'
+Settings.cron_jobs['import_stuck_project_import_jobs']['job_class'] = 'Gitlab::Import::StuckProjectImportJobsWorker'
 Settings.cron_jobs['jira_import_stuck_jira_import_jobs'] ||= Settingslogic.new({})
 Settings.cron_jobs['jira_import_stuck_jira_import_jobs']['cron'] ||= '* 0/15 * * *'
 Settings.cron_jobs['jira_import_stuck_jira_import_jobs']['job_class'] = 'Gitlab::JiraImport::StuckJiraImportJobsWorker'

db/post_migrate/20200525144525_migrate_stuck_import_jobs_queue_to_stuck_project_import_jobs.rb

+# frozen_string_literal: true
+
+class MigrateStuckImportJobsQueueToStuckProjectImportJobs < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sidekiq_queue_migrate 'cronjob:stuck_import_jobs', to: 'cronjob:import_stuck_project_import_jobs'
+  end
+
+  def down
+    sidekiq_queue_migrate 'cronjob:import_stuck_project_import_jobs', to: 'cronjob:stuck_import_jobs'
+  end
+end

db/structure.sql

@@ -13781,6 +13781,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200522235146
 20200525114553
 20200525121014
+20200525144525
 20200526000407
 20200526013844
 20200526120714

doc/development/github_importer.md

@@ -121,12 +121,12 @@ also reduce pressure on the system as a whole.
 
 ## Refreshing import JIDs
 
-GitLab includes a worker called `StuckImportJobsWorker` that will periodically
-run and mark project imports as failed if they have been running for more than
-15 hours. For GitHub projects, this poses a bit of a problem: importing large
-projects could take several hours depending on how often we hit the GitHub rate
-limit (more on this below), but we don't want `StuckImportJobsWorker` to mark
-our import as failed because of this.
+GitLab includes a worker called `Gitlab::Import::StuckProjectImportJobsWorker`
+that will periodically run and mark project imports as failed if they have been
+running for more than 15 hours. For GitHub projects, this poses a bit of a
+problem: importing large projects could take several hours depending on how
+often we hit the GitHub rate limit (more on this below), but we don't want
+`Gitlab::Import::StuckProjectImportJobsWorker` to mark our import as failed because of this.
 
 To prevent this from happening we periodically refresh the expiration time of
 the import process. This works by storing the JID of the import job in the

doc/development/import_export.md

@@ -44,19 +44,34 @@ WARN: Work still in progress <struct with JID>
 
 ### Timeouts
 
-Timeout errors occur due to the `StuckImportJobsWorker` marking the process as failed:
+Timeout errors occur due to the `Gitlab::Import::StuckProjectImportJobsWorker` marking the process as failed:
 
 ```ruby
-class StuckImportJobsWorker
-  include ApplicationWorker
-  include CronjobQueue
+module Gitlab
+  module Import
+    class StuckProjectImportJobsWorker
+      include Gitlab::Import::StuckImportJob
+      # ...
+    end
+  end
+end
 
+module Gitlab
+  module Import
+    module StuckImportJob
+      # ...
       IMPORT_JOBS_EXPIRATION = 15.hours.to_i
-
+      # ...
       def perform
-    imports_without_jid_count = mark_imports_without_jid_as_failed!
-    imports_with_jid_count = mark_imports_with_jid_as_failed!
-    ...
+        stuck_imports_without_jid_count = mark_imports_without_jid_as_failed!
+        stuck_imports_with_jid_count = mark_imports_with_jid_as_failed!
+
+        track_metrics(stuck_imports_with_jid_count, stuck_imports_without_jid_count)
+      end
+      # ...
+    end
+  end
+end
 ```
 
 ```shell

ee/spec/lib/gitlab/sidekiq_config_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe Gitlab::SidekiqConfig do
     it 'expands queue namespaces to concrete queue names' do
       queues = described_class.expand_queues(%w[cronjob])
 
-      expect(queues).to include('cronjob:stuck_import_jobs')
+      expect(queues).to include('cronjob:import_stuck_project_import_jobs')
       expect(queues).to include('cronjob:jira_import_stuck_jira_import_jobs')
       expect(queues).to include('cronjob:stuck_merge_jobs')
     end

lib/gitlab/import/set_async_jid.rb

@@ -2,8 +2,8 @@
 
 # The original import JID is the JID of the RepositoryImportWorker job,
 # which will be removed once that job completes. Reusing that JID could
-# result in StuckImportJobsWorker marking the job as stuck before we get
-# to running Stage::ImportRepositoryWorker.
+# result in Gitlab::Import::StuckProjectImportJobsWorker marking the job
+# as stuck before we get to running Stage::ImportRepositoryWorker.
 #
 # We work around this by setting the JID to a custom generated one, then
 # refreshing it in the various stages whenever necessary.
@@ -13,8 +13,7 @@ module Gitlab
       def self.set_jid(import_state)
         jid = generate_jid(import_state)
 
-        Gitlab::SidekiqStatus
-          .set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
+        Gitlab::SidekiqStatus.set(jid, Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
 
         import_state.update_column(:jid, jid)
       end

spec/lib/gitlab/sidekiq_config/cli_methods_spec.rb

@@ -77,7 +77,7 @@ describe Gitlab::SidekiqConfig::CliMethods do
   describe '.expand_queues' do
     let(:worker_queues) do
       [
-        'cronjob:stuck_import_jobs',
+        'cronjob:import_stuck_project_import_jobs',
         'cronjob:jira_import_stuck_jira_import_jobs',
         'cronjob:stuck_merge_jobs',
         'post_receive'
@@ -93,12 +93,22 @@ describe Gitlab::SidekiqConfig::CliMethods do
       allow(described_class).to receive(:worker_queues).and_return(worker_queues)
 
       expect(described_class.expand_queues(['cronjob']))
-        .to contain_exactly('cronjob', 'cronjob:stuck_import_jobs', 'cronjob:jira_import_stuck_jira_import_jobs', 'cronjob:stuck_merge_jobs')
+        .to contain_exactly(
+          'cronjob',
+          'cronjob:import_stuck_project_import_jobs',
+          'cronjob:jira_import_stuck_jira_import_jobs',
+          'cronjob:stuck_merge_jobs'
+        )
     end
 
     it 'expands queue namespaces to concrete queue names' do
       expect(described_class.expand_queues(['cronjob'], worker_queues))
-        .to contain_exactly('cronjob', 'cronjob:stuck_import_jobs', 'cronjob:jira_import_stuck_jira_import_jobs', 'cronjob:stuck_merge_jobs')
+        .to contain_exactly(
+          'cronjob',
+          'cronjob:import_stuck_project_import_jobs',
+          'cronjob:jira_import_stuck_jira_import_jobs',
+          'cronjob:stuck_merge_jobs'
+        )
     end
 
     it 'lets concrete queue names pass through' do

spec/lib/gitlab/sidekiq_config_spec.rb

@@ -18,7 +18,7 @@ describe Gitlab::SidekiqConfig do
 
       expect(queues).to include('post_receive')
       expect(queues).to include('merge')
-      expect(queues).to include('cronjob:stuck_import_jobs')
+      expect(queues).to include('cronjob:import_stuck_project_import_jobs')
       expect(queues).to include('cronjob:jira_import_stuck_jira_import_jobs')
       expect(queues).to include('mailers')
       expect(queues).to include('default')

spec/lib/gitlab/sidekiq_versioning/manager_spec.rb

@@ -16,7 +16,7 @@ describe Gitlab::SidekiqVersioning::Manager do
       expect(queues).to include('post_receive')
       expect(queues).to include('repository_fork')
       expect(queues).to include('cronjob')
-      expect(queues).to include('cronjob:stuck_import_jobs')
+      expect(queues).to include('cronjob:import_stuck_project_import_jobs')
       expect(queues).to include('cronjob:jira_import_stuck_jira_import_jobs')
       expect(queues).to include('cronjob:stuck_merge_jobs')
       expect(queues).to include('unknown')

spec/workers/gitlab/import/stuck_project_import_jobs_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Import::StuckProjectImportJobsWorker do
+  let(:worker) { described_class.new }
+
+  describe 'with scheduled import_status' do
+    it_behaves_like 'stuck import job detection' do
+      let(:import_state) { create(:project, :import_scheduled).import_state }
+
+      before do
+        import_state.update(jid: '123')
+      end
+    end
+  end
+
+  describe 'with started import_status' do
+    it_behaves_like 'stuck import job detection' do
+      let(:import_state) { create(:project, :import_started).import_state }
+
+      before do
+        import_state.update(jid: '123')
+      end
+    end
+  end
+end