Specifying queues for active_storage breaks Sidekiq

[mperham]

The code here specifies two new queues for active_storage jobs.

rails/railties/lib/rails/application/configuration.rb

Lines 147 to 148 in 600132a

	active_storage.queues.analysis = :active_storage_analysis
	active_storage.queues.purge = :active_storage_purge

The issue is that each user must configure Sidekiq to listen to any new queues, meaning that every Sidekiq user who upgrades to Rails 6.1 will need to manually add these queues to their config. This is a major footgun.

https://twitter.com/stevepolitodsgn/status/1333976003505434626

My suggestion is to not specify queues but let the jobs use the system's default queue:

  active_storage.queues.analysis = nil
  active_storage.queues.purge    = nil

And allow users to override that config if they want these job types to go into other, custom queues.

[georgeclaghorn]

Other framework components have default queues, too. Action Mailer’s deliver_later feature has defaulted to the :mailers queue since 2014. Would you recommend changing all of them?

[mperham]

ActionMailer was updated to default to "default" for its queue, because it was breaking for so many Sidekiq users. I don't believe mailers is used by new Rails apps but obviously the option remains for people who want to configure it so.

https://github.com/rails/rails/blob/master/activejob/lib/active_job/queue_name.rb#L9

I'm suggesting you not explicitly configure a queue for each job type, let them use the default and everything should work. If the user wants to reroute them to a custom queue, that's their decision. For example some people use queue naming to encode priority: low, default, high, critical and they would want to select a priority queue for each feature:

config.active_storage.queues.purge    = :low

[georgeclaghorn]

ActionMailer was updated to default to "default" for its queue, because it was breaking for so many Sidekiq users. I don't believe mailers is used by new Rails apps but obviously the option remains for people who want to configure it so.

Could you point to where that happened? As far as I can tell, the default for Action Mailer is still :mailers:

rails/actionmailer/lib/action_mailer/delivery_methods.rb

Line 15 in 5384bbc

cattr_accessor :deliver_later_queue_name, default: :mailers

rails/actionmailer/lib/action_mailer/mail_delivery_job.rb

Line 12 in 5384bbc

queue_as { ActionMailer::Base.deliver_later_queue_name }

This is where the default would’ve been changed and I’m not seeing anything there.

[mperham]

Good to know, I guess I'm mistaken.

The bigger picture still remains: will Rails continue to add new queue names for each release, requiring users to configure Rails or Sidekiq manually so they work together? I want to get to a place where new Rails features work by default on Sidekiq.

[mperham]

I wrote this to document the current state. Let me know if there's any Rails internal queues I missed or if it is missing detail. Does Basecamp run Resque using QUEUE=* so no one has to manually enable queue processing for new queues?

[rafaelfranca]

I personally think we should use the default queue by default, but another way forward is to use the Sidekiq::Engine to set the default queue of those components to the default queue on sidekiq if users didn't override it.

[georgeclaghorn]

👍 for switching to :default everywhere.

[mperham]

That's certainly possible. I'm reluctant to add additional "magic" to override Rails defaults. Each version of Rails has different queues to configure and future versions would require future overrides due to new queues.

We're adding more config to overrule more config, Rails -> Sidekiq::Engine -> User config, that mountain of config leads to madness. Just remove all of it (within reason, I get that backwards compatibility is a thing) and let the job runner's defaults work.

[rafaelfranca]

If @georgeclaghorn is ok with changing the queue everywhere, I think we can just change the defaults. The problem is that I don't feel comfortable of doing this change in the rc cycle given people may already changed their sidekiq config to use the new queues.

@mperham do you think we do embrace this change right now or wait for 6.2?

[mperham]

Yeah, I know it's really late in the release cycle. I wish I had known about this earlier.

Per the tweet linked above, I'm worried about the many Rails/Sidekiq users who will upgrade to 6.1, use these features and have it silently break. If we assume that everyone will be processing the default queue, I don't think it will break anyone to switch the default now. If you want to leave mailers as is for legacy reasons but switch active_storage, that's still an improvement.

[mperham]

And don't be afraid to mention me if there are further queue things you want to discuss. It's literally my job to make Sidekiq work great with Rails.