-
Notifications
You must be signed in to change notification settings - Fork 132
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
Set enqueue_after_transaction_commit
to true
and don't allow changing it
#301
Set enqueue_after_transaction_commit
to true
and don't allow changing it
#301
Conversation
acdfb0f
to
0cf64b5
Compare
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... |
@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 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. |
Also
😆 |
0cf64b5
to
84ab1a5
Compare
…ging it See the discussion in rails/rails#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.
84ab1a5
to
60c132e
Compare
See the discussion at rails/rails#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.