Skip to content
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

Active Job: async adapter should always run jobs immediately if immediate set #48626

Merged
merged 1 commit into from Jul 3, 2023

Conversation

ghiculescu
Copy link
Member

This is an internal fix, not user facing. I noticed it while working on #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.

@@ -95,7 +95,7 @@ def enqueue(job, queue_name:)

def enqueue_at(job, timestamp, queue_name:)
delay = timestamp - Time.current.to_f
if delay > 0
if !immediate && delay > 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix.

end
assert_match(/Stopped retrying RetryJob \(Job ID: .*?\) due to a CustomCatchError.*, which reoccurred on \d+ attempts\./, @logger.messages)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this assertion out of the block fixes the backtrace that shows if this test fails. Not strictly related to this PR, but this is how I discovered the issue.


test "in immediate run, enqueue with wait: runs immediately" do
HelloJob.set(wait_until: Date.tomorrow.noon).perform_later "Alex"
assert_match(/Alex/, JobBuffer.last_value)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails without the fix.

…mediate` set

This is an internal fix, not user facing. I noticed it while working on rails#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.
@yahonda yahonda merged commit d867313 into rails:main Jul 3, 2023
9 checks passed
@ghiculescu ghiculescu deleted the active-job-async-test branch July 3, 2023 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants