From 731ab290a0575f873d863c16a6da697d1ffac0e2 Mon Sep 17 00:00:00 2001 From: Zach Koopmans Date: Fri, 18 Aug 2023 10:19:52 -0700 Subject: [PATCH] Fix redis test after new image is used. Fix the redis test after the image has been changed. The new image has a different output format, so update the report function to accomodate. This also means we can collect more metrics easily. Also use the redis-cli to check that the server is serving instead of wget --spider. PiperOrigin-RevId: 558181384 --- test/benchmarks/database/redis_test.go | 21 ++++++- test/benchmarks/tools/BUILD | 1 - test/benchmarks/tools/redis.go | 55 +++++++++------- test/benchmarks/tools/redis_test.go | 87 -------------------------- 4 files changed, 49 insertions(+), 115 deletions(-) delete mode 100644 test/benchmarks/tools/redis_test.go diff --git a/test/benchmarks/database/redis_test.go b/test/benchmarks/database/redis_test.go index 9ea800747..d3f8b5030 100644 --- a/test/benchmarks/database/redis_test.go +++ b/test/benchmarks/database/redis_test.go @@ -30,7 +30,7 @@ import ( // run both PING_INLINE and PING_BUILD. var operations []string = []string{ "PING_INLINE", - "PING_BULK", + "PING_MBULK", "SET", "GET", "INCR", @@ -90,9 +90,24 @@ func doBenchmarkRedis(b *testing.B, ops []string) { b.Fatalf("failed to start redis server: %v %s", err, out) } - if err = harness.WaitUntilContainerServing(ctx, clientMachine, server, port); err != nil { - b.Fatalf("failed to start redis with: %v", err) + pinger := clientMachine.GetNativeContainer(ctx, b) + defer pinger.CleanUp(ctx) + + out, err := pinger.Run(ctx, dockerutil.RunOpts{ + Image: "benchmarks/redis", + Links: []string{ + server.MakeLink("redis"), + }, + }, strings.Split("redis-cli -h redis -r 5 -i 1 ping", " ")...) + + if err != nil { + b.Fatalf("redis-benchmark failed with: %v", err) } + + if !strings.Contains(strings.ToLower(out), "pong") { + b.Fatalf("redis-benchmark failed to start redis server: %s", out) + } + for _, operation := range ops { param := tools.Parameter{ Name: "operation", diff --git a/test/benchmarks/tools/BUILD b/test/benchmarks/tools/BUILD index 0c3314a8d..4c4aa85b5 100644 --- a/test/benchmarks/tools/BUILD +++ b/test/benchmarks/tools/BUILD @@ -33,7 +33,6 @@ go_test( "hey_test.go", "iperf_test.go", "meminfo_test.go", - "redis_test.go", "sysbench_test.go", ], library = ":tools", diff --git a/test/benchmarks/tools/redis.go b/test/benchmarks/tools/redis.go index 15b695eb0..d374af9e9 100644 --- a/test/benchmarks/tools/redis.go +++ b/test/benchmarks/tools/redis.go @@ -16,8 +16,8 @@ package tools import ( "fmt" - "regexp" "strconv" + "strings" "testing" ) @@ -30,17 +30,6 @@ type Redis struct { func (r *Redis) MakeCmd(host string, port, requests int) []string { // There is no -t PING_BULK for redis-benchmark, so adjust the command in that case. // Note that "ping" will run both PING_INLINE and PING_BULK. - if r.Operation == "PING_BULK" { - return []string{ - "redis-benchmark", - "--csv", - "-t", "ping", - "-h", host, - "-p", fmt.Sprintf("%d", port), - "-n", fmt.Sprintf("%d", requests), - } - } - // runs redis-benchmark -t operation for 100K requests against server. return []string{ "redis-benchmark", @@ -55,19 +44,37 @@ func (r *Redis) MakeCmd(host string, port, requests int) []string { // Report parses output from redis-benchmark client and reports metrics. func (r *Redis) Report(b *testing.B, output string) { b.Helper() - result, err := r.parseOperation(output) - if err != nil { - b.Fatalf("parsing result %s failed with err: %v", output, err) + lines := strings.Split(output, "\n") + if len(lines) < 2 { + b.Fatalf("redis-benchmark failed to parse redis output: %s", output) + } + titleLine := lines[0] + resultLine := "" + for _, line := range lines[1:] { + if strings.Contains(line, r.Operation) { + resultLine = line + break + } + } + if len(resultLine) < 1 { + b.Fatalf("redis-benchmark failed to find LRANGE_100 in redis output: %s", output) } - ReportCustomMetric(b, result, r.Operation /*metric_name*/, "QPS" /*unit*/) -} -// parseOperation grabs the metric operations per second from redis-benchmark output. -func (r *Redis) parseOperation(data string) (float64, error) { - re := regexp.MustCompile(fmt.Sprintf(`"%s( .*)?","(\d*\.\d*)"`, r.Operation)) - match := re.FindStringSubmatch(data) - if len(match) < 3 { - return 0.0, fmt.Errorf("could not find %s in %s", r.Operation, data) + titles := strings.Split(titleLine, ",") + results := strings.Split(resultLine, ",") + if len(titles) != len(results) { + b.Fatalf("redis-benchmark failed to parse redis output: %s", output) + } + + for i := range titles { + title := strings.Trim(titles[i], "\"") + if strings.Contains(title, "test") { + continue + } + result, err := strconv.ParseFloat(strings.Trim(results[i], "\""), 64) + if err != nil { + b.Fatalf("redis-benchmark failed to parse redis output %v: %s", err, output) + } + ReportCustomMetric(b, result, r.Operation /*metric_name*/, title /*unit*/) } - return strconv.ParseFloat(match[2], 64) } diff --git a/test/benchmarks/tools/redis_test.go b/test/benchmarks/tools/redis_test.go deleted file mode 100644 index 4bafda66f..000000000 --- a/test/benchmarks/tools/redis_test.go +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package tools - -import ( - "testing" -) - -// TestRedis checks the Redis parsers on sample output. -func TestRedis(t *testing.T) { - sampleData := ` - "PING_INLINE","48661.80" - "PING_BULK","50301.81" - "SET","48923.68" - "GET","49382.71" - "INCR","49975.02" - "LPUSH","49875.31" - "RPUSH","50276.52" - "LPOP","50327.12" - "RPOP","50556.12" - "SADD","49504.95" - "HSET","49504.95" - "SPOP","50025.02" - "LPUSH (needed to benchmark LRANGE)","48875.86" - "LRANGE_100 (first 100 elements)","33955.86" - "LRANGE_300 (first 300 elements)","16550.81"// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package tools - - "LRANGE_500 (first 450 elements)","13653.74" - "LRANGE_600 (first 600 elements)","11219.57" - "MSET (10 keys)","44682.75" - ` - wants := map[string]float64{ - "PING_INLINE": 48661.80, - "PING_BULK": 50301.81, - "SET": 48923.68, - "GET": 49382.71, - "INCR": 49975.02, - "LPUSH": 49875.31, - "RPUSH": 50276.52, - "LPOP": 50327.12, - "RPOP": 50556.12, - "SADD": 49504.95, - "HSET": 49504.95, - "SPOP": 50025.02, - "LRANGE_100": 33955.86, - "LRANGE_300": 16550.81, - "LRANGE_500": 13653.74, - "LRANGE_600": 11219.57, - "MSET": 44682.75, - } - for op, want := range wants { - redis := Redis{ - Operation: op, - } - if got, err := redis.parseOperation(sampleData); err != nil { - t.Fatalf("failed to parse %s: %v", op, err) - } else if want != got { - t.Fatalf("wanted %f for op %s, got %f", want, op, got) - } - } -}