Fix: https://github.com/rails/rails/pull/51426#issuecomment-2042611790
`perform_later` is supposed to return the Job instance on success,
and `false` on error.
When the `enqueue` is automatically delayed, it's of course impossible
to predict if the actual queueing will succeed, but for backward compatibility
reasons, it's best to assume it will.
If necessary, you can hold onto the job instance and check for
`#successfully_enqueued?` after the transaction has completed.
A fairly common mistake with Rails is to enqueue a job from inside a
transaction, with a record as argumemnt, which then lead to a RecordNotFound
error when picked up by the queue.
This is even one of the arguments advanced for job runners backed by the
database such as `solid_queue`, `delayed_job` or `good_job`.
But relying on this is undesirable in my opinion as it makes the Active Job
abstraction leaky, and if in the future you need to migrate to another backend
or even just move the queue to a separate database, you may experience a lot of
race conditions of the sort.
To resolve this problem globally, we can make Active Job optionally transaction
aware, and automatically defer job queueing to `after_commit`.
Co-Authored-By: Cristian Bica <cristian.bica@gmail.com>
Since #50715, there are cases where `ActiveJob::Arguments` is used
but not required anymore in the test environment. This could be a
test for a custom argument serializer.
One solution could be to add a require "active_job/arguments" in
all the usages, but that isn't a convention we follow in Rails.
Instead, let's register the autoload for `ActiveJob::Arguments` in
`active_job.rb` so that it's always available when needed.
Similarly to Action Mailbox, these tests didn't pass because the skip
patch was included before ActiveSupport::TestCase was defined. Moving
the patch to the bottom of the file fixes the issue.
Once the skip patch was fixed, all of the skips were due to differences
in adapters which aren't really test skips as much as tests that should
not ever run against those adapters.
https://github.com/rails/rails/pull/50090 broke serialization of String
subclasses that don't have serializers, like ActiveSupport::SafeBuffer.
Co-authored-by: John Hawthorn <john@hawthorn.email>
We had a few cases of tests being skipped accidentally on CI
hence not bein ran for a long time.
Skipping make sense when running the test suite locally, e.g.
you may not have Redis or some other dependency running.
But on CI, a test not being ran should be considered an error.
It's possible since Rails 6 (3ea2857943dc294d7809930b4cc5b318b9c39577) to let the framework create Event objects, but the guides and docs weren't updated to lead with this example.
Manually instantiating an Event doesn't record CPU time and allocations, I've seen it more than once that people copy-pasting the example code get confused about these stats returning 0. The tests here show that - just like the apps I've worked on - the old pattern keeps getting copy-pasted.
There are assertions that expected/actual arguments are passed in the
reversed order by mistake. Enabling the LiteralAsActualArgument rule
prevents this mistake from happening.
The existing tests were auto-corrected by rubocop with a bit of
indentation adjustment.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Durations that were round-tripped through ActiveJob::Arguments.serialize
would appear fine at a first glance, but trying to perform
duration-math on them would fail:
```
irb(main):001:0> d = ActiveJob::Arguments.deserialize(ActiveJob::Arguments.serialize([1.year]))[0]
=> 1 year
irb(main):002:0> d + 1.day
activesupport-6.1.4.4/lib/active_support/duration.rb:242:in `+': undefined method `merge' for [[:years, 1]]:Array (NoMethodError)
```
This is an internal fix, not user facing. I noticed it while working on https://github.com/rails/rails/pull/48585.
The `async` adapter has an `immediate` option, which should only be used in tests. This option should tell the adapter to run jobs inline. This works correctly with `perform_later`, but it does not work with `enqueue_at`, which is what other internal mechanisms such as `retry_job` use.
This PR fixes this bug.
This is an internal fix, not user facing.
Currently the `LoggingJob` does not accept more than one argument. But there's [a few tests](f46d3452ae/activejob/test/cases/test_helper_test.rb (L518-L537)) that call it with multiple arguments and assert that it is queued correctly. Those tests pass because the job is not performed, but if the job was performed, they'd fail.
This PR just fixes `LoggingJob` to accept a splat of arguments, and adds a test to ensure that it works correctly.
The testing guide for Active Job currently implies that when you queue a job it will be performed.
This isn't true; by default jobs are enqueued, not performed.
This PR fleshes out the docs a bit to show both examples, and adds a test to confirm the default behaviour.
This has utility for gems/modules included on jobs, which can tie into this behaviour and run something when a job fails.
after_discard respects the existing retry behaviour, but will run even if a retried exception is handled in a block.
In this test case we only check if the third child's queue adpter is nil or not. We should instead check if the queue adapter is the same as base adapter to ensure that it did not change when we changed qdapters for other jobs.
50f0c0f064/activejob/test/cases/queue_adapter_test.rb (L44-L52)
This gives queue adapters more freedom to name and organize their code.
For example, if `FancyQueue` wants to have their adapter at
`FancyQueue::ActiveJobAdapter`, the name would be `active_job` before this
change. After this change, they can implement `queue_adapter_name` to return
`fancy_queue`.
Before this change, setting a module as queue adapter would result in
`queue_adapter_name` being `"module"`. This PR fixes the name extraction logic
to handle module or class queue adapters.
The display_name method is used by delayed job to log information
about a certain job, including failure messages. Whenever a job class
is moved or deleted, the instances still scheduled cannot be
constantized anymore, causing display_name and hence the log method to
raise an exception. In certain cases, e.g. when logging happens in a
rescue block, this may terminate the entire delayed job worker. With
the failsafe method, the worker handles failed jobs gracefully and
continues work, all with appropriate log output.
Sidekiq has a useful optimisation called `push_bulk` that enqueues many jobs at
once, eliminating the repeated Redis roundtrips. However, this feature is not
exposed through Active Job, so it only works for `Sidekiq::Worker` jobs. This
adds a barrier to Active Job adoption for apps that rely on this feature. It
also makes it harder for other queue adapters to implement similar
functionality, as they then have to take care of serialization, callbacks, etc.
themselves.
This commit adds `ActiveJob.perform_all_later(<job1>, <job2>)`, backed by
Sidekiq's `push_bulk` and with a fallback to enqueuing serially if the queue
adapter does not support bulk enqueue.
The performance benefit for 1000 jobs can be more than an order of magnitude:
| Enqueue type | Serial time (ms) | Bulk time (ms) | Speedup |
| ------------------ | ---------------- | -------------- | ------- |
| Raw Sidekiq | 2661 | 119 | 22x |
| Active Job Sidekiq | 2853 | 208 | 14x |
(Measured in a simple test app in our production environment.)
Instrumentation for perform_all_later uses a new event `enqueue_all.active_job`
This commit adds `ActiveJob.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `activejob/lib` with
`ActiveJob.deprecator`.
Additionally, this commit adds `ActiveJob.deprecator` to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
This commit also removes a defunct `ActiveSupport::Deprecation.silence`
call that was added in 9eb4b4ed01f893c2fc7ff3ee4f99301dec7ea3c2 but not
removed when the deprecation was completed in
10bd5e59c3291bb2034869e97cca30634a559afb.
I noticed a difference in behavior when inspecting `ActiveJob::Core#scheduled_at` in non-test environments vs when inspecting in while executing tests.
Prior to this change, `ActiveJob::TestHelper` would set `ActiveJob::Core#scheduled_at` to a `Time` object in tests,
however this attribute was set to a `Float` objects in non-test environments:
<d1aa6af5cc/activejob/lib/active_job/core.rb (L161)>
This change ensures parity in all runtime environments.
Previously, BigDecimal was listed as not needing a serializer. However,
when used with an adapter storing the job arguments as JSON, it would get
serialized as a simple String, resulting in deserialization also producing
a String (instead of a BigDecimal).
By using a serializer, we ensure the round trip is safe.
During upgrade deployments of applications with multiple replicas making use of
BigDecimal job arguments with a queue adapter serializing to JSON, there exists
a possible race condition, whereby a "new" replica enqueues a job with an
argument serialized using `BigDecimalSerializer`, and an "old" replica fails to
deserialize it (as it does not have `BigDecimalSerializer`).
Therefore, to ensure safe upgrades, serialization will not use
`BigDecimalSerializer` until `config.active_job.use_big_decimal_serializer` is
enabled, which can be done safely after successful deployment of Rails 7.1.
This option will be removed in Rails 7.2, when it will become the default.
In other log messages like perform/performed and ActiveJob error logging, the job ID and exception message were already included, eg:
Error performing TestFailureJob (Job ID: d70ad13e-e58b-409c-a8cc-e0447fc792b5) from Resque(default) in 1446.56ms: RuntimeError (Error Message):
But log message related to retry/discard behavior from ActiveJob::Exceptions did not include the Job ID or exception message. We now include them, in a consistent format with other existing messages.
Job ID is especially useful because, while other ActiveJob-related log messages get the Job ID via tagged logging, retry-related log messages do not currently end up tagged with Job ID. And it's really useful to use Job ID to be able to collect all the log lines related to a particular job, put the error messages together with the retry and subsequent perform messages.
We also include job.executions in the enqueue_retry log message because it was available and useful, to know if this is the 1st retry or 2nd or whatever. Previously job.executions was included in retry_stopped logging, but not enqueue_retry.
