From f5f98ce8b57931dfb5956f59c58e35f3eccc1984 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 27 Nov 2023 13:47:30 -0800 Subject: [PATCH] Only check out a Redis connection if necessary In Sidekiq 7, calls to `Sidekiq.redis` checks out a connection from an internal connection pool instead of the connection pool used by the job processors. If a connection is checked out but not used, it's possible that the heartbeat thread won't keep the connection alive before the Redis server client timeout. To avoid this, update the checks to return earlier if the Redis connection is not needed. This might help reduce intermittent `Broken pipe` errors as reported in https://github.com/redis-rb/redis-client/issues/119. --- lib/sidekiq/cron/job.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/sidekiq/cron/job.rb b/lib/sidekiq/cron/job.rb index bfc930a8..52e5644a 100644 --- a/lib/sidekiq/cron/job.rb +++ b/lib/sidekiq/cron/job.rb @@ -21,13 +21,15 @@ class Job # Crucial part of whole enqueuing job. def should_enque? time + return false unless status == "enabled" + return false unless not_past_scheduled_time?(time) + return false unless not_enqueued_after?(time) + enqueue = Sidekiq.redis do |conn| - status == "enabled" && - not_past_scheduled_time?(time) && - not_enqueued_after?(time) && - conn.zadd(job_enqueued_key, formatted_enqueue_time(time), formatted_last_time(time)) + conn.zadd(job_enqueued_key, formatted_enqueue_time(time), formatted_last_time(time)) end - enqueue == true || enqueue == 1 + + enqueue == 1 end # Remove previous information about run times, @@ -239,12 +241,11 @@ def self.count def self.find name # If name is hash try to get name from it. name = name[:name] || name['name'] if name.is_a?(Hash) + return unless exists? name output = nil Sidekiq.redis do |conn| - if exists? name - output = Job.new conn.hgetall( redis_key(name) ) - end + output = Job.new conn.hgetall( redis_key(name) ) end output if output && output.valid? end