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

Jump to bottom

x/net/http2: support graceful close of client connection #17292

Closed
rs opened this issue Sep 30, 2016 · 7 comments
Closed

x/net/http2: support graceful close of client connection #17292

rs opened this issue Sep 30, 2016 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@rs
Copy link
Contributor

rs commented Sep 30, 2016 •
edited

I'm implementing my own http2.ConnPool with some health checking done using the new Ping() method. My problem is that my pool doesn't have the capability to dispose client connections because there is no way to cleanly close a client connection other than keeping it's net.Conn on the side and abruptly close it.

What about adding a Close() method http2.ClientConn that would send a GOAWAY frame and wait for the in-flight streams to complete?

Implementation proposal: https://go-review.googlesource.com/#/c/30076/
@gopherbot
Copy link

CL https://golang.org/cl/30076 mentions this issue.

@quentinmit quentinmit added this to the Unreleased milestone Oct 4, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 4, 2016
@rs
Copy link
Contributor Author

rs commented Mar 16, 2017

This PR has been sitting for month now. Is there something to be done to resume the review?

@bradfitz
Copy link
Contributor

Sorry, that CL (its tests mostly) exhausted me earlier and then I forgot about it during Go 1.8 chaos.

Its tests still need a bit of clarity about what they're trying to do. The test code itself might be fine now, but it's hard to tell without comments saying what the intent is.

It looks like the non-test code is still fine.

/cc @tombergan

@rs
Copy link
Contributor Author

rs commented Mar 16, 2017

I am sorry to hear tests exhausted you :)
I added some comments to clarify the tests.

@rs rs mentioned this issue Aug 15, 2017
Open
@tombergan tombergan modified the milestones: Go1.10, Unreleased Aug 16, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Jan 10, 2018
@rs
Copy link
Contributor Author

rs commented Jul 9, 2018

Thank you @bradfitz

@gaorong
Copy link

gaorong commented Apr 24, 2019

I'm implementing my own http2.ConnPool with some health checking done using the new Ping() method.

@rs sorry to bother you, I want to find a proper way to use Ping() method, but can hardly find some valuable reference, would you mind sharing your implement of ConnPolol? thanks

@rs
Copy link
Contributor Author

rs commented Apr 24, 2019

Our final implementation ended up not using ping.

@golang golang locked and limited conversation to collaborators Apr 23, 2020
@rsc rsc unassigned rs Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
6 participants
@bradfitz @rs @quentinmit @gopherbot @gaorong @tombergan