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

Update Pool from Future to Concurrent::Promises #459

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hms
Copy link
Contributor

@hms hms commented Dec 18, 2024

This PR updates SolidQueue::Pool from the deprecated Concurrent::Future to the more "promising" (sorry) Concurrent::Promises API.

Rational:
The Promises based API is documented as preferred over the Future API. Concurrent::Promises are more performant and deadlock resistant -- Good Things(tm) for SolidQueue.

Given Concurrent::Promises where released 2018, there is little "new" or "not battle tested" risk with this change.

@rosa
Copy link
Member

rosa commented Jan 3, 2025

Sorry for the delay on this one, @hms! With the holidays and being on-call, I haven't had time to look into this, but I'm going to run it for a bit in our app in production from today, before merging it.

@rosa
Copy link
Member

rosa commented Jan 3, 2025

Oops, sorry @hms, would you mind rebasing this branch on the last main? We need the last few changes.

@hms
Copy link
Contributor Author

hms commented Jan 3, 2025

Will do my best to get it done in the 48 hours. Have some serous family stuff up in the air at the moment.

@rosa
Copy link
Member

rosa commented Jan 3, 2025

Oh no! I'm so sorry to hear that 😔 Please don't worry about this; there's absolutely no rush or anything. Wishing you the best.

This PR updates SolidQueue::Pool from the deprecated Concurrent::Future
to the more "promising" (sorry) Concurrent::Promises API.

Rational:
The Promises based API is documented as preferred over the Future API.
Concurrent::Promises are more performant and deadlock resistant --
Good Things(tm) for SolidQueue.

Given Concurrent::Promises where released 2018, there is little "new" or
"not battle tested" risk with this change.
@hms
Copy link
Contributor Author

hms commented Jan 7, 2025

@rosa rebased.

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.

2 participants