rails / solid_queue

Database-backed Active Job backend
MIT License
1.84k stars 110 forks source link

Set `enqueue_after_transaction_commit` to `true` and don't allow changing it #301

Closed rosa closed 2 weeks ago

rosa commented 1 month ago

See the discussion at https://github.com/rails/rails/pull/52659. Adapters should set their own behaviour without making users choose. If users want to choose, they can just use Active Job's configuration. Besides, we're changing the value of this to true to make it harder for people to run into trouble in the future when switching adapters.

hms commented 1 month ago

@rosa

At the risk of sounding a little dense, I'm totally confused by your response above and after reading the linked issue, I feel like I'm only getting to read parts of the conversation and even those bits are out of order.

One of the reasons I've always used database backed ActiveJob backends (delayed_job, good_job, and now solid_queue) is specifically so I get access to ACID behaviors when I require them. I completely understand that once one gets to a given scale (and one requires the speed of a non-database queue or need to split the job system across databases), things get harder, but I really want to push those issues off as far as I can.

A simple Real-world example: activating a new user/customer and sending a welcome email. This is the quintessential example of Job A enqueues Job B and requires almost zero thinking with ACID enqueuing -- the User is Activated and welcomed or not.

Without ACID enqueuing, this requires tracking state and having some form of a watchdog / polling process to detect and manage state transitions.

Given the amount of code between #perform_later and a job being durably enqueued in the after_commit block, there is (relatively speaking) a lot of time for an asynchronous event intervene and is the kind of "heisenbug" that drives developers to drink...

rosa commented 1 month ago

@hms, no worries, not dense! Sorry for the confusion. I kept this PR description short as I preferred to link to the conversation where this was decided. I realise that the conversation is very long and a bit twisted. I think the essence is in this comment. In short, I agree with you about ACID behaviours in these cases and not just that, but simply being able to control exactly when a job is enqueued. However, this is very much against what Rails Core wants to encourage; they want exactly the opposite, and since we're going to propose Solid Queue as default for Rails, this change follows that philosophy.

As for your example of activating + sending a welcome email, you can still achieve this even after this PR is merged. For this, you would just need to set:

class WelcomeCustomerJob < ApplicationJob
  self.enqueue_after_transaction_commit = :never
end

or false instead of :never once those changes to simplify this configuration land in Rails.

We're currently running HEY with

config.active_job.enqueue_after_transaction_commit = :always

and we still opt out of this in specific jobs, not because of ACID advantages (since we run Solid Queue in its own DB after all, so we can't take advantage of this because the committed job would see stale data in the app DB if the main app DB transaction didn't commit) but because, in some cases, we need to know whether the job was enqueued successfully or not.

rosa commented 1 month ago

Also

is the kind of "heisenbug" that drives developers to drink...

😆