Conversation
lib/sidekiq/fetch.rb
Outdated
There was a problem hiding this comment.
No need to do this map every time this method is called.
There was a problem hiding this comment.
Well, I was thinking that for each iteration you'd want to invoke the @queues.sample(@num_queues) so that you apply the weights for each iteration. Once you know which queues you want to pass to blpop, then you'd do queues.map { |q| "queue:#{q}" }. I think we could have a map that create in the #initialize that maps queue names to their equivalent "queue:#{q}" form.
There was a problem hiding this comment.
That's what I mean: the queue name can be mapped in advance.
There was a problem hiding this comment.
Cool, refresh pull request - I've updated it to map the queue names just once in #initialize.
|
Needs more hot developer-on-changelog action. |
|
I am interested by the fact that you only have tests for the cli. Is this common practice to merge untested code? |
|
I have a series of manual integration tests I run through with live processing to verify nothing is obviously broken. I welcome pull requests to increase test coverage. :-) |
Here's a first pass at re-introducing weighted queues. The basic idea is to dynamically pass in a set of queues (respecting weights) to Redis#blpop. This way we avoid queue starvation. Thoughts welcome.