1
0
Fork 0
mirror of https://github.com/mperham/sidekiq.git synced 2022-11-09 13:52:34 -05:00

More defensive job acknowledgment logic, fixes #4211

Invert boolean and use handle_interrupt to avoid potential loss edge case
This commit is contained in:
Mike Perham 2019-08-05 11:00:21 -07:00
parent 4c2ea6ebad
commit 530b7a7a28

View file

@ -154,21 +154,22 @@ module Sidekiq
return work.acknowledge return work.acknowledge
end end
ack = true ack = false
begin begin
dispatch(job_hash, queue) do |worker| dispatch(job_hash, queue) do |worker|
Sidekiq.server_middleware.invoke(worker, job_hash, queue) do Sidekiq.server_middleware.invoke(worker, job_hash, queue) do
execute_job(worker, cloned(job_hash["args"])) execute_job(worker, cloned(job_hash["args"]))
end end
end end
ack = true
rescue Sidekiq::Shutdown rescue Sidekiq::Shutdown
# Had to force kill this job because it didn't finish # Had to force kill this job because it didn't finish
# within the timeout. Don't acknowledge the work since # within the timeout. Don't acknowledge the work since
# we didn't properly finish it. # we didn't properly finish it.
ack = false
rescue Sidekiq::JobRetry::Handled => h rescue Sidekiq::JobRetry::Handled => h
# this is the common case: job raised error and Sidekiq::JobRetry::Handled # this is the common case: job raised error and Sidekiq::JobRetry::Handled
# signals that we created a retry successfully. We can acknowlege the job. # signals that we created a retry successfully. We can acknowlege the job.
ack = true
e = h.cause || h e = h.cause || h
handle_exception(e, {context: "Job raised exception", job: job_hash, jobstr: jobstr}) handle_exception(e, {context: "Job raised exception", job: job_hash, jobstr: jobstr})
raise e raise e
@ -176,11 +177,15 @@ module Sidekiq
# Unexpected error! This is very bad and indicates an exception that got past # 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 # the retry subsystem (e.g. network partition). We won't acknowledge the job
# so it can be rescued when using Sidekiq Pro. # so it can be rescued when using Sidekiq Pro.
ack = false
handle_exception(ex, {context: "Internal exception!", job: job_hash, jobstr: jobstr}) handle_exception(ex, {context: "Internal exception!", job: job_hash, jobstr: jobstr})
raise e raise e
ensure 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
end end