Issues with pgbench's rate limiting (and some more)

Hi,

There are a few things that imo make pgbench's rate limiting a lot less useful
than it could be.

My main complaint is that using a poisson distribution may be useful for some
uses of pgbench, but for others it's terrible:

If one wants to reduce server-side variations of query execution time, adding
random delays on the client side isn't useful. To show how useless the
current -R is for the above purpose, take this example, where the server-side
is infinitely fast:

pgbench --random-seed=0 -n -R100 -c1 -Mprepared -f <(echo "\set a 1") -T10 -P1
pgbench: setting random seed to 0
pgbench (20devel)
progress: 1.0 s, 89.0 tps, lat 0.122 ms stddev 0.021, 0 failed, lag 0.119 ms
progress: 2.0 s, 92.0 tps, lat 0.122 ms stddev 0.021, 0 failed, lag 0.119 ms
progress: 3.0 s, 105.0 tps, lat 0.122 ms stddev 0.023, 0 failed, lag 0.120 ms
progress: 4.0 s, 107.0 tps, lat 0.119 ms stddev 0.024, 0 failed, lag 0.117 ms
progress: 5.0 s, 97.0 tps, lat 0.124 ms stddev 0.019, 0 failed, lag 0.122 ms
progress: 6.0 s, 107.0 tps, lat 0.120 ms stddev 0.025, 0 failed, lag 0.118 ms
progress: 7.0 s, 92.0 tps, lat 0.126 ms stddev 0.017, 0 failed, lag 0.123 ms
progress: 8.0 s, 130.0 tps, lat 0.122 ms stddev 0.021, 0 failed, lag 0.119 ms
progress: 9.0 s, 99.0 tps, lat 0.118 ms stddev 0.027, 0 failed, lag 0.116 ms
...

The poisson randomness hides any potential signal (if this were measuring actual
queries).

It's imo *important* to be able to measure at a reduced rate. Real world
latency sensitive workloads should *never* be run full tilt, that guarantees
highly variable latency. At the same time it *is* important for postgres to
improve tail latencies when *not* running full tilt (and we have some problems
around that), but that can't currently be usefully measured with pgbench.

Therefore I'd like to add support for a fixed rate scheduling mode. Maybe
--rate-distribution {poisson,fixed}? For compat I'd probably leave poisson
the default with just -R, even though I personally think it's far less
commonly useful than a fixed rate limit.

My second complaint is that the poisson distribution as implemented today is
not very useful. There are a two main issues imo:

1) Having the poisson distribution implemented on a per-thread basis doesn't
really model anything commonly useful.

Take e.g. the typical bank teller example - it's hardly common for a
customer to choose a queue for a teller independent of the size of the
queue. Either one would have one queue for multiple tellers, or a customer
would choose one of the shorter queues.

This is true for common web application implementations as well - the
connection that already is in use for query processing won't be chosen to
execute a specific new incoming request. Instead the first web
process/thread that is done processing the last request will choose one of
the incoming requests, using either the connection it used last, or one of
the currently unused connections.

2) Having the poisson distribution be per-thread state doesn't make sense

Today the throttling prng state lives in the per-thread state. That makes
afaict no sense whatsoever. I don't think anybody would expect that
-c 10 -j2
would give a completely different distribution than
-c 10 -j10
but they model something rather fundamentally different. Without that
difference being documented anywhere.

It gets even weirder with ba3deeefb0f (-c doesn't need to be a multiple of
-j anymore), because the number of connections per-thread can be uneven.

I suspect we might have ended up with this by accident. In the past pgbench
supported fork based concurrency, which would have a hard time sharing global
state for something like the prng state. As long as all connections use the
same thread / process, the rate limiting *is* global. But with -j > 1 that's
not the case anymore.

I think at the very least we either need to use one global randomness
throttling state, protected by a lock, or per-thread state. The latter doesn't
make the distribution particularly meaningful, but at least makes it
consistent across different -c -j combinations.

I guess I could see an argument for adding
--rate-distribution {poisson-global,poisson-connection,fixed}

Since I mainly care about having fixed, I'd probably just go for making the
poisson state per-connection without adding the global one, but I could easily
be convinced otherwise. One argument for just implementing poisson-connection
mode is that we IIRC don't yet have a proper locking implementation that
pgbench could use.

A third issue is that the rounding for -R seems to introduce a bias due to
rounding. It's probably not that big a deal in real world cases, but e.g. with
something like
--random-seed=0 -n -R1000000 -c1 -j1 -Mprepared -f <(echo "\set a 1") -T10 -P1
you get
...
progress: 5.0 s, 1042672.6 tps, lat 0.029 ms stddev 0.017, 0 failed, lag 0.029 ms
progress: 6.0 s, 1041477.0 tps, lat 0.029 ms stddev 0.017, 0 failed, lag 0.029 ms
progress: 7.0 s, 1041558.0 tps, lat 0.029 ms stddev 0.017, 0 failed, lag 0.029 ms
...
tps = 1041827.372031 (without initial connection time)

I.e. the results are biased to be higher than the rate.

The problem is that the throttle trigger is incremented by the rounded-down
results of getPoissonRand() for every transaction.

I think we need to make the throttle trigger a double and instead round the
total for the schedule time. I think there's no reason to keep the current
behavior around.

A fourth issue I have with pgbench -R's is one I complained about before
[1]. When -R is used, pgbench -P's "lat" and the final "latency average" do
not measure the actual execution time of the executed script anymore, instead
they report the execution time + scheduling lag. Which imo is so misleading
to be wrong.

pgbench --random-seed=0 -n -c1 -j1 -Mprepared -S -T10 -P1 -R55000
...
progress: 2.0 s, 55202.8 tps, lat 0.246 ms stddev 0.201, 0 failed, lag 0.229 ms
progress: 3.0 s, 55203.6 tps, lat 2.047 ms stddev 2.324, 0 failed, lag 2.029 ms
progress: 4.0 s, 54660.5 tps, lat 0.457 ms stddev 0.650, 0 failed, lag 0.439 ms
progress: 5.0 s, 54758.2 tps, lat 0.313 ms stddev 0.320, 0 failed, lag 0.295 ms
...
latency average = 0.481 ms
latency stddev = 0.963 ms
rate limit schedule lag: avg 0.463 (max 7.245) ms

Reporting a latency of 0.457 ms for something executed ~55k times a second
over one connection is just utterly bogus. Quite obviously it is not possible
to execute something 55k times in one second if one execution takes 0.457 ms.

I know it is documented:

Under throttling (-R), the latency is computed with respect to the
transaction scheduled start time, not the actual transaction beginning
time, thus it also includes the average schedule lag time

but that doesn't make it sane, it just documents insanity.

In that prior thread there were some arguments made for the current behavior,
but I continue to think that they don't make much sense. Part of the argument
was that one can just compute the "actual" latency values from the
output. While that is true for the -P output, I don't think that's really true
for "latency stddev".

I'm planning to send a patch that replaces the current 'lat' with a 'txtime'
column that's always unaffected by -R and, if -R is used, a 'txlat' column
that shows the current lag value. I'd personally not add 'txlat', as 'lat'
seems enough, but Heikki and Fabien seemed to want that.

Another issue, not really related to -R but not really worth starting a
separate thread about, is that currently the choice of script is per-thread
state, but should probably be per-connection state. As-is, --random-seed=x
doesn't really lead to a repeatable choice of scripts during an execution,
unless -j == -c. ISTM that ts_choose_rs should just be moved to CState. I'd
probably also move ts_sample_rs, mainly because it would be the only remaining
per-thread state; it seems simpler to just have all of these be
per-connection.

Yet another independent issue that I happened to come across while writing
this up:

pgbench --random-seed=0 -n -R5 -c5 -j5 -Mprepared -f <(echo "\set a 1") -T10 -P1
progress: 1.0 s, 9.0 tps, lat 0.129 ms stddev 0.013, 0 failed, lag 0.124 ms
progress: 2.0 s, 1.0 tps, lat 0.129 ms stddev NaN, 0 failed, lag 0.126 ms
progress: 3.0 s, 3.0 tps, lat 0.109 ms stddev 0.029, 0 failed, lag 0.107 ms
progress: 4.0 s, 6.0 tps, lat 0.097 ms stddev 0.025, 0 failed, lag 0.094 ms
progress: 5.0 s, 3.0 tps, lat 0.113 ms stddev 0.019, 0 failed, lag 0.110 ms
transaction type: /dev/fd/63
...

Note the -P output stops after 5 seconds. Afaict that's not *really* related
to -P, it just happens to be an easy way of hitting it reproducibly. Afaict
the problem is that the progress report is printed in the main thread and that
after the 5s report the chosen time for the next transaction is after the end
time of the benchmark and thus transitions to CSTATE_FINISHED.

Not quite sure what the best fix here is. Easiest might be just to have a
separate loop in thread 0 after it's done?

Thoughts?

Not including patches yet, I didn't want to spend time cleaning up what I have
when the naming and behavioral details aren't yet nailed down.

Greetings,

Andres Freund

[1] https://postgr.es/m/20190611045631.qyball7zakgvknea%40alap3.anarazel.de