From 530b7a7a28bf64f5bcb11e9e31eee557d5923309 Mon Sep 17 00:00:00 2001 From: Mike Perham Date: Mon, 5 Aug 2019 11:00:21 -0700 Subject: [PATCH] More defensive job acknowledgment logic, fixes #4211 Invert boolean and use handle_interrupt to avoid potential loss edge case --- lib/sidekiq/processor.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/sidekiq/processor.rb b/lib/sidekiq/processor.rb index c894db96..c349da33 100644 --- a/lib/sidekiq/processor.rb +++ b/lib/sidekiq/processor.rb @@ -154,21 +154,22 @@ module Sidekiq return work.acknowledge end - ack = true + ack = false begin dispatch(job_hash, queue) do |worker| Sidekiq.server_middleware.invoke(worker, job_hash, queue) do execute_job(worker, cloned(job_hash["args"])) end end + ack = true rescue Sidekiq::Shutdown # Had to force kill this job because it didn't finish # within the timeout. Don't acknowledge the work since # we didn't properly finish it. - ack = false rescue Sidekiq::JobRetry::Handled => h # this is the common case: job raised error and Sidekiq::JobRetry::Handled # signals that we created a retry successfully. We can acknowlege the job. + ack = true e = h.cause || h handle_exception(e, {context: "Job raised exception", job: job_hash, jobstr: jobstr}) raise e @@ -176,11 +177,15 @@ module Sidekiq # Unexpected error! This is very bad and indicates an exception that got past # the retry subsystem (e.g. network partition). We won't acknowledge the job # so it can be rescued when using Sidekiq Pro. - ack = false handle_exception(ex, {context: "Internal exception!", job: job_hash, jobstr: jobstr}) raise e ensure - work.acknowledge if ack + if ack + # We don't want a shutdown signal to interrupt job acknowledgment. + Thread.handle_interrupt(Sidekiq::Shutdown => :never) do + work.acknowledge + end + end end end