Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

Add priority reliable queue #4

Merged
merged 2 commits into from
Aug 11, 2017
Merged

Add priority reliable queue #4

merged 2 commits into from
Aug 11, 2017

Conversation

mudge
Copy link
Contributor

@mudge mudge commented Aug 6, 2017

So that a single worker can process work from multiple queues of different priority (e.g. a "critical" queue, a default queue and a "backlog" queue), add a new Priority Reliable Queue class which will check a given array of Redis keys for work in a specific order.

This will ensure that the highest priority queue is empty before checking the next queue for work and so on until the last queue.

So that a single worker can process work from multiple queues of
different priority (e.g. a "critical" queue, a default queue and a
"backlog" queue), add a new Priority Reliable Queue class which will
check a given array of Redis keys for work in a specific order.

This will ensure that the highest priority queue is empty before
checking the next queue for work and so on until the last queue.
@mudge
Copy link
Contributor Author

mudge commented Aug 6, 2017

An issue with this is that it relies on polling all queues with RPOPLPUSH as fast as possible which causes a lot of Redis traffic. We could artificially block for a second on each attempt to reduce this churn but sacrifice efficiency of queue workers.

See redis/redis#1785 and sidekiq/sidekiq#1769 for the discussion of a new command that would make pop-push-ing multiple queues easier.

}
}

public function rewind()

Choose a reason for hiding this comment

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

I'm not sure I understand what is the meaning of rewind in the context of the queue? Is it only here to make sure we conform to the Iterator interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, take a look at the Iterator interface, it's a key part of how foreach works:

In short, when you loop over an iterator, it calls the following methods in order:

  1. rewind
  2. valid
  3. current
  4. key
  5. next
  6. Go to 2.

Choose a reason for hiding this comment

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

I guess I'm just not used to rewind being stateful, but maybe that's okay for practical reasons 🤔

@mudge
Copy link
Contributor Author

mudge commented Aug 7, 2017

See Sidekiq's Weighted random algorithm for an alternate algorithm we could try here.

@AnnaKL
Copy link

AnnaKL commented Aug 7, 2017

@mudge why is this better than having processes with different priorities?
sidekiq -e production -q critical -q default -q bulk
sidekiq -e production -q bulk -q default -q critical

@mudge
Copy link
Contributor Author

mudge commented Aug 7, 2017

@AnnaKL it's not necessarily "better": it would be an alternative to the current solution of running one worker per queue.

The idea here being that you could run fewer, simpler workers (e.g. all configured identically) and have them consume and process work of different priorities without having to scale up and down yourself.

At the moment, if you want to consume a critical and a default priority queue, you have to have two processes consuming two different queues. I want to provide an alternative that would allow you to run one process and have it consume multiple queues with some notion of priority.

@mudge mudge force-pushed the priority-queue branch 3 times, most recently from abbeea1 to b0a7975 Compare August 7, 2017 20:30
@mudge
Copy link
Contributor Author

mudge commented Aug 8, 2017

As testing randomness across PHP versions is proving somewhat tricky due to backwards-incompatible changes to rand, srand, mt_rand and mt_srand. We might need to inject a source of randomness as a dependency that we can replace in tests.

@mudge mudge force-pushed the priority-queue branch 2 times, most recently from 1acc589 to f2b475a Compare August 11, 2017 14:40
Instead of waiting for each queue to drain in priority order, rely on
probability to pull from ordered queues in rough priority.

This way, we can block for a second when waiting for work to alleviate
pressure on Redis when queues are empty and we can avoid queue
starvation by checking all queues even if the highest priority is always
full.
@mudge mudge merged commit e2b4d4e into master Aug 11, 2017
@mudge mudge deleted the priority-queue branch August 11, 2017 15:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants