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

Implement perform_all_later via enqueue_all #93

Merged
merged 8 commits into from
Dec 26, 2023
Merged

Implement perform_all_later via enqueue_all #93

merged 8 commits into from
Dec 26, 2023

Conversation

rosa
Copy link
Member

@rosa rosa commented Dec 26, 2023

This feature was introduced in Active Job in rails/rails#46603.

It wasn't properly supported until now in Solid Queue, as the adapter didn't respond to #enqueue_all, which means all jobs passed over to perform_all_later were enqueued one by one. With this pull request, we add proper support, minimizing the number of queries we have to perform. We still need to enqueue concurrency-limited jobs one by one, just like when moving them from scheduled to ready, but for scheduled jobs and ready-to-be-run jobs, we can insert these via insert_all.

With this implementation, everything is run within the same transaction and right now, there's no batching. All jobs passed to enqueue_all are inserted at once. This will be problematic if the batch is very large, causing problems with locking and with replica lag. However, this matches the behaviour of other adapters, leaving the batching up to the user of perform_all_later.

rosa added 8 commits December 26, 2023 15:40
Supporting concurrency-limited, scheduled and immediate jobs.
It's not really necessary, and will allow for some DRY. Just let Rails
set it.
From now, used only when moving scheduled executions to ready executions.

Also, change `assume_attributes_from_job` to `assumes_attributes_from_job`,
3rd person singular, to be consistent with other similar methods.
Taking advantage of the new methods created in previous commits.
This allows even further DRYing and simplification, as we can unify the
dispatching done when moving a scheduled job batch to ready, having to
deal with concurrency-limited jobs, and the same thing when enqueuing
multiple jobs.
Only for those enqueued successfully, that's it.
@rosa rosa merged commit d903c82 into main Dec 26, 2023
3 checks passed
@rosa rosa deleted the multi-enqueue branch December 26, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant