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

Get Object from pool with priority #272

Closed
kellpossible opened this issue Sep 26, 2023 · 6 comments
Closed

Get Object from pool with priority #272

kellpossible opened this issue Sep 26, 2023 · 6 comments
Labels
A-core Area: Core / deadpool enhancement New feature or request S-blocked Status: Marked as blocked ❌ on something else.

Comments

@kellpossible
Copy link

I was thinking, maybe it would be kind of nice to be able to specify a priority with a pool.get_with_priority(n), that way I can force some tasks which tend to hog the connections to use them less than others, but when the others don't need them, then they have full access to the database.

@kellpossible
Copy link
Author

kellpossible commented Sep 26, 2023

I suppose, just like a task priority proposal for tokio itself tokio-rs/tokio#3150 (as mentioned tokio-rs/tokio#3242 (comment) ), such a system is likely to want a custom scheduler to suite a specific application, so I guess that might might need to be considered?

@bikeshedder bikeshedder added enhancement New feature or request A-core Area: Core / deadpool labels Sep 26, 2023
@bikeshedder
Copy link
Owner

The synchronization primitive is a tokio::sync::Semaphore which doesn't support an acquire_with_priority method. It's the only fair async semaphore implementation I'm aware of. async-lock isn't even fair at the moment:

It's tempting to change the pool implementation from a passive pool to an active pool with a background worker. That would allow all sorts of fancy things. It would also add a non negligible amount of complexity to the project which I was always very careful not to add.

I have to think about this for a bit... 🤔

@kellpossible
Copy link
Author

I haven't looked into it in detail, but perhaps something like this could be achieved, at least proof of concept using a wrapper around the pool?

@bikeshedder
Copy link
Owner

In theory it should be possible to implement this using a priority queue (binary heap) of Wakers. Though there are many dragons lurking and implementing a proper and well tested synchronization primitive is hard work. While working on deadpool I tried many different semaphore implementations. In the async space the one from tokio is the only one that is usable and even proven correct.

If anyone wants to give this a try I'd be glad to adopt this to deadpool and provide a new function fetching objects with priority from the pool. I'm curious how such synchronization primitive could look like and I'd be happy to assist in reviewing and testing such primitive.

@bikeshedder
Copy link
Owner

Marking this issue blocked as this depends on a feature that isn't available in tokio, yet:

@bikeshedder bikeshedder added the S-blocked Status: Marked as blocked ❌ on something else. label May 7, 2024
@bikeshedder
Copy link
Owner

I'm closing this for the time being as it is blocked by tokio-rs/tokio#3242.

@bikeshedder bikeshedder closed this as not planned Won't fix, can't repro, duplicate, stale Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core / deadpool enhancement New feature or request S-blocked Status: Marked as blocked ❌ on something else.
Projects
None yet
Development

No branches or pull requests

2 participants