From c37d00791cdd41a27dfd4be96d3af27b6b024dcc Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Mon, 10 Feb 2020 15:13:53 -0600 Subject: [PATCH] make the TestRPC_RPCMaxConnsPerClient test less flaky (#7255) --- agent/consul/rpc_test.go | 6 ++++ go.mod | 2 +- go.sum | 4 +-- .../hashicorp/go-connlimit/connlimit.go | 29 ++++++++++++++++--- vendor/modules.txt | 2 +- 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/agent/consul/rpc_test.go b/agent/consul/rpc_test.go index 29bbf90a1..62370e00e 100644 --- a/agent/consul/rpc_test.go +++ b/agent/consul/rpc_test.go @@ -631,7 +631,13 @@ func TestRPC_RPCMaxConnsPerClient(t *testing.T) { defer conn3.Close() // If we close one of the earlier ones, we should be able to open another + addr := conn1.RemoteAddr() conn1.Close() + retry.Run(t, func(r *retry.R) { + if n := s1.rpcConnLimiter.NumOpen(addr); n >= 2 { + r.Fatal("waiting for open conns to drop") + } + }) conn4 := connectClient(t, s1, tc.magicByte, tc.tlsEnabled, true, "conn4") defer conn4.Close() diff --git a/go.mod b/go.mod index 0bd07abb0..97d657d39 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/hashicorp/go-bexpr v0.1.2 github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de github.com/hashicorp/go-cleanhttp v0.5.1 - github.com/hashicorp/go-connlimit v0.1.0 + github.com/hashicorp/go-connlimit v0.2.0 github.com/hashicorp/go-discover v0.0.0-20191202160150-7ec2cfbda7a2 github.com/hashicorp/go-hclog v0.12.0 github.com/hashicorp/go-memdb v1.0.3 diff --git a/go.sum b/go.sum index ffdba4224..505eda881 100644 --- a/go.sum +++ b/go.sum @@ -122,8 +122,8 @@ github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de/go.mod h1: github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= -github.com/hashicorp/go-connlimit v0.1.0 h1:j8XwYaCBgm7n82jaQpaUeaDlsJH1x5/ycAJhlkrXv7E= -github.com/hashicorp/go-connlimit v0.1.0/go.mod h1:OUj9FGL1tPIhl/2RCfzYHrIiWj+VVPGNyVPnUX8AqS0= +github.com/hashicorp/go-connlimit v0.2.0 h1:OZjcfNxH/hPh/bT2Iw5yOJcLzz+zuIWpsp3I1S4Pjw4= +github.com/hashicorp/go-connlimit v0.2.0/go.mod h1:OUj9FGL1tPIhl/2RCfzYHrIiWj+VVPGNyVPnUX8AqS0= github.com/hashicorp/go-discover v0.0.0-20191202160150-7ec2cfbda7a2 h1:r7GtRT+VXoM5WqHMxSVDIKgVCfK9T8CoS51RDKeOjBM= github.com/hashicorp/go-discover v0.0.0-20191202160150-7ec2cfbda7a2/go.mod h1:NnH5X4UCBEBdTuK2L8s4e4ilJm3UmGX0bANHCz0HSs0= github.com/hashicorp/go-hclog v0.0.0-20180709165350-ff2cf002a8dd/go.mod h1:9bjs9uLqI8l75knNv3lV1kA55veR+WUPSiKIWcQHudI= diff --git a/vendor/github.com/hashicorp/go-connlimit/connlimit.go b/vendor/github.com/hashicorp/go-connlimit/connlimit.go index cefcb8dad..d445df103 100644 --- a/vendor/github.com/hashicorp/go-connlimit/connlimit.go +++ b/vendor/github.com/hashicorp/go-connlimit/connlimit.go @@ -64,7 +64,7 @@ func NewLimiter(cfg Config) *Limiter { // or transient failure to read or parse the remote IP. The free func will be a // no-op in this case and need not be called. func (l *Limiter) Accept(conn net.Conn) (func(), error) { - addrKey := addrKey(conn) + addrKey := connKey(conn) // Load config outside locked section since it's not updated under lock anyway // and the atomic Load might be slower/contented so better to do outside lock. @@ -101,8 +101,29 @@ func (l *Limiter) Accept(conn net.Conn) (func(), error) { return free, nil } -func addrKey(conn net.Conn) string { - addr := conn.RemoteAddr() +func (l *Limiter) NumOpen(addr net.Addr) int { + addrKey := addrKey(addr) + + l.l.Lock() + defer l.l.Unlock() + + if l.cs == nil { + return 0 + } + + cs := l.cs[addrKey] + if cs == nil { + return 0 + } + + return len(cs) +} + +func connKey(conn net.Conn) string { + return addrKey(conn.RemoteAddr()) +} + +func addrKey(addr net.Addr) string { switch a := addr.(type) { case *net.TCPAddr: return "ip:" + a.IP.String() @@ -119,7 +140,7 @@ func addrKey(conn net.Conn) string { // freeConn removes a connection from the map if it's present. It is a no-op if // the conn was never accepted by Accept. func (l *Limiter) freeConn(conn net.Conn) { - addrKey := addrKey(conn) + addrKey := connKey(conn) l.l.Lock() defer l.l.Unlock() diff --git a/vendor/modules.txt b/vendor/modules.txt index 15498ec5d..cb5b874f0 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -177,7 +177,7 @@ github.com/hashicorp/go-bexpr github.com/hashicorp/go-checkpoint # github.com/hashicorp/go-cleanhttp v0.5.1 github.com/hashicorp/go-cleanhttp -# github.com/hashicorp/go-connlimit v0.1.0 +# github.com/hashicorp/go-connlimit v0.2.0 github.com/hashicorp/go-connlimit # github.com/hashicorp/go-discover v0.0.0-20191202160150-7ec2cfbda7a2 github.com/hashicorp/go-discover