golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.3k stars 17.38k forks source link

x/net/http2: allow setting MaxConcurrentStreams on client #63196

Open Rohsichan opened 9 months ago

Rohsichan commented 9 months ago

What version of Go are you using (go version)?

$ go version
go version go1.19.13 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GOOS="linux"
GOPATH="/root/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.13"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2198329798=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I downloaded 9M files using 100 goroutines from http2.transport.

What did you expect to see?

There are currently more than 100 active streams in http2. I thought the connection would be newly opened, but it didn't.

What did you see instead?

After checking the http2.transport connection pool, we are reusing a single connection. The netstat -nap check shows that there is 1 connection, rx full, tx full a lot.

The following modifications improve performance by creating connections in units of 20 active streams.

func (cc *ClientConn) State() ClientConnState {
    cc.wmu.Lock()
    maxConcurrent := cc.maxConcurrentStreams
    if !cc.seenSettings {
        maxConcurrent = 0
    }
    cc.wmu.Unlock()

+   // cc.mu.Lock()
+   // defer cc.mu.Unlock()
    return ClientConnState{
        Closed:               cc.closed,
        Closing:              cc.closing || cc.singleUse || cc.doNotReuse || cc.goAway != nil,
        StreamsActive:        len(cc.streams),
        StreamsReserved:      cc.streamsReserved,
        StreamsPending:       cc.pendingRequests,
        LastIdle:             cc.lastIdle,
        MaxConcurrentStreams: maxConcurrent,
    }
}
func (cc *ClientConn) idleStateLocked() (st clientConnIdleState) {
    if cc.singleUse && cc.nextStreamID > 1 {
        return
    }
    var maxConcurrentOkay bool
    if cc.t.StrictMaxConcurrentStreams {
        // We'll tell the caller we can take a new request to
        // prevent the caller from dialing a new TCP
        // connection, but then we'll block later before
        // writing it.
        maxConcurrentOkay = true
    } else {
        maxConcurrentOkay = int64(len(cc.streams)+cc.streamsReserved+1) <= int64(cc.maxConcurrentStreams)
    }

    st.canTakeNewRequest = cc.goAway == nil && !cc.closed && !cc.closing && maxConcurrentOkay &&
        !cc.doNotReuse &&
        int64(cc.nextStreamID)+2*int64(cc.pendingRequests) < math.MaxInt32 &&
        !cc.tooIdleLocked()

+   if st.canTakeNewRequest {
+       if cc.State().StreamsActive > 20 {
+           st.canTakeNewRequest = false
+       }
+   }
    return
}

As the number of active streams increases, http2 becomes slower and slower.

thanm commented 9 months ago

Thanks for the report. Could you please post (via playground link) the code you are using?

thanm commented 9 months ago

@neild @tombergan per owners

Rohsichan commented 9 months ago

The test code is here. https://go.dev/play/p/et4HPzU3wLq?v=goprev

I checked 1 connection on my machine. The response time was 10 seconds in total. In http1.1, the response time is within 2 seconds.

http2

time is 10.555606421

http1.1

time is 1.013303016s
Rohsichan commented 9 months ago

Please let me know if you need any more information.

seankhliao commented 5 months ago

note that the server can set MaxConcurrentStreams, similar performance improvements can be achieved through #47840

Rohsichan commented 5 months ago

I think we need a way to set MaxConcurrentStreams on the client. (http2.transport)

In addition to the go server, I am using a server such as go reverse proxy, nginx, etc. If Golang becomes a client, it is the same.

The MaxReadFrameSize fluctuates and the numbers remain the same.