QueueClassic/queue_classic

Does not work the default ActionMailer queue introduced in Rails 4.2

myobie opened this issue · 15 comments

The default queue where emails are added is "mailers" and it should be assumed that queue is being worked by default. delayed_job does this.

This can be worked around with QUEUES=mailers rake jobs:work for now.

Got it. So, we could either make it listen by default. Not sure of the side impact of this, probably not much. That said, it's not going to make 3.1 as it would be a breaking change.

@senny: thoughts on this? Do you see a reason not to change this behavior? I personally would not change it, but don't have a strong opinion. If it makes it easier for some people to get started, maybe it makes sense.

senny commented

@jipiboily I wouldn't change it to work both "default" and "mailers" if nothing else is specified. That seems very arbitrary. I see the following possible solutions:

  1. Have the default bin/rake qc:work work all queues unless specified otherwise
  2. In the queue classic railtie configure ActionMailer to use the default queue instead of the mailers queue.

Yeah, sorry for the confusion, I had in mind all queues by default, or stick with the current...as the only two decent options.

I would lean toward the first option you gave, or the status quo.

senny commented

I'm fine with option 1.)

I don't have time to work on this but if @myobie or anyone else want to send a PR, that would be awesome!

I would love to help. After looking more into it I don't see an easy way to work all queues yet, so if you have any suggestions of where to look let me know.

senny commented

@myobie that's correct. Currently there's no q_name wildcard for the worker to get it to work all queues.

OK, then I think it's best to add a feature to accept QUEUE=* and then just make that the return value of default_queue. Does that sound good? I'll try to get a PR for that this week.

senny commented

@myobie would that really solve the problem? Imagine default_queue is set to "default". How would QUEUE=* pickup on the mailer queue?

I meant that default_queue would be "*".

senny commented

@myobie sounds good 😊

The way the triggers work makes it difficult to listen to all queues, since the locks are per queue_name. I thought about doing something like this:

@@ -75,6 +75,7 @@ $$ LANGUAGE plpgsql;
 -- queue_classic_notify function and trigger
 create function queue_classic_notify() returns trigger as $$ begin
   perform pg_notify(new.q_name, '');
+  perform pg_notify('*', new.q_name);
   return null;
 end $$ language plpgsql;

but the lock_head function still is by queue_name. Listening to all queues could require a complete change of things and I'm not sure that's a good idea.

So I wanted to ask your opinion of what might be best? Is it worth going down this road or should this really just be a documentation problem?

senny commented

On Rails we have rails/rails#18587 open. I think it could be reasonable to change the default Action Mailer queue to default in our QC Railtie. It should be done in such a fashion that the user can still overwrite that configuration option in either config/application.rb or config/environments/*.rb.

@jipiboily thoughts?

I agree with @senny. I think changing the default via our railtie but making it possible to overwrite + document this in the README would be perfect!

I'm closing this since it's so old.