Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify queue for batch callbacks #2200

Closed
johnathanludwig opened this issue Feb 22, 2015 · 12 comments
Closed

Specify queue for batch callbacks #2200

johnathanludwig opened this issue Feb 22, 2015 · 12 comments
Labels

Comments

@johnathanludwig
Copy link

Similar to #1953, we queue up a multitude of batches at once with varying numbers of jobs. The success callbacks of the smaller batches are being held back by the larger ones, which we would like to avoid.

In our case, the batches themselves are created in higher priority queue than the jobs they enqueue. I noticed the batch callbacks get queued in the same queue as the last job of the batch that finished.

Could you add the ability to specify the queue of the batch callback job? This would allow us to put it in the higher priority queue so they get picked up before the jobs for other batches are done.

@johnathanludwig johnathanludwig changed the title Specify Queue for Batch callbacks Specify queue for batch callbacks Feb 22, 2015
@mperham
Copy link
Collaborator

mperham commented Feb 22, 2015

I could do something like this:

batch.callback_queue = 'critical'

The only issue is that right now the Batch middleware doesn't need to fetch any Batch info from Redis. This would require an extra round trip to Redis so there's a performance penalty.

@jonhyman
Copy link
Contributor

Could you make your batch callback simply enqueue another job in the queue of your choice, and have that job do the callback work?

@johnathanludwig
Copy link
Author

@jonhyman I could but that wouldn't solve the problem. The problem is the callbacks are already a job themselves and Sidekiq enqueues them in the same queue as the rest of the jobs.

@mperham So the jobs do not know what the batch class is when they create the callback job? How about making it a global setting that people could set in their Sidekiq initializer or config. Not as customizable but it would get around the performance issue.

@mperham
Copy link
Collaborator

mperham commented Feb 23, 2015

@johnathanludwig Never mind, I could do it without a network trip.

@mperham mperham added the pro label Feb 23, 2015
@johnathanludwig
Copy link
Author

Has the work on this already been done? If not I should have some time to make a contribution.

@mperham
Copy link
Collaborator

mperham commented Mar 12, 2015

I thought about it for some time and couldn't determine an implementation I was happy with. Can you not spin up more worker processes so the queues drain faster?

@jonhyman
Copy link
Contributor

@mperham I assume that you thought about publishing the queue name to the batch and getting it back in the multi which adds successes/failures?

@johnathanludwig
Copy link
Author

We are already running quite a few workers (around 500). Our jobs are making lots of external API calls so they aren't very fast anyways.

The problem isn't a huge deal anymore. It was made more noticeable by a regression in our code that caused the workers to slow down. We fixed that so all of our batches are finishing more timely, but it would still be preferable to not have the success callbacks for one batch wait on the other batches to finish.

@mperham
Copy link
Collaborator

mperham commented Mar 13, 2015

I've been busy with other work but I'll see if I can't implement this and get 2.0.1 released next week.

@mperham mperham closed this as completed Mar 15, 2015
mperham added a commit that referenced this issue Mar 15, 2015
@mhuggins
Copy link
Contributor

@mperham It took a bit of searching to find this. Would it be possible to update the wiki page with this option? Unless there is somewhere else to find it that I simply overlooked. Thanks!

@mperham
Copy link
Collaborator

mperham commented Feb 24, 2016

@mhuggins The wiki is publicly editable. Added here: https://github.com/mperham/sidekiq/wiki/Batches#callback-details

@mhuggins
Copy link
Contributor

Good to know, I surprisingly haven't used github wiki previously. I'll keep that in mind for next time. Much appreciated. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants