From dc9b528f78e36d9b66acaf2ae25b89d7adae5581 Mon Sep 17 00:00:00 2001 From: "Derrick J. Wippler" Date: Mon, 20 May 2024 15:48:26 -0500 Subject: [PATCH] Improving benchmark tests --- benchmark_test.go | 66 +++++++++++++++++++++++++++++++++++----------- client.go | 27 +++++++++++++++---- functional_test.go | 2 ++ tls_test.go | 3 +-- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/benchmark_test.go b/benchmark_test.go index 4cac6dc..81b92a6 100644 --- a/benchmark_test.go +++ b/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) + } + } + }) }) } diff --git a/client.go b/client.go index c62fe86..c16de3f 100644 --- a/client.go +++ b/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, }, }, } diff --git a/functional_test.go b/functional_test.go index df907fd..820b139 100644 --- a/functional_test.go +++ b/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] diff --git a/tls_test.go b/tls_test.go index c70af5b..a974438 100644 --- a/tls_test.go +++ b/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(), }, }