Skip to content

Commit

Permalink
Only check out a Redis connection if necessary
Browse files Browse the repository at this point in the history
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 that 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 redis-rb/redis-client#119.
  • Loading branch information
stanhu committed Nov 27, 2023
1 parent dcee41c commit 5dae081
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions lib/sidekiq/cron/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5dae081

Please sign in to comment.