diff --git a/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb b/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb index c10dac35812f..39f3e79c38cf 100644 --- a/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb +++ b/app/workers/project_identifiers/convert_instance_to_semantic_ids_job.rb @@ -35,7 +35,7 @@ class ProjectIdentifiers::ConvertInstanceToSemanticIdsJob < ApplicationJob queue_with_priority :above_normal def perform - GoodJob::Batch.enqueue(on_success: ProjectIdentifiers::FinishSemanticConversionJob) do + GoodJob::Batch.enqueue(on_finish: ProjectIdentifiers::FinishSemanticConversionJob) do ProjectIdentifiers::PendingProjectsFinder.project_ids.each do |project_id| ProjectIdentifiers::ConvertProjectToSemanticIdsJob.perform_later(project_id) end diff --git a/app/workers/project_identifiers/finish_semantic_conversion_job.rb b/app/workers/project_identifiers/finish_semantic_conversion_job.rb index dc14f3c5868c..f5e9777832fa 100644 --- a/app/workers/project_identifiers/finish_semantic_conversion_job.rb +++ b/app/workers/project_identifiers/finish_semantic_conversion_job.rb @@ -28,30 +28,41 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# GoodJob on_success callback invoked after a ConvertInstanceToSemanticIdsJob -# batch completes. Performs up to MAX_SWEEPS synchronous sweeps to catch any -# projects created or modified during the batch run, then enables semantic mode. -# If projects still remain after MAX_SWEEPS sweeps a RuntimeError is raised to -# abort the job (leaving the setting as classic); the instance is either under +# GoodJob on_finish callback invoked after a ConvertInstanceToSemanticIdsJob batch +# completes (regardless of per-project job failures or discards). Performs up to +# MAX_SWEEPS synchronous sweeps to catch any projects created or modified during +# the batch run, then enables semantic mode. If projects still remain after +# MAX_SWEEPS sweeps the instance reverts to classic mode; it is either under # active load or there is a code bug. class ProjectIdentifiers::FinishSemanticConversionJob < ApplicationJob queue_with_priority :high MAX_SWEEPS = 5 - def perform(*) + class ConversionFailed < StandardError; end + + def perform(batch, *) + raise ConversionFailed, "Batch had failures" if batch.discarded? + corrective_sweep set_semantic_mode! + rescue ConversionFailed => e + revert_to_classic!(reason: e.message) end private + def revert_to_classic!(reason:) + Rails.logger.error "[#{self.class.name}] #{reason} - reverting to classic mode." + ProjectIdentifiers::RevertInstanceToClassicIdsJob.perform_later + end + def set_semantic_mode! result = Settings::UpdateService .new(user: User.system) .call(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) - raise "[FinishSemanticConversionJob] Failed to enable semantic mode: #{result.message}" unless result.success? + raise ConversionFailed, "Failed to enable semantic mode: #{result.message}" unless result.success? end def corrective_sweep @@ -69,10 +80,8 @@ def corrective_sweep return if pending_project_ids.empty? - message = "[FinishSemanticConversionJob] Giving up after #{MAX_SWEEPS} sweeps — " \ - "projects still remain pending. The instance may be under active load or there is a bug." - Rails.logger.warn message - raise message + raise ConversionFailed, "Giving up after #{MAX_SWEEPS} sweeps — " \ + "projects still remain pending. The instance may be under active load or there is a bug." end def pending_project_ids diff --git a/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb b/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb index ab43f67bd50e..0185d2ef423d 100644 --- a/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb +++ b/spec/workers/project_identifiers/convert_instance_to_semantic_ids_job_spec.rb @@ -46,11 +46,11 @@ expect(GoodJob::Job.where(job_class: ProjectIdentifiers::ConvertProjectToSemanticIdsJob.name).count).to eq(2) end - it "sets FinishSemanticConversionJob as the on_success callback" do + it "sets FinishSemanticConversionJob as the on_finish callback" do allow(GoodJob::Batch).to receive(:enqueue).and_call_original job.perform expect(GoodJob::Batch).to have_received(:enqueue) - .with(hash_including(on_success: ProjectIdentifiers::FinishSemanticConversionJob)) + .with(hash_including(on_finish: ProjectIdentifiers::FinishSemanticConversionJob)) end end diff --git a/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb b/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb index 673041901abc..415c7cf4a6e8 100644 --- a/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb +++ b/spec/workers/project_identifiers/finish_semantic_conversion_job_spec.rb @@ -33,10 +33,12 @@ RSpec.describe ProjectIdentifiers::FinishSemanticConversionJob do subject(:job) { described_class.new } + let(:batch) { instance_double(GoodJob::Batch, discarded?: false) } let(:update_service) { instance_double(Settings::UpdateService, call: ServiceResult.success) } before do allow(Settings::UpdateService).to receive(:new).with(user: User.system).and_return(update_service) + allow(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to receive(:perform_later) end describe "#perform" do @@ -45,7 +47,7 @@ it "enables semantic mode without running any conversion" do allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new) - job.perform + job.perform(batch) expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new) expect(update_service).to have_received(:call) .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) @@ -63,7 +65,7 @@ end it "runs one conversion sweep then enables semantic mode" do - job.perform + job.perform(batch) expect(service).to have_received(:call).once expect(update_service).to have_received(:call) .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) @@ -82,7 +84,7 @@ end it "enables semantic mode after the final sweep clears all projects" do - job.perform + job.perform(batch) expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times expect(update_service).to have_received(:call) .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) @@ -97,16 +99,24 @@ allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set[1]) allow(Project).to receive(:find_by).with(id: 1).and_return(project) allow(ProjectIdentifiers::ConvertProjectToSemanticService).to receive(:new).with(project).and_return(service) + allow(Rails.logger).to receive(:error) end - it "raises after MAX_SWEEPS sweeps, logging a warning and not enabling semantic mode" do - allow(Rails.logger).to receive(:warn) - give_up_pattern = /Giving up after #{described_class::MAX_SWEEPS} sweeps/o + it "does not enable semantic mode and reverts to classic" do + job.perform(batch) + expect(update_service).not_to have_received(:call) + expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later) + end + + it "logs the failure reason" do + job.perform(batch) + expect(Rails.logger).to have_received(:error) + .with(/Giving up after #{described_class::MAX_SWEEPS} sweeps/o) + end - expect { job.perform }.to raise_error(RuntimeError, give_up_pattern) + it "runs the maximum number of sweeps before giving up" do + job.perform(batch) expect(service).to have_received(:call).exactly(described_class::MAX_SWEEPS).times - expect(Rails.logger).to have_received(:warn).with(give_up_pattern) - expect(update_service).not_to have_received(:call) end end @@ -118,11 +128,57 @@ end it "skips the missing project and still enables semantic mode" do - job.perform + job.perform(batch) expect(ProjectIdentifiers::ConvertProjectToSemanticService).not_to have_received(:new) expect(update_service).to have_received(:call) .with(work_packages_identifier: Setting::WorkPackageIdentifier::SEMANTIC) end end + + context "when enabling semantic mode fails" do + before do + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids).and_return(Set.new) + allow(update_service).to receive(:call) + .and_return(instance_double(ServiceResult, success?: false, message: "update failed")) + allow(Rails.logger).to receive(:error) + end + + it "reverts to classic" do + job.perform(batch) + expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later) + end + + it "logs the failure reason" do + job.perform(batch) + expect(Rails.logger).to have_received(:error).with(/Failed to enable semantic mode/) + end + end + + context "when the batch had failures" do + let(:batch) { instance_double(GoodJob::Batch, discarded?: true) } + + before { allow(Rails.logger).to receive(:error) } + + it "does not run a corrective sweep" do + allow(ProjectIdentifiers::PendingProjectsFinder).to receive(:project_ids) + job.perform(batch) + expect(ProjectIdentifiers::PendingProjectsFinder).not_to have_received(:project_ids) + end + + it "does not enable semantic mode" do + job.perform(batch) + expect(update_service).not_to have_received(:call) + end + + it "enqueues RevertInstanceToClassicIdsJob" do + job.perform(batch) + expect(ProjectIdentifiers::RevertInstanceToClassicIdsJob).to have_received(:perform_later) + end + + it "logs the failure reason" do + job.perform(batch) + expect(Rails.logger).to have_received(:error).with(/Batch had failures/) + end + end end end