feat: make client to wait when no free connections

[chanjarster]

Add a new field MaxConnWaitTimeout on Client.

• If MaxConnWaitTimeout == 0, when all connections are busy, will return ErrNoFreeConns immediately (just like what it already does).
• If MaxConnWaitTimeout > 0, when all connections are busy, Client will wait at most MaxConnWaitTimeout for a chance to get a connection, if Client failed, will return ErrNoFreeConns

[erikdubbelboer]

First thing I notice is a lot of potential heap allocations which can slow things down. Did you try doing any benchmarks or checking the number of allocations with a test like this?:

fasthttp/allocation_test.go

Lines 40 to 72 in 38aa88a

	func TestAllocationClient(t *testing.T) {
	ln, err := net.Listen("tcp4", "127.0.0.1:0")
	if err != nil {
	t.Fatalf("cannot listen: %s", err)
	}
	defer ln.Close()
	s := &Server{
	Handler: func(ctx *RequestCtx) {
	},
	}
	go s.Serve(ln) //nolint:errcheck
	c := &Client{}
	url := "http://test:test@" + ln.Addr().String() + "/foo?bar=baz"
	n := testing.AllocsPerRun(100, func() {
	req := AcquireRequest()
	res := AcquireResponse()
	req.SetRequestURI(url)
	if err := c.Do(req, res); err != nil {
	t.Fatal(err)
	}
	ReleaseRequest(req)
	ReleaseResponse(res)
	})
	if n != 0 {
	t.Fatalf("expected 0 allocations, got %f", n)
	}
	}

[chanjarster]

First thing I notice is a lot of potential heap allocations which can slow things down. Did you try doing any benchmarks or checking the number of allocations with a test like this?:

fasthttp/allocation_test.go

Lines 40 to 72 in 38aa88a

	func TestAllocationClient(t *testing.T) {
	ln, err := net.Listen("tcp4", "127.0.0.1:0")
	if err != nil {
	t.Fatalf("cannot listen: %s", err)
	}
	defer ln.Close()
	s := &Server{
	Handler: func(ctx *RequestCtx) {
	},
	}
	go s.Serve(ln) //nolint:errcheck
	c := &Client{}
	url := "http://test:test@" + ln.Addr().String() + "/foo?bar=baz"
	n := testing.AllocsPerRun(100, func() {
	req := AcquireRequest()
	res := AcquireResponse()
	req.SetRequestURI(url)
	if err := c.Do(req, res); err != nil {
	t.Fatal(err)
	}
	ReleaseRequest(req)
	ReleaseResponse(res)
	})
	if n != 0 {
	t.Fatalf("expected 0 allocations, got %f", n)
	}
	}

I've noticed that the current benchmark and allocation test are based on enough MaxConnsPerHost.

By default (which MaxConnWaitTimeout=0) this implementation doesn't queue the request and return ErrNoFreeConns immediately.

I'll provide an allocation test based on slow server and enable MaxConnWaitTimeout later. But I suspect that queueing the request will definitely cause heap allocations.

[chanjarster]

Hi @erikdubbelboer , I add some benchmarks to show heap allocations in situation of waiting for a free connection, below is my test result:

GOMAXPROCS=4 go test -bench='EndToEndWaitConn' -benchmem -benchtime=10s
goos: darwin
goarch: amd64
pkg: github.com/valyala/fasthttp
BenchmarkClientGetEndToEndWaitConn1Inmemory-4             	     237	  50512742 ns/op	     904 B/op	       4 allocs/op
BenchmarkClientGetEndToEndWaitConn10Inmemory-4            	     237	  50466510 ns/op	     551 B/op	       3 allocs/op
BenchmarkClientGetEndToEndWaitConn100Inmemory-4           	     566	  17675896 ns/op	     912 B/op	       8 allocs/op
BenchmarkClientGetEndToEndWaitConn1000Inmemory-4          	    7539	   1332690 ns/op	     525 B/op	       5 allocs/op
BenchmarkClientGetEndToEndWaitConn10kInmemory-4           	   41928	    239216 ns/op	    1643 B/op	      14 allocs/op
BenchmarkNetHTTPClientGetEndToEndWaitConn1Inmemory-4      	     237	  50459617 ns/op	    6788 B/op	      77 allocs/op
BenchmarkNetHTTPClientGetEndToEndWaitConn10Inmemory-4     	     237	  50427900 ns/op	    6840 B/op	      78 allocs/op
BenchmarkNetHTTPClientGetEndToEndWaitConn100Inmemory-4    	     570	  17549629 ns/op	    4550 B/op	      54 allocs/op
BenchmarkNetHTTPClientGetEndToEndWaitConn1000Inmemory-4   	    7824	   1283183 ns/op	    3637 B/op	      41 allocs/op
PASS
ok  	github.com/valyala/fasthttp	229.573s

[erikdubbelboer]

popFront can return nil in which case this will panic.

[chanjarster]

Since q.len() > 0, so popFront is impossible return nil

[erikdubbelboer]

Would it be possible to reuse wantConn or the channel? Or is that going to make the code so complicated that its not worth it?

[chanjarster]

For the first question, if we want to reuse the wantConn, then we must reuse the channel, but AFAIK there's no way to reopen a closed channel (maybe).
In terms of the heap allocation, yes, this will cause heap allocation and slow things down, but if we want to waiting for a free connection, the waiting dominates the slow, not gc.

For the second question, many http clients provide connection pooling and waiting mechanism (by blocking). This waiting mechanism is very useful, it can simplify the code on the caller side, otherwise the caller have two options:

1. busy waiting, try again immediately at some max try count, which consumes cpu time.
2. sleep waiting, sleep for a while and try again, which can't get connection in time.

Both are not perfect. So I think the waiting mechanism is worth it.

For the complication of this PR, em ..., actually this PR is based on net/http/transport.go, maybe there is a more simple way, we can have a discussion about that :)

[erikdubbelboer]

I have thought about it and I agree. Let's merge this!