Merge branch 'dm-stuck-import-jobs-verify' into 'master'
Verify project import status again before marking as failed Closes #43539 See merge request gitlab-org/gitlab-ce!17319
This commit is contained in:
commit
52c56400f1
|
@ -16,43 +16,41 @@ class StuckImportJobsWorker
|
|||
private
|
||||
|
||||
def mark_projects_without_jid_as_failed!
|
||||
started_projects_without_jid.each do |project|
|
||||
enqueued_projects_without_jid.each do |project|
|
||||
project.mark_import_as_failed(error_message)
|
||||
end.count
|
||||
end
|
||||
|
||||
def mark_projects_with_jid_as_failed!
|
||||
completed_jids_count = 0
|
||||
|
||||
started_projects_with_jid.find_in_batches(batch_size: 500) do |group|
|
||||
jids = group.map(&:import_jid)
|
||||
jids_and_ids = enqueued_projects_with_jid.pluck(:import_jid, :id).to_h
|
||||
|
||||
# Find the jobs that aren't currently running or that exceeded the threshold.
|
||||
completed_jids = Gitlab::SidekiqStatus.completed_jids(jids).to_set
|
||||
completed_jids = Gitlab::SidekiqStatus.completed_jids(jids_and_ids.keys)
|
||||
return unless completed_jids.any?
|
||||
|
||||
if completed_jids.any?
|
||||
completed_jids_count += completed_jids.count
|
||||
group.each do |project|
|
||||
project.mark_import_as_failed(error_message) if completed_jids.include?(project.import_jid)
|
||||
completed_project_ids = jids_and_ids.values_at(*completed_jids)
|
||||
|
||||
# We select the projects again, because they may have transitioned from
|
||||
# scheduled/started to finished/failed while we were looking up their Sidekiq status.
|
||||
completed_projects = enqueued_projects_with_jid.where(id: completed_project_ids)
|
||||
|
||||
Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_projects.map(&:import_jid).join(', ')}")
|
||||
|
||||
completed_projects.each do |project|
|
||||
project.mark_import_as_failed(error_message)
|
||||
end.count
|
||||
end
|
||||
|
||||
Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_jids.to_a.join(', ')}")
|
||||
end
|
||||
def enqueued_projects
|
||||
Project.with_import_status(:scheduled, :started)
|
||||
end
|
||||
|
||||
completed_jids_count
|
||||
def enqueued_projects_with_jid
|
||||
enqueued_projects.where.not(import_jid: nil)
|
||||
end
|
||||
|
||||
def started_projects
|
||||
Project.with_import_status(:started)
|
||||
end
|
||||
|
||||
def started_projects_with_jid
|
||||
started_projects.where.not(import_jid: nil)
|
||||
end
|
||||
|
||||
def started_projects_without_jid
|
||||
started_projects.where(import_jid: nil)
|
||||
def enqueued_projects_without_jid
|
||||
enqueued_projects.where(import_jid: nil)
|
||||
end
|
||||
|
||||
def error_message
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Verify project import status again before marking as failed
|
||||
merge_request:
|
||||
author:
|
||||
type: fixed
|
|
@ -2,35 +2,59 @@ require 'spec_helper'
|
|||
|
||||
describe StuckImportJobsWorker do
|
||||
let(:worker) { described_class.new }
|
||||
let(:exclusive_lease_uuid) { SecureRandom.uuid }
|
||||
|
||||
shared_examples 'project import job detection' do
|
||||
context 'when the job has completed' do
|
||||
context 'when the import status was already updated' do
|
||||
before do
|
||||
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).and_return(exclusive_lease_uuid)
|
||||
allow(Gitlab::SidekiqStatus).to receive(:completed_jids) do
|
||||
project.import_start
|
||||
project.import_finish
|
||||
|
||||
[project.import_jid]
|
||||
end
|
||||
end
|
||||
|
||||
it 'does not mark the project as failed' do
|
||||
worker.perform
|
||||
|
||||
expect(project.reload.import_status).to eq('finished')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the import status was not updated' do
|
||||
before do
|
||||
allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([project.import_jid])
|
||||
end
|
||||
|
||||
it 'marks the project as failed' do
|
||||
worker.perform
|
||||
|
||||
expect(project.reload.import_status).to eq('failed')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the job is still in Sidekiq' do
|
||||
before do
|
||||
allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([])
|
||||
end
|
||||
|
||||
it 'does not mark the project as failed' do
|
||||
expect { worker.perform }.not_to change { project.reload.import_status }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'with scheduled import_status' do
|
||||
it_behaves_like 'project import job detection' do
|
||||
let(:project) { create(:project, :import_scheduled, import_jid: '123') }
|
||||
end
|
||||
end
|
||||
|
||||
describe 'with started import_status' do
|
||||
it_behaves_like 'project import job detection' do
|
||||
let(:project) { create(:project, :import_started, import_jid: '123') }
|
||||
|
||||
describe 'long running import' do
|
||||
it 'marks the project as failed' do
|
||||
allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(['123'])
|
||||
|
||||
expect { worker.perform }.to change { project.reload.import_status }.to('failed')
|
||||
end
|
||||
end
|
||||
|
||||
describe 'running import' do
|
||||
it 'does not mark the project as failed' do
|
||||
allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([])
|
||||
|
||||
expect { worker.perform }.not_to change { project.reload.import_status }
|
||||
end
|
||||
|
||||
describe 'import without import_jid' do
|
||||
it 'marks the project as failed' do
|
||||
expect { worker.perform }.to change { project.reload.import_status }.to('failed')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue