Re: pgbench throttling latency limit

On 09/09/2014 03:35 PM, Fabien COELHO wrote:
>
> Hello Heikki,
>
>>> I think we have to reconsider what we're reporting in 9.4, when --rate
>>> is enabled, even though it's already very late in the release cycle.
>>> It's a bad idea to change the definition of latency between 9.4 and 9.5,
>>> so let's get it right in 9.4.
>
> Indeed.
>
>> As per the attached patch. I think we should commit this to 9.4. Any
>> objections?
>
> Ok for me. Some more propositions about the doc below.

I looked closer at the this, and per Jan's comments, realized that we
don't log the lag time in the per-transaction log file. I think that's a
serious omission; when --rate is used, the schedule lag time is
important information to make sense of the result. I think we have to
apply the attached patch, and backpatch to 9.4. (The documentation on
the log file format needs updating)

Also, this is bizarre:

> /*
> * Use inverse transform sampling to randomly generate a delay, such
> * that the series of delays will approximate a Poisson distribution
> * centered on the throttle_delay time.
> *
> * 10000 implies a 9.2 (-log(1/10000)) to 0.0 (log 1) delay
> * multiplier, and results in a 0.055 % target underestimation bias:
> *
> * SELECT 1.0/AVG(-LN(i/10000.0)) FROM generate_series(1,10000) AS i;
> * = 1.000552717032611116335474
> *
> * If transactions are too slow or a given wait is shorter than a
> * transaction, the next transaction will start right away.
> */
> int64 wait = (int64) (throttle_delay *
> 1.00055271703 * -log(getrand(thread, 1, 10000) / 10000.0));

We're using getrand() to generate a uniformly distributed random value
between 1 and 10000, and then convert it to a double between 0.0 and
1.0. But getrand() is implemented by taking a double between 0.0 and 1.0
and converting it an integer, so we're just truncating the original
floating point number unnecessarily. I think we should add a new
function, getPoissonRand(), that uses pg_erand48() directly. We already
have similiar getGaussianRand() and getExponentialRand() functions.
Barring objections, I'll prepare another patch to do that, and backpatch
to 9.4.
- Heikki