Re: Print latency percentiles in pgbench

Hello,

+1 to the NaN rejection, tests, and documenting the '=' form.

One small addition: duplicates like --latency-percentiles=50,50
are accepted; probably fine, but worth confirming that's intentional.

Best,
Phil Alger

--
EDB

On Wednesday, November 4th, 2026 at 08:51, Hữu Đôn Hoàng <hoanghuudon02hp(at)gmail(dot)com> wrote:

> Hi Roman,
>
> I've continued reviewing the patch, on master at 30faf13889b.
>
> 1. --latency-percentiles=nan passes the range check and then fails
> an assertion:
>
> $ pgbench --latency-percentiles=nan -t 10 bench
> pgbench: pgbench.c:1517: latencyHistogramPercentile: Assertion
> `p > 0.0 && p <= 100.0' failed.
>
> Every comparison with NaN is false, so "percentile <= 0.0 ||
> percentile > 100.0" does not reject it. Writing the check as
> !(percentile > 0.0 && percentile <= 100.0) would.
>
> 2. The patch adds no tests. It would be good to cover the parse
> errors (0, 101, abc, ",50", 11 values, nan) in
> 002_pgbench_no_server.pl and check the new output lines in
> 001_pgbench_with_server.pl.
>
> 3. Because the argument is optional, the list has to be attached
> with "=". With a space, the list is taken as the database name:
>
> $ pgbench --latency-percentiles 50,99 -t 10 bench
> pgbench: error: too many command-line arguments (first is "bench")
>
> $ pgbench --latency-percentiles 50,99 -t 10
> pgbench: error: connection to server ... failed: FATAL: database
> "50,99" does not exist
>
> Neither message points at the option, so I think the docs should
> say that the value must be given as --latency-percentiles=LIST.
>
> 4. The docs say the reported percentile "lies within the histogram
> bucket that contains the requested rank", but not how wide a
> bucket is. It would help to state the resolution (about 3% of the
> value) and the cases where it does not hold:
>
> - A percentile can fall outside [min, max] because of the
> interpolation inside a bucket. With a script containing only
> "\sleep 2 ms" (500 transactions) I got:
>
> latency min = 2.023 ms
> latency p0.1 = 2.016 ms
>
> Clamping the result to the min/max that SimpleStats already
> tracks would avoid this.
>
> - Below 32 us an octave is a single bucket, so nothing is
> interpolated: for 1000 latencies between 20 and 29 us, p1, p50
> and p99 all come out as 16 us.
>
> Regards,
> Don Hoang
>
>
>
>
>