Improving benchmark tests

benchmark_test.go

@@ -19,6 +19,7 @@ package gubernator_test
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"runtime"
 	"testing"
 
@@ -59,7 +60,7 @@ func BenchmarkServer(b *testing.B) {
 	createdAt := epochMillis(clock.Now())
 	d := cluster.GetRandomDaemon(cluster.DataCenterNone)
 
-	b.Run("GetPeerRateLimit", func(b *testing.B) {
+	b.Run("Forward", func(b *testing.B) {
 		client := d.MustClient().(guber.PeerClient)
 		b.ResetTimer()
 
@@ -84,9 +85,9 @@ func BenchmarkServer(b *testing.B) {
 		}
 	})
 
-	b.Run("GetRateLimits batching", func(b *testing.B) {
+	b.Run("CheckRateLimits batching", func(b *testing.B) {
 		client := cluster.GetRandomDaemon(cluster.DataCenterNone).MustClient()
-		require.NoError(b, err, "Error in guber.DialV1Server")
+		require.NoError(b, err)
 		b.ResetTimer()
 
 		for n := 0; n < b.N; n++ {
@@ -108,9 +109,9 @@ func BenchmarkServer(b *testing.B) {
 		}
 	})
 
-	b.Run("GetRateLimits global", func(b *testing.B) {
+	b.Run("CheckRateLimits global", func(b *testing.B) {
 		client := cluster.GetRandomDaemon(cluster.DataCenterNone).MustClient()
-		require.NoError(b, err, "Error in guber.DialV1Server")
+		require.NoError(b, err)
 		b.ResetTimer()
 
 		for n := 0; n < b.N; n++ {
@@ -128,14 +129,14 @@ func BenchmarkServer(b *testing.B) {
 				},
 			}, &resp)
 			if err != nil {
-				b.Errorf("Error in client.GetRateLimits: %s", err)
+				b.Errorf("Error in client.CheckRateLimits: %s", err)
 			}
 		}
 	})
 
 	b.Run("HealthCheck", func(b *testing.B) {
 		client := cluster.GetRandomDaemon(cluster.DataCenterNone).MustClient()
-		require.NoError(b, err, "Error in guber.DialV1Server")
+		require.NoError(b, err)
 		b.ResetTimer()
 
 		for n := 0; n < b.N; n++ {
@@ -146,8 +147,7 @@ func BenchmarkServer(b *testing.B) {
 		}
 	})
 
-	b.Run("Thundering herd", func(b *testing.B) {
-		require.NoError(b, err, "Error in guber.DialV1Server")
+	b.Run("Concurrency CheckRateLimits", func(b *testing.B) {
 		var clients []guber.Client
 
 		// Create a client for each CPU on the system. This should allow us to simulate the
@@ -157,23 +157,28 @@ func BenchmarkServer(b *testing.B) {
 			require.NoError(b, err)
 			clients = append(clients, client)
 		}
-		b.ResetTimer()
-		mask := len(clients) - 1
 
+		keys := GenerateRandomKeys(8_000)
+		keyMask := len(keys) - 1
+		clientMask := len(clients) - 1
 		var idx int
+
+		b.ResetTimer()
 		b.RunParallel(func(pb *testing.PB) {
-			client := clients[idx&mask]
+			client := clients[idx&clientMask]
 			idx++
 
 			for pb.Next() {
+				keyIdx := int(rand.Uint32() & uint32(clientMask))
 				var resp guber.CheckRateLimitsResponse
 				err = client.CheckRateLimits(ctx, &guber.CheckRateLimitsRequest{
 					Requests: []*guber.RateLimitRequest{
 						{
 							Name:      b.Name(),
-							UniqueKey: guber.RandomString(10),
-							Limit:     10,
-							Duration:  guber.Second * 5,
+							UniqueKey: keys[keyIdx&keyMask],
+							Behavior:  guber.Behavior_GLOBAL,
+							Duration:  guber.Minute,
+							Limit:     100,
 							Hits:      1,
 						},
 					},
@@ -183,5 +188,36 @@ func BenchmarkServer(b *testing.B) {
 				}
 			}
 		})
+
+		for _, client := range clients {
+			_ = client.Close(context.Background())
+		}
+	})
+
+	b.Run("Concurrency HealthCheck", func(b *testing.B) {
+		var clients []guber.Client
+
+		// Create a client for each CPU on the system. This should allow us to simulate the
+		// maximum contention possible for this system.
+		for i := 0; i < runtime.NumCPU(); i++ {
+			client, err := guber.NewClient(guber.WithNoTLS(d.Listener.Addr().String()))
+			require.NoError(b, err)
+			clients = append(clients, client)
+		}
+		mask := len(clients) - 1
+		var idx int
+
+		b.ResetTimer()
+		b.RunParallel(func(pb *testing.PB) {
+			client := clients[idx&mask]
+			idx++
+
+			for pb.Next() {
+				var resp guber.HealthCheckResponse
+				if err := client.HealthCheck(ctx, &resp); err != nil {
+					b.Errorf("Error in client.HealthCheck: %s", err)
+				}
+			}
+		})
 	})
 }

client.go

@@ -32,7 +32,6 @@ import (
 	"github.com/mailgun/holster/v4/setter"
 	"github.com/pkg/errors"
 	"go.opentelemetry.io/otel/propagation"
-	"golang.org/x/net/http2"
 	"google.golang.org/protobuf/proto"
 )
 
@@ -67,7 +66,14 @@ type client struct {
 
 // NewClient creates a new instance of the Gubernator user client
 func NewClient(opts ClientOptions) (Client, error) {
-	setter.SetDefault(&opts.Client, &http.Client{})
+	setter.SetDefault(&opts.Client, &http.Client{
+		Transport: &http.Transport{
+			MaxConnsPerHost:     2_000,
+			MaxIdleConns:        2_000,
+			MaxIdleConnsPerHost: 2_000,
+			IdleConnTimeout:     60 * time.Second,
+		},
+	})
 
 	if len(opts.Endpoint) == 0 {
 		return nil, errors.New("opts.Endpoint is empty; must provide an address")
@@ -163,7 +169,14 @@ func (c *client) Close(_ context.Context) error {
 func WithNoTLS(address string) ClientOptions {
 	return ClientOptions{
 		Endpoint: fmt.Sprintf("http://%s", address),
-		Client:   &http.Client{},
+		Client: &http.Client{
+			Transport: &http.Transport{
+				MaxConnsPerHost:     2_000,
+				MaxIdleConns:        2_000,
+				MaxIdleConnsPerHost: 2_000,
+				IdleConnTimeout:     60 * time.Second,
+			},
+		},
 	}
 }
 
@@ -172,8 +185,12 @@ func WithTLS(tls *tls.Config, address string) ClientOptions {
 	return ClientOptions{
 		Endpoint: fmt.Sprintf("https://%s", address),
 		Client: &http.Client{
-			Transport: &http2.Transport{
-				TLSClientConfig: tls,
+			Transport: &http.Transport{
+				TLSClientConfig:     tls,
+				MaxConnsPerHost:     2_000,
+				MaxIdleConns:        2_000,
+				MaxIdleConnsPerHost: 2_000,
+				IdleConnTimeout:     60 * time.Second,
 			},
 		},
 	}

functional_test.go

@@ -2110,6 +2110,8 @@ func TestGlobalBehavior(t *testing.T) {
 				// Assert PeerCheckRateLimits endpoint called on owner
 				// for each non-owner that received hits.
 				// Used by global hits update.
+				// TODO(thrawn01): It is more important to verify the counts exist on each peer instead of how they got there. As the method of
+				//  how they got there is an implementation detail and can/will change. Also, this test flaps occasionally.
 				gprlCounters2 := getPeerCounters(t, cluster.GetDaemons(), "gubernator_http_handler_duration_count{path=\"/v1/peer.forward\"}")
 				for _, peer := range cluster.GetDaemons() {
 					expected := gprlCounters[peer.InstanceID]

tls_test.go

@@ -31,7 +31,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/net/http2"
 )
 
 func spawnDaemon(t *testing.T, conf gubernator.DaemonConfig) *gubernator.Daemon {
@@ -268,7 +267,7 @@ func TestTLSClusterWithClientAuthentication(t *testing.T) {
 
 	config := d2.Config()
 	client := &http.Client{
-		Transport: &http2.Transport{
+		Transport: &http.Transport{
 			TLSClientConfig: config.ClientTLS(),
 		},
 	}