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

Add support for Pool::get with timeout #9

Closed
bikeshedder opened this issue Dec 4, 2019 · 5 comments
Closed

Add support for Pool::get with timeout #9

bikeshedder opened this issue Dec 4, 2019 · 5 comments
Labels
A-core Area: Core / deadpool enhancement New feature or request

Comments

@bikeshedder
Copy link
Owner

I currently wonder if it should be part of the core of deadpool:

Adding a timeout to the Pool::get function should be as simple as wrapping the call in one of the following two:

There is little to be gained when using Pool::get_timeout() over timeout(Pool::get()), is there?

@bikeshedder
Copy link
Owner Author

I currently wonder wether I should add a get_timeout function which allows specifying multiple timeouts:

async fn get_timeout(&self,
    create_timeout: Option(Duration),
    wait_timeout: Option(Duration),
    recycle_timeout: Option(Duration))
  • create_timeout is the maximum time spent when creating a new connection
  • wait_timeout is the maximum time spent when waiting for a connection to become available in the pool.
  • recycle_timeout is the maximum time spent recycling an existing connection

For most applications I guess wait_timeout and an overall timeout would suffice so I wonder if create_timeout and recycle_timeout is even needed.

@nihiluis
Copy link

nihiluis commented Feb 5, 2020

I think this feature was only added to managed pools?

I think this would be beneficial for unmanaged ones too. Or do you not think so? If so, why?

@bikeshedder
Copy link
Owner Author

Unmanaged pools don't recycle and create new objects. Thus only wait_timeout makes any sense. Right now this can already be achieved by calling tokio::time::timeout(std::time::Duration::from_secs(5), pool.get()). It might make sense to add this as a configuration option to the pool nonetheless. It will require the pool.get() method to return a Result<T, PoolError> instead of T.

Feel free to open a new issue so we can discuss this.

@ansjsun
Copy link

ansjsun commented Jun 5, 2020

my lib not use tokio . use async-st, when add this config
it has err:

thread 'main' panicked at 'there is no timer running, must be called from the context of Tokio runtime', /Users/sunjian11/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.2.21/src/time/driver/handle.rs:24:9

@bikeshedder

@bikeshedder
Copy link
Owner Author

That's a bummer. I thought tokio::time::timeout was runtime agnostic. It seams that this is not the case.

Could you please open a new issue regarding this issue? I don't use async-std myself at the moment that's why I didn't notice that.

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
Projects
None yet
Development

No branches or pull requests

3 participants