Fix divide-by-zero in redis-benchmark and redis-cli (#14371)
Some checks failed
CI / test-ubuntu-latest (push) Waiting to run
CI / test-sanitizer-address (push) Waiting to run
CI / build-debian-old (push) Waiting to run
CI / build-macos-latest (push) Waiting to run
CI / build-32bit (push) Waiting to run
CI / build-libc-malloc (push) Waiting to run
CI / build-centos-jemalloc (push) Waiting to run
CI / build-old-chain-jemalloc (push) Waiting to run
Codecov / code-coverage (push) Waiting to run
External Server Tests / test-external-standalone (push) Waiting to run
External Server Tests / test-external-cluster (push) Waiting to run
External Server Tests / test-external-nodebug (push) Waiting to run
Spellcheck / Spellcheck (push) Waiting to run
Reply-schemas linter / reply-schemas-linter (push) Has been cancelled

Resolve https://github.com/redis/redis/issues/14358 and
https://github.com/redis/redis/issues/14359. Fix division by zero in
redis-benchmark and redis-cli when histograms are empty or elapsed time
is zero. Guard all affected divisions in showLatencyReport(),
showThroughput(), and displayKeyStatsSizeDist(). Added integration tests
for both cases.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
This commit is contained in:
h.o.t. neglected 2026-03-18 07:57:58 -04:00 committed by GitHub
parent 31a4356ac0
commit 4ecc07fcff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 47 additions and 4 deletions

View file

@ -828,8 +828,21 @@ static void createMissingClients(client c) {
}
static void showLatencyReport(void) {
if (config.latency_histogram->total_count == 0) {
if (config.csv) {
printf("\"%s\",\"0.00\",\"0.000\",\"0.000\",\"0.000\",\"0.000\",\"0.000\",\"0.000\"\n", config.title);
} else if (config.quiet) {
printf("%*s\r", config.last_printed_bytes, " "); // ensure there is a clean line
printf("%s: 0.00 requests per second, p50=0.000 msec\n", config.title);
} else {
printf("%*s\r", config.last_printed_bytes, " "); // ensure there is a clean line
printf("====== %s ======\n", config.title);
printf("No latency samples collected\n");
}
return;
}
const float reqpersec = (float)config.requests_finished/((float)config.totlatency/1000.0f);
const float reqpersec = config.totlatency > 0 ? (float)config.requests_finished/((float)config.totlatency/1000.0f) : 0.0f;
const float p0 = ((float) hdr_min(config.latency_histogram))/1000.0f;
const float p50 = hdr_value_at_percentile(config.latency_histogram, 50.0 )/1000.0f;
const float p95 = hdr_value_at_percentile(config.latency_histogram, 95.0 )/1000.0f;
@ -1666,13 +1679,15 @@ int showThroughput(struct aeEventLoop *eventLoop, long long id, void *clientData
return SHOW_THROUGHPUT_INTERVAL;
}
const float dt = (float)(current_tick-config.start)/1000.0;
const float rps = (float)requests_finished/dt;
const float rps = dt > 0 ? (float)requests_finished/dt : 0.0f;
const float instantaneous_dt = (float)(current_tick-config.previous_tick)/1000.0;
const float instantaneous_rps = (float)(requests_finished-previous_requests_finished)/instantaneous_dt;
const float instantaneous_rps = instantaneous_dt > 0 ? (float)(requests_finished-previous_requests_finished)/instantaneous_dt : 0.0f;
config.previous_tick = current_tick;
atomicSet(config.previous_requests_finished,requests_finished);
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, hdr_mean(config.current_sec_latency_histogram)/1000.0f, hdr_mean(config.latency_histogram)/1000.0f);
double avg_mean = config.current_sec_latency_histogram->total_count > 0 ? hdr_mean(config.current_sec_latency_histogram)/1000.0f : 0.0;
double overall_mean = config.latency_histogram->total_count > 0 ? hdr_mean(config.latency_histogram)/1000.0f : 0.0;
int printed_bytes = printf("%s: rps=%.1f (overall: %.1f) avg_msec=%.3f (overall: %.3f)\r", config.title, instantaneous_rps, rps, avg_mean, overall_mean);
config.last_printed_bytes = printed_bytes;
hdr_reset(config.current_sec_latency_histogram);
fflush(stdout);

View file

@ -10492,6 +10492,11 @@ static int displayKeyStatsSizeDist(struct hdr_histogram *keysize_histogram) {
struct hdr_iter iter;
int64_t last_displayed_cumulative_count = 0;
if (keysize_histogram->total_count == 0) {
line_count += cleanPrintfln("No key size samples collected");
return line_count;
}
hdr_iter_percentile_init(&iter, keysize_histogram, 1);
line_count += cleanPrintfln("Key size Percentile Total keys");

View file

@ -134,6 +134,15 @@ tags {"benchmark network external:skip logreqres:skip"} {
r get key
} {arg}
test {benchmark: no NaN or Inf in latency report with fast requests} {
# With -n 1 on localhost, totlatency can round to 0 ms. Verify showLatencyReport() handles this gracefully.
set cmd [redisbenchmark $master_host $master_port "-c 1 -n 1 -t set"]
set output [exec {*}$cmd 2>@1]
if {[regexp -nocase {nan|(?:^|[^a-z])inf(?:[^o]|$)} $output]} {
fail "redis-benchmark output contains NaN or Inf: $output"
}
}
# tls specific tests
if {$::tls} {
test {benchmark: specific tls-ciphers} {

View file

@ -849,4 +849,18 @@ start_server {tags {"cli external:skip"}} {
}
}
start_server {tags {"cli external:skip"}} {
test "keystats on empty database should not produce garbage stats" {
# On an empty DB the keystats histogram has total_count = 0. Verify hdr_mean(), hdr_stddev(),
# and the percentile calculation handle this gracefully.
set cmd [rediscli [srv host] [srv port] [list --keystats]]
catch {exec {*}$cmd 2>@1} result
assert_match "*Scanning the entire keyspace*" $result
# The "Note:" line with Mean/StdDeviation is only printed when displayKeyStatsSizeDist()
# compute stats. When keysize_histogram->total_count == 0, it should be skipped entirely.
assert_match "*No key size samples collected*" $result
}
}
file delete ./.rediscli_history_test