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

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

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

Comments

Projects
None yet
5 participants
@gopherbot
Copy link

commented Dec 2, 2014

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

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2014

Comment 1:

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

@bradfitz

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented Dec 2, 2014

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!
@tombergan

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2016

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

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

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

@bradfitz bradfitz closed this Oct 17, 2016

@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.
You can’t perform that action at this time.