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

Allow library consumer to drain a connection #2307

Closed

Conversation

nightkr
Copy link

@nightkr nightkr commented Oct 23, 2020

A draining connection will no longer be used for new requests, but allow existing requests to finish. As discussed on Discord earlier (https://discordapp.com/channels/500028886025895936/670880858630258689/768499073644756992).

This feels like the wrong layer to add this on, since Connected is otherwise unaware of any connection pooling going on. It also feels like a smell to me that this will break if the pool starts refreshing Connection::connected() for any reason. The more appropriate place would probably be to store it on the PoolClient, but that's getting constructed and destroyed all over the place, so I'm not really sure about what guarantees we have there about the object identity over time.

This is also missing a bunch of docs, I just want to find a sound approach before getting into that.

A draining connection will no longer be used for new requests, but allow
existing requests to finish.
@nightkr
Copy link
Author

nightkr commented Oct 23, 2020

This would be used something like this:

    let response = client.request(...).await?;
    if response.status() == StatusCode::SERVICE_UNAVAILABLE {
        // Server is dying, try to reconnect
        response.extensions().get::<PoolHandle>().unwrap().drain();
    }

@seanmonstar
Copy link
Member

Figuring out a way to drain the pool would be cool. I think #2605 is similar. We've moving the pool as part of 1.0 to hyper-util, so it would be more acceptable to experiment there.

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