From e8e02f900cbf0fbaae6eb53e52ce9c4cc8318b99 Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Sun, 9 Jan 2022 17:04:18 -0800 Subject: Changed latency histogram output to omit trailing 0s and periods (#10075) Changed latency percentile output to omit trailing 0s and periods --- redis.conf | 2 +- src/config.c | 12 ++++++++---- src/server.c | 6 ++++-- src/util.c | 14 ++++++++++++++ src/util.h | 1 + tests/unit/info.tcl | 18 +++++++++--------- tests/unit/moduleapi/blockonbackground.tcl | 2 +- 7 files changed, 38 insertions(+), 17 deletions(-) diff --git a/redis.conf b/redis.conf index 4b8da181c..0753f3166 100644 --- a/redis.conf +++ b/redis.conf @@ -1781,7 +1781,7 @@ latency-monitor-threshold 0 # By default the exported latency percentiles via the INFO latencystats command # are the p50, p99, and p999. -# latency-tracking-info-percentiles 50.0 99.0 99.9 +# latency-tracking-info-percentiles 50 99 99.9 ############################# EVENT NOTIFICATION ############################## diff --git a/src/config.c b/src/config.c index 6dd4cc844..055c849ce 100644 --- a/src/config.c +++ b/src/config.c @@ -2698,8 +2698,10 @@ static sds getConfigLatencyTrackingInfoPercentilesOutputOption(typeData data) { UNUSED(data); sds buf = sdsempty(); for (int j = 0; j < server.latency_tracking_info_percentiles_len; j++) { - buf = sdscatprintf(buf,"%f", - server.latency_tracking_info_percentiles[j]); + char fbuf[128]; + size_t len = sprintf(fbuf, "%f", server.latency_tracking_info_percentiles[j]); + len = trimDoubleString(fbuf, len); + buf = sdscatlen(buf, fbuf, len); if (j != server.latency_tracking_info_percentiles_len-1) buf = sdscatlen(buf," ",1); } @@ -2718,8 +2720,10 @@ void rewriteConfigLatencyTrackingInfoPercentilesOutputOption(typeData data, cons line = sdscat(line," \"\""); } else { for (int j = 0; j < server.latency_tracking_info_percentiles_len; j++) { - line = sdscatprintf(line," %f", - server.latency_tracking_info_percentiles[j]); + char fbuf[128]; + size_t len = sprintf(fbuf, " %f", server.latency_tracking_info_percentiles[j]); + len = trimDoubleString(fbuf, len); + line = sdscatlen(line, fbuf, len); } } rewriteConfigRewriteLine(state,name,line,1); diff --git a/src/server.c b/src/server.c index 46b454ff3..2baf3bfb5 100644 --- a/src/server.c +++ b/src/server.c @@ -1782,7 +1782,6 @@ void initServerConfig(void) { server.page_size = sysconf(_SC_PAGESIZE); server.pause_cron = 0; - server.latency_tracking_enabled = 1; server.latency_tracking_info_percentiles_len = 3; server.latency_tracking_info_percentiles = zmalloc(sizeof(double)*(server.latency_tracking_info_percentiles_len)); server.latency_tracking_info_percentiles[0] = 50.0; /* p50 */ @@ -4628,7 +4627,10 @@ void bytesToHuman(char *s, unsigned long long n) { sds fillPercentileDistributionLatencies(sds info, const char* histogram_name, struct hdr_histogram* histogram) { info = sdscatfmt(info,"latency_percentiles_usec_%s:",histogram_name); for (int j = 0; j < server.latency_tracking_info_percentiles_len; j++) { - info = sdscatprintf(info,"p%f=%.3f", server.latency_tracking_info_percentiles[j], + char fbuf[128]; + size_t len = sprintf(fbuf, "%f", server.latency_tracking_info_percentiles[j]); + len = trimDoubleString(fbuf, len); + info = sdscatprintf(info,"p%s=%.3f", fbuf, ((double)hdr_value_at_percentile(histogram,server.latency_tracking_info_percentiles[j]))/1000.0f); if (j != server.latency_tracking_info_percentiles_len-1) info = sdscatlen(info,",",1); diff --git a/src/util.c b/src/util.c index d11a07655..75086db42 100644 --- a/src/util.c +++ b/src/util.c @@ -594,6 +594,20 @@ int d2string(char *buf, size_t len, double value) { return len; } +/* Trims off trailing zeros from a string representing a double. */ +int trimDoubleString(char *buf, size_t len) { + if (strchr(buf,'.') != NULL) { + char *p = buf+len-1; + while(*p == '0') { + p--; + len--; + } + if (*p == '.') len--; + } + buf[len] = '\0'; + return len; +} + /* Create a string object from a long double. * If mode is humanfriendly it does not use exponential format and trims trailing * zeroes at the end (may result in loss of precision). diff --git a/src/util.h b/src/util.h index e2b0d13ca..7dce8ff69 100644 --- a/src/util.h +++ b/src/util.h @@ -60,6 +60,7 @@ int string2ull(const char *s, unsigned long long *value); int string2l(const char *s, size_t slen, long *value); int string2ld(const char *s, size_t slen, long double *dp); int string2d(const char *s, size_t slen, double *dp); +int trimDoubleString(char *buf, size_t len); int d2string(char *buf, size_t len, double value); int ld2string(char *buf, size_t len, long double value, ld2string_mode mode); sds getAbsolutePath(char *filename); diff --git a/tests/unit/info.tcl b/tests/unit/info.tcl index 74c3a24c9..f9660f2c8 100644 --- a/tests/unit/info.tcl +++ b/tests/unit/info.tcl @@ -20,7 +20,7 @@ start_server {tags {"info" "external:skip"}} { assert_match {} [latency_percentiles_usec set] r CONFIG SET latency-tracking yes r set a b - assert_match {*p50.000000=*,p99.000000=*,p99.900000=*} [latency_percentiles_usec set] + assert_match {*p50=*,p99=*,p99.9=*} [latency_percentiles_usec set] r config resetstat assert_match {} [latency_percentiles_usec set] } @@ -31,12 +31,12 @@ start_server {tags {"info" "external:skip"}} { r CONFIG SET latency-tracking yes r SET a b r GET a - assert_match {*p50.000000=*,p99.000000=*,p99.900000=*} [latency_percentiles_usec set] - assert_match {*p50.000000=*,p99.000000=*,p99.900000=*} [latency_percentiles_usec get] + assert_match {*p50=*,p99=*,p99.9=*} [latency_percentiles_usec set] + assert_match {*p50=*,p99=*,p99.9=*} [latency_percentiles_usec get] r CONFIG SET latency-tracking-info-percentiles "0.0 50.0 100.0" - assert_match [r config get latency-tracking-info-percentiles] {latency-tracking-info-percentiles {0.000000 50.000000 100.000000}} - assert_match {*p0.000000=*,p50.000000=*,p100.000000=*} [latency_percentiles_usec set] - assert_match {*p0.000000=*,p50.000000=*,p100.000000=*} [latency_percentiles_usec get] + assert_match [r config get latency-tracking-info-percentiles] {latency-tracking-info-percentiles {0 50 100}} + assert_match {*p0=*,p50=*,p100=*} [latency_percentiles_usec set] + assert_match {*p0=*,p50=*,p100=*} [latency_percentiles_usec get] r config resetstat assert_match {} [latency_percentiles_usec set] } @@ -71,7 +71,7 @@ start_server {tags {"info" "external:skip"}} { wait_for_blocked_client r lpush list1{t} b assert_equal [$rd read] {list1{t} b} - assert_match {*p50.000000=*,p99.000000=*,p99.900000=*} [latency_percentiles_usec blpop] + assert_match {*p50=*,p99=*,p99.9=*} [latency_percentiles_usec blpop] $rd close } @@ -83,8 +83,8 @@ start_server {tags {"info" "external:skip"}} { r SET k v set latencystatline_debug [latency_percentiles_usec debug] set latencystatline_set [latency_percentiles_usec set] - regexp "p50.000000=(.+\..+)" $latencystatline_debug -> p50_debug - regexp "p50.000000=(.+\..+)" $latencystatline_set -> p50_set + regexp "p50=(.+\..+)" $latencystatline_debug -> p50_debug + regexp "p50=(.+\..+)" $latencystatline_set -> p50_set assert {$p50_debug >= 50000} assert {$p50_set >= 0} assert {$p50_debug >= $p50_set} diff --git a/tests/unit/moduleapi/blockonbackground.tcl b/tests/unit/moduleapi/blockonbackground.tcl index 14c5b4898..fcd7f1dd4 100644 --- a/tests/unit/moduleapi/blockonbackground.tcl +++ b/tests/unit/moduleapi/blockonbackground.tcl @@ -31,7 +31,7 @@ start_server {tags {"modules"}} { set latencystatline_debug [latency_percentiles_usec block.debug] regexp "calls=1,usec=(.*?),usec_per_call=(.*?),rejected_calls=0,failed_calls=0" $cmdstatline -> usec usec_per_call - regexp "p50.000000=(.+\..+)" $latencystatline_debug -> p50 + regexp "p50=(.+\..+)" $latencystatline_debug -> p50 assert {$usec >= 100000} assert {$usec_per_call >= 100000} assert {$p50 >= 100000} -- cgit v1.2.1