diff --git a/README.md b/README.md index 33d1dc3ee..4367c6b07 100644 --- a/README.md +++ b/README.md @@ -555,6 +555,8 @@ It's important to note that after one or more candidate jobs are unblocked (eith When using `discard` as the behaviour to handle conflicts, you might have jobs discarded for until the `duration` interval if something happens and a running job fails to release the semaphore. +If a job's class no longer exists by the time its concurrency controls are checked—say it was renamed or removed in a deploy while jobs referencing it were still in the queue—the job is marked as failed with a `SolidQueue::Job::ClassMissingError`, so it shows up in [failed jobs](#failed-jobs-and-retries), where it can be retried once the class is back, or discarded. Jobs with a missing class picked up by a worker fail with the same error. + For example: ```ruby diff --git a/app/models/solid_queue/blocked_execution.rb b/app/models/solid_queue/blocked_execution.rb index 68551a5f5..a84660a46 100644 --- a/app/models/solid_queue/blocked_execution.rb +++ b/app/models/solid_queue/blocked_execution.rb @@ -46,7 +46,12 @@ def releasable(concurrency_keys) def release SolidQueue.instrument(:release_blocked, job_id: job.id, concurrency_key: concurrency_key, released: false) do |payload| transaction do - if acquire_concurrency_lock + if job.job_class.nil? + job.failed_with(Job::ClassMissingError.for(job)) + destroy! + + payload[:failed] = true + elsif acquire_concurrency_lock promote_to_ready destroy! @@ -58,7 +63,8 @@ def release private def set_expires_at - self.expires_at = job.concurrency_duration.from_now + duration = job.job_class ? job.concurrency_duration : SolidQueue.default_concurrency_control_period + self.expires_at = duration.from_now end def acquire_concurrency_lock diff --git a/app/models/solid_queue/claimed_execution.rb b/app/models/solid_queue/claimed_execution.rb index 90b56b42b..4b39c8c10 100644 --- a/app/models/solid_queue/claimed_execution.rb +++ b/app/models/solid_queue/claimed_execution.rb @@ -115,6 +115,8 @@ def finalizing end def execute + raise Job::ClassMissingError.for(job) if job.job_class.nil? + ActiveJob::Base.execute(job.arguments.merge("provider_job_id" => job.id)) Result.new(true, nil) rescue Exception => e diff --git a/app/models/solid_queue/job.rb b/app/models/solid_queue/job.rb index 49b47bb91..cb8f25bee 100644 --- a/app/models/solid_queue/job.rb +++ b/app/models/solid_queue/job.rb @@ -4,6 +4,15 @@ module SolidQueue class Job < Record class EnqueueError < StandardError; end + # Raised when a job's class can't be resolved anymore, typically because it + # was renamed or removed in a deploy while jobs referencing it were in + # flight. It subclasses NameError, which is what resolving the class raises. + class ClassMissingError < NameError + def self.for(job) + new("Job class #{job.class_name.inspect} could not be resolved") + end + end + include Executable, Clearable, Recurrable, Batchable serialize :arguments, coder: JSON diff --git a/app/models/solid_queue/job/concurrency_controls.rb b/app/models/solid_queue/job/concurrency_controls.rb index 30d4399ed..b59c464ad 100644 --- a/app/models/solid_queue/job/concurrency_controls.rb +++ b/app/models/solid_queue/job/concurrency_controls.rb @@ -33,6 +33,10 @@ def blocked? blocked_execution.present? end + def job_class + @job_class ||= class_name.safe_constantize + end + private def concurrency_on_conflict job_class.concurrency_on_conflict.to_s.inquiry @@ -66,10 +70,6 @@ def release_next_blocked_job BlockedExecution.release_one(concurrency_key) end - def job_class - @job_class ||= class_name.safe_constantize - end - def execution super || blocked_execution end diff --git a/lib/solid_queue/log_subscriber.rb b/lib/solid_queue/log_subscriber.rb index 6806b853e..fd4f54fd0 100644 --- a/lib/solid_queue/log_subscriber.rb +++ b/lib/solid_queue/log_subscriber.rb @@ -59,7 +59,7 @@ def release_many_blocked(event) end def release_blocked(event) - debug formatted_event(event, action: "Release blocked job", **event.payload.slice(:job_id, :concurrency_key, :released)) + debug formatted_event(event, action: "Release blocked job", **event.payload.slice(:job_id, :concurrency_key, :released, :failed)) end def enqueue_recurring_task(event) diff --git a/test/models/solid_queue/blocked_execution_test.rb b/test/models/solid_queue/blocked_execution_test.rb new file mode 100644 index 000000000..8586edb5d --- /dev/null +++ b/test/models/solid_queue/blocked_execution_test.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require "test_helper" + +class SolidQueue::BlockedExecutionTest < ActiveSupport::TestCase + self.use_transactional_tests = false + + class NonOverlappingJob < ApplicationJob + limits_concurrency key: ->(job_result, **) { job_result } + + def perform(job_result) + end + end + + setup do + @result = JobResult.create!(queue_name: "default") + end + + teardown do + SolidQueue::Job.destroy_all + SolidQueue::Semaphore.delete_all + JobResult.delete_all + end + + test "release marks the job as failed and destroys the blocked row when the job class no longer resolves" do + # Enqueue and consume the semaphore so the next job blocks. + NonOverlappingJob.perform_later(@result) + blocking_job = SolidQueue::Job.last + NonOverlappingJob.perform_later(@result) + blocked_job = SolidQueue::Job.last + blocked = blocked_job.blocked_execution + assert blocked, "expected the second job to be blocked" + + # Simulate the class being renamed/removed between deploys + blocked_job.update_columns(class_name: "GoneJob") + + assert_difference -> { SolidQueue::BlockedExecution.count } => -1, + -> { SolidQueue::FailedExecution.count } => 1 do + assert_nothing_raised do + blocked.reload.release + end + end + + # No ready execution was promoted — the orphan is now surfaced as a failed execution. + assert_nil SolidQueue::ReadyExecution.find_by(job_id: blocked_job.id) + + failed = SolidQueue::FailedExecution.find_by(job_id: blocked_job.id) + assert failed, "expected a failed execution to be created for the orphan" + assert_equal "SolidQueue::Job::ClassMissingError", failed.exception_class + assert_match "GoneJob", failed.message + end +end diff --git a/test/models/solid_queue/claimed_execution_test.rb b/test/models/solid_queue/claimed_execution_test.rb index 01242883c..b09fdeb90 100644 --- a/test/models/solid_queue/claimed_execution_test.rb +++ b/test/models/solid_queue/claimed_execution_test.rb @@ -146,7 +146,7 @@ class SolidQueue::ClaimedExecutionTest < ActiveSupport::TestCase claimed_execution = claim_job(job) assert_difference -> { SolidQueue::ClaimedExecution.count } => -1, -> { SolidQueue::FailedExecution.count } => 1 do - assert_raises NameError do + assert_raises SolidQueue::Job::ClassMissingError do claimed_execution.perform end end @@ -159,7 +159,7 @@ class SolidQueue::ClaimedExecutionTest < ActiveSupport::TestCase claimed_execution = claim_job(job) assert_difference -> { SolidQueue::ClaimedExecution.count } => -1, -> { SolidQueue::FailedExecution.count } => 1 do - assert_raises NameError do + assert_raises SolidQueue::Job::ClassMissingError do claimed_execution.perform end end