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

Define queue priority separately #782

Open
itskingori opened this issue Feb 16, 2015 · 11 comments
Open

Define queue priority separately #782

itskingori opened this issue Feb 16, 2015 · 11 comments

Comments

@itskingori
Copy link

This is a feature request/suggestion ... not a bug request. Actually, my objective is to spur conversation.

Was just wondering if it would be useful to be able to pre-define priorities e.g. using the delayed_jobs initialiser. For example ... in config/initializers/delayed_job_config.rb we could have queues and their pre-defined priorities.

# List of queues in our app and their attributes etc etc
Delayed::Worker.queue_attributes = [
  { name: 'qname_1', priority: 10 },
  { name: 'qname_2', priority: 13 },
  { name: 'qname_3', priority: 12 },
  { name: 'qname_4', priority: 9 },
  { name: 'qname_5', priority: 0 },
  { name: 'qname_6', priority: 5 }
]

So in your app you just create jobs and assign them to queues (without specifying priorities).

Currently, this is not the case. For example in this Image class I have a delayed job to delete the image file from S3 after the object has been destroyed. I have to specify the priority with the queue like so ...

# Callbacks
after_commit(on: :destroy) { Image.delay(queue: 'delete-image', priority: 10).delete_file_from_s3 key }

If we had predefined priorities, all I'd have had to do would be to just to this ...

# Callbacks
after_commit(on: :destroy) { Image.delay(queue: 'delete-image').delete_file_from_s3 key }

Immediate benefit of this approach would be that queue priorities are centrally managed. And if you want to shift things around you don't have to dig into the code to change priorities in every place that you specified.

Oh and locally specified values would override initialiser defined value ... or something like that. These are just details that can be discussed ... but the general idea has been laid out.

Ps: I'd assume that most people use queues to lump jobs with the same characteristics ... so it feels natural to expect that there'd be a place where we can define the queue characteristics so that we simply assign jobs to those queues.

@ryannealmes
Copy link

I feel like this is a must! I have actually been chatting on stackoverflow around not being able to set priority when leveraging activejob and delayed_job. The main reason for this is you cannot set queue names and priority via the config file.

I really want to contribute to this so I am going to investigate how to actually achieve this. From my initial investigation is simply seems like the change would involve modifying delayed/backend/base.rb a little bit and parsing the configuration information.

Some side notes - I am guessing if the queue doesn't exist in the config hash it would just use the default. I would also imagine using a list of queue objects instead of a hash might be better. Not sure what data structures are already there but I will take a look.

@ryannealmes
Copy link

I have made an enhancement for this. I still have to write the tests, but it would be great to get some input before I make a PR.

@itskingori
Copy link
Author

@ryannealmes I completely forgot about this. Thanks for picking it up! And I've just followed your post on SO. I don't know enough about ActiveJob at the moment other than it's an abstraction so can't chip in much there. So far I've just used delayed_job as I've always had. This has been the missing feature! 👍

@ryannealmes
Copy link

Great ... well it's pretty much done. I will look into getting the tests written up today and create the PR.

@obskein
Copy link

obskein commented Jan 8, 2016

As the main documentation alludes to this functinality being implemented, are we anywhere near cutting a release? Tis been ~3 months since the last release and we have dev's who are very interested in this functionality...

@ryannealmes
Copy link

Sorry I am not in control of this. Glad to hear it will be used once released.

@svoop
Copy link

svoop commented Jan 11, 2016

👍 Thanks for your PR, @ryannealmes. We're using it in production for months now and I'm looking forward to removing the github: in the Gemfile. This is an important bit for Rails 4.2 and since Rails 5 is already lurking around the corner, why not cut a new release now, @collectiveidea? 🚀

@cseelus
Copy link

cseelus commented Feb 14, 2016

Thanks @ryannealmes, this works great and is a very useful addition to DJ.

@ConfusedVorlon
Copy link

should this functionality be working? It seems that the 4.1.1 release was after the merge - but I get

undefined method `queue_attributes=' for Delayed::Worker:Class

am I missing something?

@cseelus
Copy link

cseelus commented Mar 17, 2016

Currently you still need to use the Github release:

gem 'delayed_job', github: 'collectiveidea/delayed_job'

@ConfusedVorlon
Copy link

@cseelus thanks.

@collectiveidea - any chance of a release? You have some nice functionality described in the docs, merged 6 months ago, and not working in a release...

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

No branches or pull requests

6 participants