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.

Sign up for GitHub

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

net/http: Transport: add ConnState callback for http.Transport #9194

Closed
gopherbot opened this issue Dec 2, 2014 · 5 comments
Closed

net/http: Transport: add ConnState callback for http.Transport #9194

gopherbot opened this issue Dec 2, 2014 · 5 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@gopherbot
Copy link

by sasha.klizhentas@mailgunhq.com:

What does 'go version' print?

go version go1.3.1 linux/amd64

What steps reproduce the problem?

It would be useful to have ConnState handler for http.Transports in the same way we have
it for Server:

// ConnState specifies an optional callback function that is
// called when a client connection changes state. See the
// ConnState type and associated constants for details.
ConnState func(net.Conn, ConnState)

I think it would be possible to re-use the same data structure ConnState for connections
to clients in the Transport.

The use cases include:

* Limiting the connections to the clients
* Cooperation with back-end servers in case of reverse proxy (e.g. draining off
connections)
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-none.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 2, 2014

Comment 2:

As discussed last night, I'm not against this, but it's just lower priority than the
Server because at least you're in control of the client more.  And I'm not entirely
convinced (without thinking about it much) that the same states apply.
Please write a proposal (here works) about which states apply and what they mean for the
client.

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

Comment 3 by sasha.klizhentas@mailgunhq.com:

Sure!
Here are the possible states:
1.StateNew when new connection is created in the transport
http://golang.org/src/pkg/net/http/transport.go?s=472:472#L472
2. StateActive state when conn wrote request headers:
http://golang.org/src/pkg/net/http/transport.go?s=891:891#L891
I think this would need cooperation with request.write to tell us when the request
headers were written
3. StateClosed state when persistent connection is closed
c.setState(pc, StateClosed)
http://golang.org/src/pkg/net/http/transport.go?s=1074:1074#L1074
4. StateIdle state when connection is returned to the pool
http://golang.org/src/pkg/net/http/transport.go?s=329:329#L329
Looks like Hijacked state is not relevant in case of transports.
Let me know what you think!

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@bradfitz bradfitz mentioned this issue Jun 11, 2015
Closed
@tombergan
Copy link
Contributor

Looking through old bugs ... this is (almost) obsoleted by net/http/httptrace:

  1. StateNew is GotConn with reused=false
  2. StateActive is WroteHeaders
  3. StateClosed is missing
  4. StateIdle is PutIdleConn

If StateClosed is still desired, it may be possible to add CloseConn. Otherwise, we can close this issue.

@bradfitz
Copy link
Contributor

@tombergan, thanks. Yeah, it seems like we can close this one. People can open separate bugs to track httptrace shortcomings.

@golang golang locked and limited conversation to collaborators Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@bradfitz @rsc @ianlancetaylor @gopherbot @tombergan