-
Notifications
You must be signed in to change notification settings - Fork 134
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
Remove deprecated adapter specific deferred enqueueing of jobs #412
Conversation
@@ -8,10 +8,6 @@ module QueueAdapters | |||
# | |||
# Rails.application.config.active_job.queue_adapter = :solid_queue | |||
class SolidQueueAdapter | |||
def enqueue_after_transaction_commit? | |||
true | |||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to leave this in place, unfortunately, because Rails 7.2 relied on this being defined in adapters, so removing it will break for people using Rails < 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
README.md
Outdated
@@ -350,24 +350,13 @@ to your `puma.rb` configuration. | |||
## Jobs and transactional integrity | |||
:warning: Having your jobs in the same ACID-compliant database as your application data enables a powerful yet sharp tool: taking advantage of transactional integrity to ensure some action in your app is not committed unless your job is also committed and viceversa, and ensuring that your job won't be enqueued until the transaction within which you're enqueuing it is committed. This can be very powerful and useful, but it can also backfire if you base some of your logic on this behaviour, and in the future, you move to another active job backend, or if you simply move Solid Queue to its own database, and suddenly the behaviour changes under you. | |||
|
|||
Because this can be quite tricky and many people shouldn't need to worry about it, by default Solid Queue is configured in a different database as the main app, **job enqueuing is deferred until any ongoing transaction is committed** thanks to Active Job's built-in capability to do this. This means that even if you run Solid Queue in the same DB as your app, you won't be taking advantage of this transactional integrity. | |||
An option which doesn't rely on this transactional integrity and which Active Job provides is to defer the enqueueing of a job inside an Active Record transaction until that transaction successfully commits. This option can be set via the [`enqueue_after_transaction_commit`](https://edgeapi.rubyonrails.org/classes/ActiveJob/Enqueuing.html#method-c-enqueue_after_transaction_commit) class method on the job level and is by default disabled. Either it can be enabled for individual jobs or for all jobs through `ApplicationJob`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some parts here and below that are still true and we should keep.
An option which doesn't rely on this transactional integrity and which Active Job provides is to defer the enqueueing of a job inside an Active Record transaction until that transaction successfully commits. This option can be set via the [`enqueue_after_transaction_commit`](https://edgeapi.rubyonrails.org/classes/ActiveJob/Enqueuing.html#method-c-enqueue_after_transaction_commit) class method on the job level and is by default disabled. Either it can be enabled for individual jobs or for all jobs through `ApplicationJob`: | |
Because this can be quite tricky and many people shouldn't need to worry about it, by default Solid Queue is configured in a different database as the main app. | |
If you want to use Solid Queue in the same DB as your app but still not rely on transactional integrity by default, from Rails 8, Active Job provides an option to defer the enqueueing of a job inside an Active Record transaction until all transactions are successfully committed. This option can be set via the [`enqueue_after_transaction_commit`](https://edgeapi.rubyonrails.org/classes/ActiveJob/Enqueuing.html#method-c-enqueue_after_transaction_commit) class method on the job level and is by default disabled. Either it can be enabled for individual jobs or for all jobs through `ApplicationJob`: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I adapted your suggestion a little bit: in your suggestion it sounds like this option should be mainly used if you want to use one DB and not rely on transactional integrity. But the option is also useful if you use two separate databases and want this behaviour of deferring the enqueueing of jobs. So I kept the explanation of the option more generic but added a sentence after it that it also can be useful if you have a single DB and do not want to rely on transactional integrity. If you are a of a different opinion, I can change it again.
class ApplicationJob < ActiveJob::Base | ||
self.enqueue_after_transaction_commit = true | ||
end | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep the two bits that are still true here, that is, this part:
If you don't set this option but still want to make sure you're not inadvertently on transactional integrity, you can make sure that:
- Your jobs relying on specific data are always enqueued on
after_commit
callbacks or otherwise from a place where you're certain that whatever data the job will use has been committed to the database before the job is enqueued.- Or, you configure a different database for Solid Queue, even if it's the same as your app, ensuring that a different connection on the thread handling requests or running jobs for your app will be used to enqueue jobs. For example:
class ApplicationRecord < ActiveRecord::Base self.abstract_class = true connects_to database: { writing: :primary, reading: :replica }config.solid_queue.connects_to = { database: { writing: :primary, reading: :replica } }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added them back again.
805ef0c
to
3836cf1
Compare
README.md
Outdated
@@ -348,13 +348,19 @@ to your `puma.rb` configuration. | |||
|
|||
|
|||
## Jobs and transactional integrity | |||
:warning: Having your jobs in the same ACID-compliant database as your application data enables a powerful yet sharp tool: taking advantage of transactional integrity to ensure some action in your app is not committed unless your job is also committed and viceversa, and ensuring that your job won't be enqueued until the transaction within which you're enqueuing it is committed. This can be very powerful and useful, but it can also backfire if you base some of your logic on this behaviour, and in the future, you move to another active job backend, or if you simply move Solid Queue to its own database, and suddenly the behaviour changes under you. | |||
:warning: Having your jobs in the same ACID-compliant database as your application data enables a powerful yet sharp tool: taking advantage of transactional integrity to ensure some action in your app is not committed unless your job is also committed and vice versa, and ensuring that your job won't be enqueued until the transaction within which you're enqueuing it is committed. This can be very powerful and useful, but it can also backfire if you base some of your logic on this behaviour, and in the future, you move to another active job backend, or if you extract the tables related to your active job backend to their own database, and suddenly the behaviour changes under you. Because this can be quite tricky and many people shouldn't need to worry about it, by default Solid Queue is configured in a different database as the main app. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good except the change from
if you simply move Solid Queue to its own database
to
if you extract the tables related to your active job backend to their own database.
Could you restore the previous one? The second reads a bit convoluted, because this is about Solid Queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I restored it
3836cf1
to
2c2ea70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
It is not possible anymore for an Active Job adapter to configure the deferred enqueueing of jobs via
enqueue_after_transaction_commit?
. This PR removes the code and updates the documentation regarding this topic.Resolves #409