[net] http2: Allow setting the maximum # of requests through a single connection

3 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
Jan 25, 2023, 1:46:28 AM1/25/23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot has uploaded this change for review.

View Change

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.

Gerrit-Project: net
Gerrit-Branch: master
Gerrit-Change-Id: Ic91271b7de2bd00b7ed7b41762449cf5aea34b5c
Gerrit-Change-Number: 463104
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-MessageType: newchange

Gopher Robot (Gerrit)

unread,
Jan 25, 2023, 1:47:22 AM1/25/23
to Gerrit Bot, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

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.

View Change

    To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: Ic91271b7de2bd00b7ed7b41762449cf5aea34b5c
    Gerrit-Change-Number: 463104
    Gerrit-PatchSet: 1
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Comment-Date: Wed, 25 Jan 2023 06:47:18 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Damien Neil (Gerrit)

    unread,
    Jan 25, 2023, 5:37:01 PM1/25/23
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Tom Bergan, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Tom Bergan.

    View Change

    1 comment:

    To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: Ic91271b7de2bd00b7ed7b41762449cf5aea34b5c
    Gerrit-Change-Number: 463104
    Gerrit-PatchSet: 1
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Tom Bergan <tomb...@google.com>
    Gerrit-Comment-Date: Wed, 25 Jan 2023 22:36:57 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Seiya Nuta (Gerrit)

    unread,
    Jan 26, 2023, 1:19:06 PM1/26/23
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Damien Neil, Tom Bergan, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Damien Neil, Tom Bergan.

    View Change

    1 comment:

    • File http2/transport.go:

      • 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.

    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: Ic91271b7de2bd00b7ed7b41762449cf5aea34b5c
    Gerrit-Change-Number: 463104
    Gerrit-PatchSet: 1
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Seiya Nuta <seiy...@gmail.com>
    Gerrit-Attention: Damien Neil <dn...@google.com>
    Gerrit-Attention: Tom Bergan <tomb...@google.com>
    Gerrit-Comment-Date: Thu, 26 Jan 2023 10:15:42 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Damien Neil <dn...@google.com>
    Gerrit-MessageType: comment

    Gopher Robot (Gerrit)

    unread,
    May 29, 2023, 1:49:47 AM5/29/23
    to Gerrit Bot, goph...@pubsubhelper.golang.org, Seiya Nuta, Damien Neil, Tom Bergan, golang-co...@googlegroups.com

    Gopher Robot abandoned this change.

    View Change

    Abandoned GitHub PR golang/net#161 has been closed.

    To view, visit change 463104. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: abandon
    Reply all
    Reply to author
    Forward
    0 new messages