Skip to content

Commit

Permalink
Use sort_by{rand} instead of sort{rand} for randomization as sort{ran…
Browse files Browse the repository at this point in the history 
…d} is of consistent outcome. Thanks Alex Ostleitner.
  • Loading branch information
tobi committed Dec 30, 2008
1 parent a99302b commit 9935082
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions lib/delayed/job.rb
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

module Delayed

class DeserializationError < StandardError
Expand Down Expand Up @@ -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.
Expand Down

3 comments on commit 9935082

@lukemelia
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

@maia maia commented on 9935082 Jan 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rohankini
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.