redis-benchmark: add tests, --version, a minor bug fixes (#7947)

- add test suite coverage for redis-benchmark
- add --version (similar to what redis-cli has)
- fix bug sending more requests than intended when pipeline > 1.
- when done sending requests, avoid freeing client in the write handler, in theory before
  responses are received (probably dead code since the read handler will call clientDone first)

Co-authored-by: Oran Agra <oran@redislabs.com>

5 files changed, 160 insertions, 4 deletions

diff --git a/src/Makefile b/src/Makefile
index daa201d73..45ee66267 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -259,7 +259,7 @@ REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.
 REDIS_CLI_NAME=redis-cli$(PROG_SUFFIX)
 REDIS_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o zmalloc.o release.o ae.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o
 REDIS_BENCHMARK_NAME=redis-benchmark$(PROG_SUFFIX)
-REDIS_BENCHMARK_OBJ=ae.o anet.o redis-benchmark.o adlist.o dict.o zmalloc.o siphash.o monotonic.o
+REDIS_BENCHMARK_OBJ=ae.o anet.o redis-benchmark.o adlist.o dict.o zmalloc.o release.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o
 REDIS_CHECK_RDB_NAME=redis-check-rdb$(PROG_SUFFIX)
 REDIS_CHECK_AOF_NAME=redis-check-aof$(PROG_SUFFIX)
 
diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c
index c17178e85..328f08077 100644
--- a/src/redis-benchmark.c
+++ b/src/redis-benchmark.c
@@ -29,6 +29,7 @@
  */
 
 #include "fmacros.h"
+#include "version.h"
 
 #include <stdio.h>
 #include <string.h>
@@ -179,6 +180,8 @@ typedef struct redisConfig {
 } redisConfig;
 
 /* Prototypes */
+char *redisGitSHA1(void);
+char *redisGitDirty(void);
 static void writeHandler(aeEventLoop *el, int fd, void *privdata, int mask);
 static void createMissingClients(client c);
 static benchmarkThread *createBenchmarkThread(int index);
@@ -196,6 +199,20 @@ static void updateClusterSlotsConfiguration();
 int showThroughput(struct aeEventLoop *eventLoop, long long id,
                    void *clientData);
 
+static sds benchmarkVersion(void) {
+    sds version;
+    version = sdscatprintf(sdsempty(), "%s", REDIS_VERSION);
+
+    /* Add git commit and working tree status when available */
+    if (strtoll(redisGitSHA1(),NULL,16)) {
+        version = sdscatprintf(version, " (git:%s", redisGitSHA1());
+        if (strtoll(redisGitDirty(),NULL,10))
+            version = sdscatprintf(version, "-dirty");
+        version = sdscat(version, ")");
+    }
+    return version;
+}
+
 /* Dict callbacks */
 static uint64_t dictSdsHash(const void *key);
 static int dictSdsKeyCompare(void *privdata, const void *key1,
@@ -577,9 +594,8 @@ static void writeHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
     if (c->written == 0) {
         /* Enforce upper bound to number of requests. */
         int requests_issued = 0;
-        atomicGetIncr(config.requests_issued, requests_issued, 1);
+        atomicGetIncr(config.requests_issued, requests_issued, config.pipeline);
         if (requests_issued >= config.requests) {
-            freeClient(c);
             return;
         }
 
@@ -1364,6 +1380,11 @@ int parseOptions(int argc, const char **argv) {
         if (!strcmp(argv[i],"-c")) {
             if (lastarg) goto invalid;
             config.numclients = atoi(argv[++i]);
+        } else if (!strcmp(argv[i],"-v") || !strcmp(argv[i], "--version")) {
+            sds version = benchmarkVersion();
+            printf("redis-benchmark %s\n", version);
+            sdsfree(version);
+            exit(0);
         } else if (!strcmp(argv[i],"-n")) {
             if (lastarg) goto invalid;
             config.requests = atoi(argv[++i]);
@@ -1496,7 +1517,9 @@ usage:
 " -l                 Loop. Run the tests forever\n"
 " -t <tests>         Only run the comma separated list of tests. The test\n"
 "                    names are the same as the ones produced as output.\n"
-" -I                 Idle mode. Just open N idle connections and wait.\n\n"
+" -I                 Idle mode. Just open N idle connections and wait.\n"
+" --help             Output this help and exit.\n"
+" --version          Output version and exit.\n\n"
 "Examples:\n\n"
 " Run the benchmark with the default configuration against 127.0.0.1:6379:\n"
 "   $ redis-benchmark\n\n"
diff --git a/tests/integration/redis-benchmark.tcl b/tests/integration/redis-benchmark.tcl
new file mode 100644
index 000000000..8abeabf0b
--- /dev/null
+++ b/tests/integration/redis-benchmark.tcl
@@ -0,0 +1,127 @@
+source tests/support/benchmark.tcl
+
+
+proc cmdstat {cmd} {
+    return [cmdrstat $cmd r]
+}
+
+start_server {tags {"benchmark"}} {
+    start_server {} {
+        set master_host [srv 0 host]
+        set master_port [srv 0 port]
+
+        test {benchmark: set,get} {
+            r config resetstat
+            r flushall
+            set cmd [redisbenchmark $master_host $master_port "-c 5 -n 10 -e -t set,get"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            assert_match  {*calls=10,*} [cmdstat set]
+            assert_match  {*calls=10,*} [cmdstat get]
+            # assert one of the non benchmarked commands is not present
+            assert_match  {} [cmdstat lrange]
+        }
+
+        test {benchmark: full test suite} {
+            r config resetstat
+            set cmd [redisbenchmark $master_host $master_port "-c 10 -n 100 -e"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            # ping total calls are 2*issued commands per test due to PING_INLINE and PING_BULK
+            assert_match  {*calls=200,*} [cmdstat ping]
+            assert_match  {*calls=100,*} [cmdstat set]
+            assert_match  {*calls=100,*} [cmdstat get]
+            assert_match  {*calls=100,*} [cmdstat incr]
+            # lpush total calls are 2*issued commands per test due to the lrange tests
+            assert_match  {*calls=200,*} [cmdstat lpush]
+            assert_match  {*calls=100,*} [cmdstat rpush]
+            assert_match  {*calls=100,*} [cmdstat lpop]
+            assert_match  {*calls=100,*} [cmdstat rpop]
+            assert_match  {*calls=100,*} [cmdstat sadd]
+            assert_match  {*calls=100,*} [cmdstat hset]
+            assert_match  {*calls=100,*} [cmdstat spop]
+            assert_match  {*calls=100,*} [cmdstat zadd]
+            assert_match  {*calls=100,*} [cmdstat zpopmin]
+            assert_match  {*calls=400,*} [cmdstat lrange]
+            assert_match  {*calls=100,*} [cmdstat mset]
+            # assert one of the non benchmarked commands is not present
+            assert_match {} [cmdstat rpoplpush]
+        }
+
+        test {benchmark: multi-thread set,get} {
+            r config resetstat
+            r flushall
+            set cmd [redisbenchmark $master_host $master_port "--threads 10 -c 5 -n 10 -e -t set,get"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            assert_match  {*calls=10,*} [cmdstat set]
+            assert_match  {*calls=10,*} [cmdstat get]
+            # assert one of the non benchmarked commands is not present
+            assert_match  {} [cmdstat lrange]
+
+            # ensure only one key was populated
+            assert_match  {1} [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
+        }
+
+        test {benchmark: pipelined full set,get} {
+            r config resetstat
+            r flushall
+            set cmd [redisbenchmark $master_host $master_port "-P 5 -c 10 -n 10010 -e -t set,get"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            assert_match  {*calls=10010,*} [cmdstat set]
+            assert_match  {*calls=10010,*} [cmdstat get]
+            # assert one of the non benchmarked commands is not present
+            assert_match  {} [cmdstat lrange]
+
+            # ensure only one key was populated
+            assert_match  {1} [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
+        }
+
+        test {benchmark: arbitrary command} {
+            r config resetstat
+            r flushall
+            set cmd [redisbenchmark $master_host $master_port "-c 5 -n 150 -e INCRBYFLOAT mykey 10.0"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            assert_match  {*calls=150,*} [cmdstat incrbyfloat]
+            # assert one of the non benchmarked commands is not present
+            assert_match  {} [cmdstat get]
+
+            # ensure only one key was populated
+            assert_match  {1} [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
+        }
+
+        test {benchmark: keyspace length} {
+            r flushall
+            r config resetstat
+            set cmd [redisbenchmark $master_host $master_port "-r 50 -t set -n 1000"]
+            if {[catch { exec {*}$cmd } error]} {
+                set first_line [lindex [split $error "\n"] 0]
+                puts [colorstr red "redis-benchmark non zero code. first line: $first_line"]
+                fail "redis-benchmark non zero code. first line: $first_line"
+            }
+            assert_match  {*calls=1000,*} [cmdstat set]
+            # assert one of the non benchmarked commands is not present
+            assert_match  {} [cmdstat get]
+
+            # ensure the keyspace has the desired size
+            assert_match  {50} [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d]
+        }
+    }
+}
diff --git a/tests/support/benchmark.tcl b/tests/support/benchmark.tcl
new file mode 100644
index 000000000..1130c9992
--- /dev/null
+++ b/tests/support/benchmark.tcl
@@ -0,0 +1,5 @@
+proc redisbenchmark {host port {opts {}}} {
+    set cmd [list src/redis-benchmark -h $host -p $port]
+    lappend cmd {*}$opts
+    return $cmd
+}
diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl
index 37af46947..37ab23d0c 100644
--- a/tests/test_helper.tcl
+++ b/tests/test_helper.tcl
@@ -50,6 +50,7 @@ set ::all_tests {
     integration/psync2-reg
     integration/psync2-pingoff
     integration/redis-cli
+    integration/redis-benchmark
     unit/pubsub
     unit/slowlog
     unit/scripting