Skip to content

Commit f0089cd

Browse files
committed
Include job_id in order clause when fetching scheduled jobs to dispatch and when dispatching
This was missing and makes the locking prone to deadlocks when we have a bunch of jobs scheduled at the same time with the same priority, since we'd have a non-deterministic ordering. Besides, on the DELETE query, force a order by job_id even if we don't care about that. It turns out, under certain circumstances, when the scheduled_executions table is too small, MySQL might choose not to use any indexes for the DELETE by job_id. This means it might lock other rows besides those it's going to delete. Then, when running more than one dispatcher doing this, we might end up with a deadlock because one dispatcher is deleting and locking some rows that's not deleting, and the other is doing the same, and both are waiting for the other's lock. This deadlock was happening consistently in CI, for MySQL only, but I didn't manage to reproduce it locally, and it has never happened in production for us.
1 parent 512b0ac commit f0089cd

File tree

3 files changed

+3
-2
lines changed

3 files changed

+3
-2
lines changed

app/models/solid_queue/execution/dispatching.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ def dispatch_jobs(job_ids)
1010
jobs = Job.where(id: job_ids)
1111

1212
Job.dispatch_all(jobs).map(&:id).tap do |dispatched_job_ids|
13-
where(job_id: dispatched_job_ids).delete_all
13+
where(job_id: dispatched_job_ids).order(:job_id).delete_all
1414
SolidQueue.logger.info("[SolidQueue] Dispatched #{dispatched_job_ids.size} jobs")
1515
end
1616
end

app/models/solid_queue/scheduled_execution.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class ScheduledExecution < Execution
55
include Dispatching
66

77
scope :due, -> { where(scheduled_at: ..Time.current) }
8-
scope :ordered, -> { order(scheduled_at: :asc, priority: :asc) }
8+
scope :ordered, -> { order(scheduled_at: :asc, priority: :asc, job_id: :asc) }
99
scope :next_batch, ->(batch_size) { due.ordered.limit(batch_size) }
1010

1111
assumes_attributes_from_job :scheduled_at

test/unit/dispatcher_test.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ class DispatcherTest < ActiveSupport::TestCase
8888
assert_equal 15, SolidQueue::ScheduledExecution.count
8989

9090
another_dispatcher = SolidQueue::Dispatcher.new(polling_interval: 0.1, batch_size: 10)
91+
9192
@dispatcher.start
9293
another_dispatcher.start
9394

0 commit comments

Comments
 (0)