Gerrit Bot has uploaded this change for review.
http2: Allow setting the maximum # of requests through a single connection
Some http2 server implementaion have the limit of # of requests through a single connection. This options allows setting the maximum times of reuse so that requests won't be rejected due to the limit.
Change-Id: Ic91271b7de2bd00b7ed7b41762449cf5aea34b5c
GitHub-Last-Rev: e3ef5bef76168c79b23c70a84110fe40b695360e
GitHub-Pull-Request: golang/net#161
---
M http2/transport.go
M http2/transport_test.go
2 files changed, 167 insertions(+), 19 deletions(-)
diff --git a/http2/transport.go b/http2/transport.go
index b43ec10..98754fa 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -146,6 +146,10 @@
// waiting for their turn.
StrictMaxConcurrentStreams bool
+ // MaxConnectionReuseCount is the maximum number of times a connection
+ // can be reused before it is closed. If zero, no limit is enforced.
+ MaxConnectionReuseCount uint32
+
// ReadIdleTimeout is the timeout after which a health check using ping
// frame will be carried out if no frame is received on the connection.
// Note that a ping response will is considered a received frame, so if
@@ -324,11 +328,12 @@
lastActive time.Time
lastIdle time.Time // time last idle
// Settings from peer: (also guarded by wmu)
- maxFrameSize uint32
- maxConcurrentStreams uint32
- peerMaxHeaderListSize uint64
- peerMaxHeaderTableSize uint32
- initialWindowSize uint32
+ maxFrameSize uint32
+ maxConcurrentStreams uint32
+ peerMaxHeaderListSize uint64
+ peerMaxHeaderTableSize uint32
+ initialWindowSize uint32
+ MaxConnectionReuseCount uint32
// reqHeaderMu is a 1-element semaphore channel controlling access to sending new requests.
// Write to reqHeaderMu to lock it, read from it to unlock.
@@ -733,19 +738,20 @@
func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, error) {
cc := &ClientConn{
- t: t,
- tconn: c,
- readerDone: make(chan struct{}),
- nextStreamID: 1,
- maxFrameSize: 16 << 10, // spec default
- initialWindowSize: 65535, // spec default
- maxConcurrentStreams: initialMaxConcurrentStreams, // "infinite", per spec. Use a smaller value until we have received server settings.
- peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead.
- streams: make(map[uint32]*clientStream),
- singleUse: singleUse,
- wantSettingsAck: true,
- pings: make(map[[8]byte]chan struct{}),
- reqHeaderMu: make(chan struct{}, 1),
+ t: t,
+ tconn: c,
+ readerDone: make(chan struct{}),
+ nextStreamID: 1,
+ maxFrameSize: 16 << 10, // spec default
+ initialWindowSize: 65535, // spec default
+ maxConcurrentStreams: initialMaxConcurrentStreams, // "infinite", per spec. Use a smaller value until we have received server settings.
+ peerMaxHeaderListSize: 0xffffffffffffffff, // "infinite", per spec. Use 2^64-1 instead.
+ streams: make(map[uint32]*clientStream),
+ singleUse: singleUse,
+ wantSettingsAck: true,
+ pings: make(map[[8]byte]chan struct{}),
+ reqHeaderMu: make(chan struct{}, 1),
+ MaxConnectionReuseCount: t.MaxConnectionReuseCount,
}
if d := t.idleConnTimeout(); d != 0 {
cc.idleTimeout = d
@@ -969,9 +975,14 @@
maxConcurrentOkay = int64(len(cc.streams)+cc.streamsReserved+1) <= int64(cc.maxConcurrentStreams)
}
+ streamIDUpperBound := int64(math.MaxInt32)
+ if cc.MaxConnectionReuseCount > 0 {
+ streamIDUpperBound = int64(cc.MaxConnectionReuseCount) * 2
+ }
+
st.canTakeNewRequest = cc.goAway == nil && !cc.closed && !cc.closing && maxConcurrentOkay &&
!cc.doNotReuse &&
- int64(cc.nextStreamID)+2*int64(cc.pendingRequests) < math.MaxInt32 &&
+ int64(cc.nextStreamID)+2*int64(cc.pendingRequests) < streamIDUpperBound &&
!cc.tooIdleLocked()
return
}
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 5adef42..972486e 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4316,6 +4316,130 @@
ct.run()
}
+func TestMaxConnectionReuseCount(t *testing.T) {
+ const maxConnectionReuseCount = 10
+
+ errs := make(chan error, maxConnectionReuseCount)
+ dialedTwice := false
+ clientDone := make(chan struct{})
+
+ ct := newClientTester(t)
+ ct.tr.MaxConnectionReuseCount = maxConnectionReuseCount
+ ct.client = func() error {
+ var allWg sync.WaitGroup
+ var firstConnWg sync.WaitGroup
+ allWg.Add(maxConnectionReuseCount + 1)
+ firstConnWg.Add(maxConnectionReuseCount)
+ defer func() {
+ allWg.Wait()
+ close(clientDone)
+ ct.cc.(*net.TCPConn).CloseWrite()
+
+ // Check #1: All requests should have succeeded.
+ close(errs)
+ for err := range errs {
+ if err != nil {
+ t.Error(err)
+ }
+ }
+
+ // Check #2: The connection is not reused after maxConnectionReuseCount.
+ if !dialedTwice {
+ t.Error("expected to dial twice because of reaching MaxConnectionReuseCount")
+ }
+ }()
+
+ // Make requests maxConnectionReuseCount times. All requests should be served by the same connection.
+ greet := make(chan struct{})
+ for i := 0; i < maxConnectionReuseCount; i++ {
+ go func(i int) {
+ defer allWg.Done()
+ defer firstConnWg.Done()
+
+ if i > 0 {
+ <-greet
+ }
+
+ req, _ := http.NewRequest("HEAD", "https://dummy.tld/1", nil)
+ res, err := ct.tr.RoundTrip(req)
+ if err != nil {
+ errs <- fmt.Errorf("RoundTrip(%d): unexpected error: %v", i, err)
+ return
+ }
+ ioutil.ReadAll(res.Body)
+ res.Body.Close()
+
+ if i == 0 {
+ close(greet)
+ }
+ }(i)
+ }
+
+ // Spawn another goroutine to make another request after reaching maxConnectionReuseCount.
+ go func() {
+ defer allWg.Done()
+ firstConnWg.Wait()
+
+ // This request should fail with "only one dial allowed in test mode", exploting the fact that
+ // the client tester doesn't support dialing more than once.
+ req, _ := http.NewRequest("HEAD", "https://dummy.tld/2", nil)
+ res, err := ct.tr.RoundTrip(req)
+ if err != nil {
+ if err.Error() == "only one dial allowed in test mode" {
+ dialedTwice = true
+ } else {
+ errs <- fmt.Errorf("RoundTrip(last): unexpected error: %v", err)
+ }
+ return
+ }
+ ioutil.ReadAll(res.Body)
+ res.Body.Close()
+ }()
+
+ return nil
+ }
+ ct.server = func() error {
+ ct.greet()
+
+ var buf bytes.Buffer
+ enc := hpack.NewEncoder(&buf)
+ for {
+ f, err := ct.fr.ReadFrame()
+ if err != nil {
+ select {
+ case <-clientDone:
+ // If the client's done, it will have reported any errors on its side.
+ return nil
+ default:
+ return err
+ }
+ }
+
+ switch f := f.(type) {
+ case *WindowUpdateFrame, *SettingsFrame:
+ continue
+ case *HeadersFrame:
+ if !f.HeadersEnded() {
+ return fmt.Errorf("headers should have END_HEADERS be ended: %v", f)
+ }
+
+ buf.Reset()
+ enc.WriteField(hpack.HeaderField{Name: ":status", Value: "204"})
+ ct.fr.WriteHeaders(HeadersFrameParam{
+ StreamID: f.StreamID,
+ EndHeaders: true,
+ EndStream: true,
+ BlockFragment: buf.Bytes(),
+ })
+ case *DataFrame:
+ default:
+ return fmt.Errorf("Unexpected client frame %v", f)
+ }
+ }
+ }
+ ct.run()
+}
+
func TestTransportMaxDecoderHeaderTableSize(t *testing.T) {
ct := newClientTester(t)
var reqSize, resSize uint32 = 8192, 16384
To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Attention is currently required from: Tom Bergan.
1 comment:
File http2/transport.go:
Patch Set #1, Line 151: MaxConnectionReuseCount uint32
Thanks for the contribution!
Changes which add new APIs (including new configuration knobs like this) need to start with an approved proposal:
https://github.com/golang/proposal
To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Damien Neil, Tom Bergan.
1 comment:
File http2/transport.go:
Patch Set #1, Line 151: MaxConnectionReuseCount uint32
Thanks for the contribution! […]
Thank you for the advice! Will write a proposal first :)
To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.