Add --warmup and --duration parameters to valkey-benchmark (#2581)

It's handy to be able to automatically do a warmup and/or test by
duration rather than request count. 🙂

I changed the real-time output a bit - not sure if that's wanted or not.
(Like, would it break people's weird scripts? It'll break my weird
scripts, but I know the price of writing weird fragile scripts.)

```
Prepended "Warming up " when in warmup phase:
Warming up SET: rps=69211.2 (overall: 69747.5) avg_msec=0.425 (overall: 0.393) 3.8 seconds
^^^^^^^^^^

Appended running request counter when based on -n:
SET: rps=70892.0 (overall: 69878.1) avg_msec=0.385 (overall: 0.398) 612482 requests
                                                                    ^^^^^^^^^^^^^^^

Appended running second counter when in warmup or based on --duration:
SET: rps=61508.0 (overall: 61764.2) avg_msec=0.430 (overall: 0.426) 4.8 seconds
                                                                    ^^^^^^^^^^^
```

To be clear, the report at the end remains unchanged.

---------

Signed-off-by: Rain Valentine <[email protected]>
Signed-off-by: Viktor Söderqvist <[email protected]>

Author: rainsupreme

src/valkey-benchmark.c

@@ -102,6 +102,9 @@ static struct config {
     int numclients;
     _Atomic int liveclients;
     int requests;
+    int duration;
+    int warmup_duration;
+    _Atomic int current_warmup_duration;
     _Atomic int requests_issued;
     _Atomic int requests_finished;
     _Atomic int previous_requests_finished;
@@ -252,6 +255,21 @@ static long long nstime(void) {
     return (long long)ts.tv_sec * 1000000000LL + ts.tv_nsec;
 }
 
+static bool isBenchmarkFinished(int request_count) {
+    /* don't end in warmup period */
+    int warmup_duration = atomic_load_explicit(&config.current_warmup_duration, memory_order_relaxed);
+    if (warmup_duration > 0) return false;
+
+    if (config.duration > 0) {
+        /* end after the specified duration */
+        if ((mstime() - config.start) >= (config.duration * 1000LL)) return true;
+    } else {
+        /* end after the specified number of requests */
+        if (request_count >= config.requests) return true;
+    }
+    return false;
+}
+
 static uint64_t dictSdsHash(const void *key) {
     return dictGenHashFunction((unsigned char *)key, sdslen((char *)key));
 }
@@ -503,7 +521,7 @@ static void freeClient(client c) {
     aeDeleteFileEvent(el, c->context->fd, AE_READABLE);
     if (c->thread_id >= 0) {
         int requests_finished = atomic_load_explicit(&config.requests_finished, memory_order_relaxed);
-        if (requests_finished >= config.requests) {
+        if (isBenchmarkFinished(requests_finished)) {
             aeStop(el);
         }
     }
@@ -627,7 +645,7 @@ static long long acquireTokenOrWait(int tokens) {
 
 static void clientDone(client c) {
     int requests_finished = atomic_load_explicit(&config.requests_finished, memory_order_relaxed);
-    if (requests_finished >= config.requests) {
+    if (isBenchmarkFinished(requests_finished)) {
         freeClient(c);
         if (!config.num_threads && config.el) aeStop(config.el);
         return;
@@ -718,7 +736,7 @@ static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
                     continue;
                 }
                 int requests_finished = atomic_fetch_add_explicit(&config.requests_finished, 1, memory_order_relaxed);
-                if (requests_finished < config.requests) {
+                if (!isBenchmarkFinished(requests_finished)) {
                     if (config.num_threads == 0) {
                         hdr_record_value(config.latency_histogram, // Histogram to record to
                                          (long)c->latency <= CONFIG_LATENCY_HISTOGRAM_MAX_VALUE
@@ -836,7 +854,7 @@ static void writeHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
         int requests_issued = atomic_fetch_add_explicit(&config.requests_issued,
                                                         config.pipeline * c->seqlen,
                                                         memory_order_relaxed);
-        if (requests_issued >= config.requests) {
+        if (isBenchmarkFinished(requests_issued)) {
             return;
         }
 
@@ -1237,6 +1255,7 @@ static void benchmarkSequence(const char *title, char *cmd, int len, int seqlen)
     config.requests_finished = 0;
     config.previous_requests_finished = 0;
     config.last_printed_bytes = 0;
+    config.current_warmup_duration = config.warmup_duration;
     hdr_init(CONFIG_LATENCY_HISTOGRAM_MIN_VALUE,         // Minimum value
              CONFIG_LATENCY_HISTOGRAM_MAX_VALUE,         // Maximum value
              config.precision,                           // Number of significant figures
@@ -1619,7 +1638,22 @@ int parseOptions(int argc, char **argv) {
             exit(0);
         } else if (!strcmp(argv[i], "-n")) {
             if (lastarg) goto invalid;
+            if (config.duration > 0) {
+                fprintf(stderr, "Options -n and --duration are mutually exclusive.\n");
+                exit(1);
+            }
             config.requests = atoi(argv[++i]);
+        } else if (!strcmp(argv[i], "--duration")) {
+            if (lastarg) goto invalid;
+            if (config.requests > 0) {
+                fprintf(stderr, "Options -n and --duration are mutually exclusive.\n");
+                exit(1);
+            }
+            config.duration = atoi(argv[++i]);
+        } else if (!strcmp(argv[i], "--warmup")) {
+            if (lastarg) goto invalid;
+            config.warmup_duration = atoi(argv[++i]);
+
         } else if (!strcmp(argv[i], "-k")) {
             if (lastarg) goto invalid;
             config.keepalive = atoi(argv[++i]);
@@ -1817,13 +1851,15 @@ int parseOptions(int argc, char **argv) {
         " --cert <file>      Client certificate to authenticate with.\n"
         " --key <file>       Private key file to authenticate with.\n"
         " --tls-ciphers <list> Sets the list of preferred ciphers (TLSv1.2 and below)\n"
-        "                    in order of preference from highest to lowest separated by colon (\":\").\n"
-        "                    See the ciphers(1ssl) manpage for more information about the syntax of this string.\n"
+        "                    in order of preference from highest to lowest separated by\n"
+        "                    colon (\":\"). See the ciphers(1ssl) manpage for more\n"
+        "                    information about the syntax of this string.\n"
 #ifdef TLS1_3_VERSION
         " --tls-ciphersuites <list> Sets the list of preferred ciphersuites (TLSv1.3)\n"
-        "                    in order of preference from highest to lowest separated by colon (\":\").\n"
-        "                    See the ciphers(1ssl) manpage for more information about the syntax of this string,\n"
-        "                    and specifically for TLSv1.3 ciphersuites.\n"
+        "                    in order of preference from highest to lowest separated by\n"
+        "                    colon (\":\"). See the ciphers(1ssl) manpage for more\n"
+        "                    information about the syntax of this string, and\n"
+        "                    specifically for TLSv1.3 ciphersuites.\n"
 #endif
 #endif
         "";
@@ -1870,6 +1906,11 @@ int parseOptions(int argc, char **argv) {
         "                    Note: If --cluster is used then number of clients has to be\n"
         "                    the same or higher than the number of nodes.\n"
         " -n <requests>      Total number of requests (default 100000)\n"
+        " --duration <seconds>\n"
+        "                    Run benchmark for specified number of seconds\n"
+        "                    (mutually exclusive with -n)\n"
+        " --warmup <seconds> Run benchmark for specified warmup period before\n"
+        "                    recording data\n"
         " -d <size>          Data size of SET/GET value in bytes (default 3)\n"
         " --dbnum <db>       SELECT the specified db number (default 0)\n"
         " -3                 Start session in RESP3 protocol mode.\n"
@@ -1901,9 +1942,9 @@ int parseOptions(int argc, char **argv) {
         "                    use the same key.\n"
         " --sequential       Modifies the -r argument to replace the string __rand_int__\n"
         "                    with 12 digit numbers sequentially instead of randomly.\n"
-        "                    __rand_1st__ through __rand_9th__ are available with independent\n"
-        "                    counters. Used to create expected number of elements with multiple\n"
-        "                    replacements.\n"
+        "                    __rand_1st__ through __rand_9th__ are available with\n"
+        "                    independent counters. Used to create expected number of\n"
+        "                    elements with multiple replacements.\n"
         "                    example: ZADD myzset __rand_int__ element:__rand_1st__\n"
         " -P <numreq>        Pipeline <numreq> requests. That is, send multiple requests\n"
         "                    before waiting for the replies. Default 1 (no pipeline).\n"
@@ -1912,7 +1953,8 @@ int parseOptions(int argc, char **argv) {
         "                    the number of times the command sequence is sent in each\n"
         "                    pipeline.\n",
         " -q                 Quiet. Just show query/sec values\n"
-        " --precision        Number of decimal places to display in latency output (default 0)\n"
+        " --precision        Number of decimal places to display in latency output\n"
+        "                    (default 0)\n"
         " --csv              Output in CSV format\n"
         " -l                 Loop. Run the tests forever\n"
         " -t <tests>         Only run the comma separated list of tests. The test\n"
@@ -1921,8 +1963,10 @@ int parseOptions(int argc, char **argv) {
         "                    on the command line.\n"
         " -I                 Idle mode. Just open N idle connections and wait.\n"
         " -x                 Read last argument from STDIN.\n"
-        " --rps <requests>   Limit the total number of requests per second. Default 0 (no limit)\n"
-        " --seed <num>       Set the seed for random number generator. Default seed is based on time.\n"
+        " --rps <requests>   Limit the total number of requests per second.\n"
+        "                    Default 0 (no limit)\n"
+        " --seed <num>       Set the seed for random number generator.\n"
+        "                    Default seed is based on time.\n"
         " --num-functions <num>\n"
         "                    Sets the number of functions present in the Lua lib that is\n"
         "                    loaded when running the 'function_load' test. (default 10).\n"
@@ -1957,16 +2001,28 @@ long long showThroughput(struct aeEventLoop *eventLoop, long long id, void *clie
     UNUSED(eventLoop);
     UNUSED(id);
     benchmarkThread *thread = (benchmarkThread *)clientData;
-    int liveclients = atomic_load_explicit(&config.liveclients, memory_order_relaxed);
     int requests_finished = atomic_load_explicit(&config.requests_finished, memory_order_relaxed);
     int previous_requests_finished = atomic_load_explicit(&config.previous_requests_finished, memory_order_relaxed);
     long long current_tick = mstime();
 
-    if (liveclients == 0 && requests_finished != config.requests) {
+    int liveclients = atomic_load_explicit(&config.liveclients, memory_order_relaxed);
+    if (liveclients == 0 && !isBenchmarkFinished(requests_finished)) {
         fprintf(stderr, "All clients disconnected... aborting.\n");
         exit(1);
     }
-    if (config.num_threads && requests_finished >= config.requests) {
+    int warmup_duration = atomic_load_explicit(&config.current_warmup_duration, memory_order_relaxed);
+    if (warmup_duration > 0) {
+        if ((current_tick - config.start) >= (warmup_duration * 1000LL)) {
+            /* exit the warmup period, clear all stats */
+            atomic_store_explicit(&config.current_warmup_duration, 0, memory_order_relaxed);
+
+            config.start = current_tick;
+            atomic_store_explicit(&config.requests_finished, 0, memory_order_relaxed);
+            atomic_store_explicit(&config.requests_issued, 0, memory_order_relaxed);
+            atomic_store_explicit(&config.previous_requests_finished, 0, memory_order_relaxed);
+            hdr_reset(config.latency_histogram);
+        }
+    } else if (config.num_threads && isBenchmarkFinished(requests_finished)) {
         aeStop(eventLoop);
         return AE_NOMORE;
     }
@@ -1991,11 +2047,21 @@ long long showThroughput(struct aeEventLoop *eventLoop, long long id, void *clie
 
     config.previous_tick = current_tick;
     atomic_store_explicit(&config.previous_requests_finished, requests_finished, memory_order_relaxed);
+
     printf("%*s\r", config.last_printed_bytes, " "); /* ensure there is a clean line */
-    int printed_bytes =
-        printf("%s: rps=%.1f (overall: %.1f) avg_msec=%.3f (overall: %.3f)\r", config.title, instantaneous_rps, rps,
+    config.last_printed_bytes = 0;
+    if (warmup_duration > 0) {
+        config.last_printed_bytes += printf("Warming up ");
+    }
+    config.last_printed_bytes +=
+        printf("%s: rps=%.1f (overall: %.1f) avg_msec=%.3f (overall: %.3f)", config.title, instantaneous_rps, rps,
                hdr_mean(config.current_sec_latency_histogram) / 1000.0f, hdr_mean(config.latency_histogram) / 1000.0f);
-    config.last_printed_bytes = printed_bytes;
+    if (warmup_duration > 0 || config.duration > 0) {
+        config.last_printed_bytes += printf(" %.1f seconds\r", dt);
+    } else {
+        config.last_printed_bytes += printf(" %d requests\r", requests_finished);
+    }
+
     hdr_reset(config.current_sec_latency_histogram);
     fflush(stdout);
     return SHOW_THROUGHPUT_INTERVAL;
@@ -2067,7 +2133,10 @@ int main(int argc, char **argv) {
     memset(&config.sslconfig, 0, sizeof(config.sslconfig));
     config.ct = VALKEY_CONN_TCP;
     config.numclients = 50;
-    config.requests = 100000;
+    config.requests = -1;
+    config.duration = -1;
+    config.warmup_duration = -1;
+    config.current_warmup_duration = -1;
     config.liveclients = 0;
     config.el = aeCreateEventLoop(1024 * 10);
     aeCreateTimeEvent(config.el, 1, showThroughput, NULL, NULL);
@@ -2111,6 +2180,9 @@ int main(int argc, char **argv) {
     argc -= i;
     argv += i;
 
+    /* Set default for requests if not specified */
+    if (config.requests < 0) config.requests = 100000;
+
     tag = "";
 
 #ifdef USE_OPENSSL

tests/integration/valkey-benchmark.tcl

@@ -9,11 +9,12 @@ proc cmdstat {cmd} {
 proc common_bench_setup {cmd} {
     r config resetstat
     r flushall
-    if {[catch { exec {*}$cmd } error]} {
-        set first_line [lindex [split $error "\n"] 0]
-        puts [colorstr red "valkey-benchmark non zero code, the output is: $error"]
+    if {[catch { exec {*}$cmd } output]} {
+        set first_line [lindex [split $output "\n"] 0]
+        puts [colorstr red "valkey-benchmark non zero code, the output is: $output"]
         fail "valkey-benchmark non zero code. first line: $first_line"
     }
+    return $output
 }
 
 # we use this extra asserts on a simple set,get test for features like uri parsing
@@ -208,6 +209,74 @@ tags {"benchmark network external:skip logreqres:skip"} {
             assert_match  {50} [r zcard myzset]
         }
 
+        test {benchmark: warmup and duration are cumulative} {
+            set start_time [clock clicks -millisec]
+            set cmd [valkeybenchmark $master_host $master_port "-r 5 --warmup 1 --duration 1 -t set"]
+            set output [common_bench_setup $cmd]
+            set end_time [clock clicks -millisec]
+
+            # Verify total duration was at least 2 seconds
+            set elapsed [expr {($end_time - $start_time)/1000.0}]
+            assert {$elapsed >= 2 && $elapsed <= 2.25}
+
+            # Check reported duration
+            lassign [regexp -inline {(\d+) requests completed in ([\d.]+) seconds} $output] -> requests duration
+            assert {$duration < 2.0 && $duration >= 1.0}
+        }
+
+        test {benchmark: warmup can be used with request count} {
+            set start_time [clock clicks -millisec]
+            set cmd [valkeybenchmark $master_host $master_port "-r 5 --warmup 1 -n 100 -t set"]
+            set output [common_bench_setup $cmd]
+            set end_time [clock clicks -millisec]
+
+            # Verify total duration was at least 1 seconds
+            set elapsed [expr {($end_time - $start_time)/1000.0}]
+            assert {$elapsed >= 1}
+
+            # Check reported duration and command count
+            lassign [regexp -inline {(\d+) requests completed in ([\d.]+) seconds} $output] -> requests duration
+            assert {$duration < 1.0}
+            assert {$requests >= 100 && $requests < 150}
+        }
+
+        test {benchmark: -n and --duration are mutually exclusive} {
+            set cmd [valkeybenchmark $master_host $master_port "-r 5 -n 5 --duration 1 -t set"]
+            catch { exec {*}$cmd } error
+            assert_match "*Options -n and --duration are mutually exclusive*" $error
+        }
+
+        test {benchmark: warmup applies to all tests in multi-test run} {
+            set start_time [clock clicks -millisec]
+            set cmd [valkeybenchmark $master_host $master_port "-r 5 --warmup 1 -n 50 -t set,get,incr"]
+            set output [common_bench_setup $cmd]
+            set end_time [clock clicks -millisec]
+
+            # Verify total duration includes warmup for all 3 tests (at least 3 seconds)
+            set elapsed [expr {($end_time - $start_time)/1000.0}]
+            assert {$elapsed >= 3}
+
+            # Verify all tests ran - with warmup, we expect more than 50 calls per command
+            # since warmup commands are also counted in server stats
+            assert_match  {*calls=*} [cmdstat set]
+            assert_match  {*calls=*} [cmdstat get]
+            assert_match  {*calls=*} [cmdstat incr]
+            
+            # Verify that each command was called more than the base 50 requests
+            # due to warmup period adding extra requests
+            set set_calls [regexp -inline {calls=(\d+)} [cmdstat set]]
+            set get_calls [regexp -inline {calls=(\d+)} [cmdstat get]]
+            set incr_calls [regexp -inline {calls=(\d+)} [cmdstat incr]]
+            
+            lassign $set_calls -> set_count
+            lassign $get_calls -> get_count
+            lassign $incr_calls -> incr_count
+            
+            assert {$set_count > 50}
+            assert {$get_count > 50}
+            assert {$incr_count > 50}
+        }
+
         test {benchmark: clients idle mode should return error when reached maxclients limit} {
             set cmd [valkeybenchmark $master_host $master_port "-c 10 -I"]
             set original_maxclients [lindex [r config get maxclients] 1]
@@ -224,6 +293,30 @@ tags {"benchmark network external:skip logreqres:skip"} {
             r get key
         } {arg}
 
+        test {benchmark: CSV output format} {
+            set cmd [valkeybenchmark $master_host $master_port "-c 5 -n 10 -t set,get --csv"]
+            set output [common_bench_setup $cmd]
+            # CSV output should contain comma-separated values
+            assert_match "*,*,*,*" $output
+            # Should not contain the usual formatted output
+            assert_no_match "*requests per second*" $output
+            # Should not contain carriage returns or progress indicators
+            assert_no_match "*\r*" $output
+            assert_no_match "*rps=*" $output
+            default_set_get_checks
+        }
+
+        test {benchmark: quiet mode} {
+            set cmd [valkeybenchmark $master_host $master_port "-c 5 -n 10 -t set,get -q"]
+            set output [common_bench_setup $cmd]
+            # Quiet mode should only show query/sec values (case-insensitive)
+            assert {[string match -nocase "*requests per second*" $output]}
+            # Should not contain detailed latency information (case-insensitive)
+            assert {![string match -nocase "*distribution*" $output]}
+            assert {![string match -nocase "*percentile*" $output]}
+            default_set_get_checks
+        }
+
         # tls specific tests
         if {$::tls} {
             test {benchmark: specific tls-ciphers} {