Permalink
Browse files

Use sort_by{rand} instead of sort{rand} for randomization as sort{ran…

…d} is of consistent outcome. Thanks Alex Ostleitner.
  • Loading branch information...
1 parent a99302b commit 9935082b7e6b4eb900ee6830042d0a80543f3a7d @tobi committed Dec 30, 2008
Showing with 1 addition and 2 deletions.
  1. +1 −2 lib/delayed/job.rb
View
@@ -1,4 +1,3 @@
-
module Delayed
class DeserializationError < StandardError
@@ -113,7 +112,7 @@ def self.find_available(limit = 5, max_run_time = MAX_RUN_TIME)
find(:all, :conditions => conditions, :order => NextTaskOrder, :limit => limit)
end
- records.sort { rand() }
+ records.sort_by { rand() }
end
# Get the payload of the next job we can get an exclusive lock on.

3 comments on commit 9935082

Contributor

lukemelia replied Jan 21, 2009

What’s the goal of randomizing the order here? It seems that on line 112, the code specifies a particular order (NextTaskOrder) which includes priority. Wouldn’t that order be lost by line 115?

maia replied Jan 22, 2009

luke, I came across the problem that having 3 jobs with a high priority and a dozen of lower prioritized jobs DJ decided to work off the less important jobs – simply because it grabbed 5 jobs, always ‘randomized’ them the same way, therefor always coming up with the same order, which unfortunately put the less important jobs first in the queue.

Oh, and the basic goal of picking the next job randomly out of the first 5 jobs has the goal to reduce the possibility of parallel workers attempting to work off the same job (locked) at the same time. In other words, DJ is currently optimized for 3-5 parallel workers, with the downside that if you only have one or two workers you might occasionally end up with low priority jobs being worked off first.

any further thoughts on the sort_by ? Why do we need to sort the jobs ?

Sorting the jobs on the queue feels wrong.

Please sign in to comment.