From fea95b84790923ad4b8657dd5e28359e35929442 Mon Sep 17 00:00:00 2001 From: TwiN Date: Mon, 13 Jun 2022 19:15:30 -0400 Subject: [PATCH] perf(storage): Improve benchmarks and fix race condition --- controller/handler/endpoint_status_test.go | 6 +-- core/endpoint.go | 2 +- core/endpoint_test.go | 46 +++++++++++----------- storage/store/memory/memory_test.go | 2 +- storage/store/sql/sql.go | 3 +- storage/store/sql/sql_test.go | 2 +- storage/store/store_bench_test.go | 42 +++++++++++++------- storage/store/store_test.go | 2 +- 8 files changed, 56 insertions(+), 49 deletions(-) diff --git a/controller/handler/endpoint_status_test.go b/controller/handler/endpoint_status_test.go index 8e8bc4e7..011f53b9 100644 --- a/controller/handler/endpoint_status_test.go +++ b/controller/handler/endpoint_status_test.go @@ -13,10 +13,6 @@ import ( ) var ( - firstCondition = core.Condition("[STATUS] == 200") - secondCondition = core.Condition("[RESPONSE_TIME] < 500") - thirdCondition = core.Condition("[CERTIFICATE_EXPIRATION] < 72h") - timestamp = time.Now() testEndpoint = core.Endpoint{ @@ -26,7 +22,7 @@ var ( Method: "GET", Body: "body", Interval: 30 * time.Second, - Conditions: []*core.Condition{&firstCondition, &secondCondition, &thirdCondition}, + Conditions: []core.Condition{core.Condition("[STATUS] == 200"), core.Condition("[RESPONSE_TIME] < 500"), core.Condition("[CERTIFICATE_EXPIRATION] < 72h")}, Alerts: nil, NumberOfFailuresInARow: 0, NumberOfSuccessesInARow: 0, diff --git a/core/endpoint.go b/core/endpoint.go index 4006e7ea..006da2b9 100644 --- a/core/endpoint.go +++ b/core/endpoint.go @@ -89,7 +89,7 @@ type Endpoint struct { Interval time.Duration `yaml:"interval,omitempty"` // Conditions used to determine the health of the endpoint - Conditions []*Condition `yaml:"conditions"` + Conditions []Condition `yaml:"conditions"` // Alerts is the alerting configuration for the endpoint in case of failure Alerts []*alert.Alert `yaml:"alerts,omitempty"` diff --git a/core/endpoint_test.go b/core/endpoint_test.go index 03b00b1f..33b67f90 100644 --- a/core/endpoint_test.go +++ b/core/endpoint_test.go @@ -80,11 +80,10 @@ func TestEndpoint_Type(t *testing.T) { } func TestEndpoint_ValidateAndSetDefaults(t *testing.T) { - condition := Condition("[STATUS] == 200") endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{Condition("[STATUS] == 200")}, Alerts: []*alert.Alert{{Type: alert.TypePagerDuty}}, } endpoint.ValidateAndSetDefaults() @@ -129,7 +128,7 @@ func TestEndpoint_ValidateAndSetDefaultsWithClientConfig(t *testing.T) { endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, ClientConfig: &client.Config{ Insecure: true, IgnoreRedirect: true, @@ -158,7 +157,7 @@ func TestEndpoint_ValidateAndSetDefaultsWithNoName(t *testing.T) { endpoint := &Endpoint{ Name: "", URL: "http://example.com", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, } err := endpoint.ValidateAndSetDefaults() if err == nil { @@ -172,7 +171,7 @@ func TestEndpoint_ValidateAndSetDefaultsWithNoUrl(t *testing.T) { endpoint := &Endpoint{ Name: "example", URL: "", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, } err := endpoint.ValidateAndSetDefaults() if err == nil { @@ -194,7 +193,6 @@ func TestEndpoint_ValidateAndSetDefaultsWithNoConditions(t *testing.T) { } func TestEndpoint_ValidateAndSetDefaultsWithDNS(t *testing.T) { - conditionSuccess := Condition("[DNS_RCODE] == NOERROR") endpoint := &Endpoint{ Name: "dns-test", URL: "http://example.com", @@ -202,7 +200,7 @@ func TestEndpoint_ValidateAndSetDefaultsWithDNS(t *testing.T) { QueryType: "A", QueryName: "example.com", }, - Conditions: []*Condition{&conditionSuccess}, + Conditions: []Condition{Condition("[DNS_RCODE] == NOERROR")}, } err := endpoint.ValidateAndSetDefaults() if err != nil { @@ -218,7 +216,7 @@ func TestEndpoint_buildHTTPRequest(t *testing.T) { endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, } endpoint.ValidateAndSetDefaults() request := endpoint.buildHTTPRequest() @@ -238,7 +236,7 @@ func TestEndpoint_buildHTTPRequestWithCustomUserAgent(t *testing.T) { endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, Headers: map[string]string{ "User-Agent": "Test/2.0", }, @@ -262,7 +260,7 @@ func TestEndpoint_buildHTTPRequestWithHostHeader(t *testing.T) { Name: "website-health", URL: "https://twin.sh/health", Method: "POST", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, Headers: map[string]string{ "Host": "example.com", }, @@ -283,7 +281,7 @@ func TestEndpoint_buildHTTPRequestWithGraphQLEnabled(t *testing.T) { Name: "website-graphql", URL: "https://twin.sh/graphql", Method: "POST", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, GraphQL: true, Body: `{ users(gender: "female") { @@ -314,7 +312,7 @@ func TestIntegrationEvaluateHealth(t *testing.T) { endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition, &bodyCondition}, + Conditions: []Condition{condition, bodyCondition}, } endpoint.ValidateAndSetDefaults() result := endpoint.EvaluateHealth() @@ -337,7 +335,7 @@ func TestIntegrationEvaluateHealthWithFailure(t *testing.T) { endpoint := Endpoint{ Name: "website-health", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, } endpoint.ValidateAndSetDefaults() result := endpoint.EvaluateHealth() @@ -357,7 +355,7 @@ func TestIntegrationEvaluateHealthWithInvalidCondition(t *testing.T) { endpoint := Endpoint{ Name: "invalid-condition", URL: "https://twin.sh/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, } if err := endpoint.ValidateAndSetDefaults(); err != nil { // XXX: Should this really not return an error? After all, the condition is not valid and conditions are part of the endpoint... @@ -377,7 +375,7 @@ func TestIntegrationEvaluateHealthWithError(t *testing.T) { endpoint := Endpoint{ Name: "invalid-host", URL: "http://invalid/health", - Conditions: []*Condition{&condition}, + Conditions: []Condition{condition}, UIConfig: &ui.Config{ HideHostname: true, }, @@ -408,7 +406,7 @@ func TestIntegrationEvaluateHealthForDNS(t *testing.T) { QueryType: "A", QueryName: "example.com.", }, - Conditions: []*Condition{&conditionSuccess, &conditionBody}, + Conditions: []Condition{conditionSuccess, conditionBody}, } endpoint.ValidateAndSetDefaults() result := endpoint.EvaluateHealth() @@ -428,7 +426,7 @@ func TestIntegrationEvaluateHealthForICMP(t *testing.T) { endpoint := Endpoint{ Name: "icmp-test", URL: "icmp://127.0.0.1", - Conditions: []*Condition{&conditionSuccess}, + Conditions: []Condition{conditionSuccess}, } endpoint.ValidateAndSetDefaults() result := endpoint.EvaluateHealth() @@ -448,7 +446,7 @@ func TestEndpoint_getIP(t *testing.T) { endpoint := Endpoint{ Name: "invalid-url-test", URL: "", - Conditions: []*Condition{&conditionSuccess}, + Conditions: []Condition{conditionSuccess}, } result := &Result{} endpoint.getIP(result) @@ -461,22 +459,22 @@ func TestEndpoint_NeedsToReadBody(t *testing.T) { statusCondition := Condition("[STATUS] == 200") bodyCondition := Condition("[BODY].status == UP") bodyConditionWithLength := Condition("len([BODY].tags) > 0") - if (&Endpoint{Conditions: []*Condition{&statusCondition}}).needsToReadBody() { + if (&Endpoint{Conditions: []Condition{statusCondition}}).needsToReadBody() { t.Error("expected false, got true") } - if !(&Endpoint{Conditions: []*Condition{&bodyCondition}}).needsToReadBody() { + if !(&Endpoint{Conditions: []Condition{bodyCondition}}).needsToReadBody() { t.Error("expected true, got false") } - if !(&Endpoint{Conditions: []*Condition{&bodyConditionWithLength}}).needsToReadBody() { + if !(&Endpoint{Conditions: []Condition{bodyConditionWithLength}}).needsToReadBody() { t.Error("expected true, got false") } - if !(&Endpoint{Conditions: []*Condition{&statusCondition, &bodyCondition}}).needsToReadBody() { + if !(&Endpoint{Conditions: []Condition{statusCondition, bodyCondition}}).needsToReadBody() { t.Error("expected true, got false") } - if !(&Endpoint{Conditions: []*Condition{&bodyCondition, &statusCondition}}).needsToReadBody() { + if !(&Endpoint{Conditions: []Condition{bodyCondition, statusCondition}}).needsToReadBody() { t.Error("expected true, got false") } - if !(&Endpoint{Conditions: []*Condition{&bodyConditionWithLength, &statusCondition}}).needsToReadBody() { + if !(&Endpoint{Conditions: []Condition{bodyConditionWithLength, statusCondition}}).needsToReadBody() { t.Error("expected true, got false") } } diff --git a/storage/store/memory/memory_test.go b/storage/store/memory/memory_test.go index bb3302fa..72bc0e36 100644 --- a/storage/store/memory/memory_test.go +++ b/storage/store/memory/memory_test.go @@ -22,7 +22,7 @@ var ( Method: "GET", Body: "body", Interval: 30 * time.Second, - Conditions: []*core.Condition{&firstCondition, &secondCondition, &thirdCondition}, + Conditions: []core.Condition{firstCondition, secondCondition, thirdCondition}, Alerts: nil, NumberOfFailuresInARow: 0, NumberOfSuccessesInARow: 0, diff --git a/storage/store/sql/sql.go b/storage/store/sql/sql.go index b6c91f3f..666e8753 100644 --- a/storage/store/sql/sql.go +++ b/storage/store/sql/sql.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log" + "strconv" "strings" "time" @@ -579,7 +580,7 @@ func (s *Store) getEndpointResultsByEndpointID(tx *sql.Tx, endpointID int64, pag WHERE endpoint_result_id IN (` index := 1 for endpointResultID := range idResultMap { - query += fmt.Sprintf("$%d,", index) + query += "$" + strconv.Itoa(index) + "," args = append(args, endpointResultID) index++ } diff --git a/storage/store/sql/sql_test.go b/storage/store/sql/sql_test.go index 9eaf94e2..79113103 100644 --- a/storage/store/sql/sql_test.go +++ b/storage/store/sql/sql_test.go @@ -23,7 +23,7 @@ var ( Method: "GET", Body: "body", Interval: 30 * time.Second, - Conditions: []*core.Condition{&firstCondition, &secondCondition, &thirdCondition}, + Conditions: []core.Condition{firstCondition, secondCondition, thirdCondition}, Alerts: nil, NumberOfFailuresInARow: 0, NumberOfSuccessesInARow: 0, diff --git a/storage/store/store_bench_test.go b/storage/store/store_bench_test.go index 8ba176ea..9e66716d 100644 --- a/storage/store/store_bench_test.go +++ b/storage/store/store_bench_test.go @@ -1,6 +1,7 @@ package store import ( + "strconv" "testing" "time" @@ -48,23 +49,34 @@ func BenchmarkStore_GetAllEndpointStatuses(b *testing.B) { }, } for _, scenario := range scenarios { - scenario.Store.Insert(&testEndpoint, &testSuccessfulResult) - scenario.Store.Insert(&testEndpoint, &testUnsuccessfulResult) - b.Run(scenario.Name, func(b *testing.B) { - if scenario.Parallel { - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - scenario.Store.GetAllEndpointStatuses(paging.NewEndpointStatusParams().WithResults(1, 20)) - } - }) - } else { - for n := 0; n < b.N; n++ { - scenario.Store.GetAllEndpointStatuses(paging.NewEndpointStatusParams().WithResults(1, 20)) + numberOfEndpoints := []int{10, 25, 50, 100} + for _, numberOfEndpointsToCreate := range numberOfEndpoints { + // Create endpoints and insert results + for i := 0; i < numberOfEndpointsToCreate; i++ { + endpoint := testEndpoint + endpoint.Name = "endpoint" + strconv.Itoa(i) + // Insert 20 results for each endpoint + for j := 0; j < 20; j++ { + scenario.Store.Insert(&endpoint, &testSuccessfulResult) } } - b.ReportAllocs() - }) - scenario.Store.Clear() + // Run the scenarios + b.Run(scenario.Name+"-with-"+strconv.Itoa(numberOfEndpointsToCreate)+"-endpoints", func(b *testing.B) { + if scenario.Parallel { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _, _ = scenario.Store.GetAllEndpointStatuses(paging.NewEndpointStatusParams().WithResults(1, 20)) + } + }) + } else { + for n := 0; n < b.N; n++ { + _, _ = scenario.Store.GetAllEndpointStatuses(paging.NewEndpointStatusParams().WithResults(1, 20)) + } + } + b.ReportAllocs() + }) + scenario.Store.Clear() + } } } diff --git a/storage/store/store_test.go b/storage/store/store_test.go index bb5fd037..e2828dc4 100644 --- a/storage/store/store_test.go +++ b/storage/store/store_test.go @@ -26,7 +26,7 @@ var ( Method: "GET", Body: "body", Interval: 30 * time.Second, - Conditions: []*core.Condition{&firstCondition, &secondCondition, &thirdCondition}, + Conditions: []core.Condition{firstCondition, secondCondition, thirdCondition}, Alerts: nil, NumberOfFailuresInARow: 0, NumberOfSuccessesInARow: 0,