Permalink
Browse files

Introduce a ShuffledRoundRobin reserver.

This ensures work is done more evenly across all queues. The
normal RoundRobin reserver always slightly favors the first
queue in the list since all workers will start at the same
spot everytime they get restarted.
  • Loading branch information...
1 parent 53ebcaa commit a1d134c5ed66b106db96a5c1d90ebf72469cbe18 @myronmarston myronmarston committed Mar 8, 2013
View
4 lib/qless/job_reservers/round_robin.rb
@@ -19,11 +19,13 @@ def reserve
end
def description
- @description ||= @queues.map(&:name).join(', ') + " (round robin)"
+ @description ||= @queues.map(&:name).join(', ') + " (#{self.class::TYPE_DESCRIPTION})"
end
private
+ TYPE_DESCRIPTION = "round robin"
+
def next_queue
@last_popped_queue_index = (@last_popped_queue_index + 1) % @num_queues
@queues[@last_popped_queue_index]
View
14 lib/qless/job_reservers/shuffled_round_robin.rb
@@ -0,0 +1,14 @@
+require 'qless/job_reservers/round_robin'
+
+module Qless
+ module JobReservers
+ class ShuffledRoundRobin < RoundRobin
+ def initialize(queues)
+ super(queues.shuffle)
+ end
+
+ TYPE_DESCRIPTION = "shuffled round robin"
+ end
+ end
+end
+
View
81 spec/unit/job_reservers/shuffled_round_robin_spec.rb
@@ -0,0 +1,81 @@
+require 'spec_helper'
+require 'qless/queue'
+require 'qless/job_reservers/shuffled_round_robin'
+
+module Qless
+ module JobReservers
+ describe ShuffledRoundRobin do
+ let(:q1) { fire_double("Qless::Queue") }
+ let(:q2) { fire_double("Qless::Queue") }
+ let(:q3) { fire_double("Qless::Queue") }
+
+ let(:queue_list) { [q1, q2, q3] }
+
+ def new_reserver
+ ShuffledRoundRobin.new(queue_list)
+ end
+
+ let(:reserver) { new_reserver }
+
+ describe "#reserve" do
+ it 'round robins the queues' do
+ queue_list.stub(shuffle: queue_list)
+
+ q1.should_receive(:pop).twice { :q1_job }
+ q2.should_receive(:pop).once { :q2_job }
+ q3.should_receive(:pop).once { :q3_job }
+
+ reserver.reserve.should eq(:q1_job)
+ reserver.reserve.should eq(:q2_job)
+ reserver.reserve.should eq(:q3_job)
+ reserver.reserve.should eq(:q1_job)
+ end
+
+ it 'returns nil if none of the queues have jobs' do
+ q1.should_receive(:pop).once { nil }
+ q2.should_receive(:pop).once { nil }
+ q3.should_receive(:pop).once { nil }
+ reserver.reserve.should be_nil
+ end
+ end
+
+ it 'shuffles the queues so that things are distributed more easily' do
+ order = []
+
+ [q1, q2, q3].each do |q|
+ q.stub(:pop) { order << q }
+ end
+
+ uniq_orders = 10.times.map do
+ order.clear
+ reserver = new_reserver
+ 3.times { reserver.reserve }
+ order.dup
+ end.uniq
+
+ expect(uniq_orders).to have_at_least(3).different_orders
+ end
+
+ it 'does not change the passed queue list as a side effect' do
+ orig_list = queue_list.dup
+
+ 10.times do
+ new_reserver
+ expect(queue_list).to eq(orig_list)
+ end
+ end
+
+ describe "#description" do
+ it 'returns a useful human readable string' do
+ queue_list.stub(shuffle: [q2, q1, q3])
+ q1.stub(:name) { "Queue1" }
+ q2.stub(:name) { "Queue2" }
+ q3.stub(:name) { "Queue3" }
+
+ reserver.description.should eq("Queue2, Queue1, Queue3 (shuffled round robin)")
+ end
+ end
+ end
+ end
+end
+

0 comments on commit a1d134c

Please sign in to comment.