PATCH: pgbench - random sampling of transaction written into log

Hi,

attached is a patch that adds support for random sampling in pgbench, when
it's executed with "-l" flag. You can do for example this:

$ pgbench -l -T 120 -R 1 db

and then only 1% of transactions will be written into the log file. If you
omit the tag, all the transactions are written (i.e. it's backward
compatible).

Recently I've been using pgbench on hardware that can handle a lot of
transactions (various flavors of SSDs, lots of RAM, ...), and in such
cases the log files may get pretty big (even several gigabytes for a
single 1-hour run). A reasonable random sample (along with the stats
provided by pgbench itself) is often more than enough in such cases.

kind regards
Tomas

On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> Hi,
>
> attached is a patch that adds support for random sampling in pgbench, when
> it's executed with "-l" flag. You can do for example this:
>
> $ pgbench -l -T 120 -R 1 db
>
> and then only 1% of transactions will be written into the log file. If you
> omit the tag, all the transactions are written (i.e. it's backward
> compatible).

Hi Tomas,

You use the rand() function. Isn't that function not thread-safe?
Or, if it is thread-safe, does it accomplish that with a mutex? That
was a problem with a different rand function used in pgbench that
Robert Haas fixed a while ago, 4af43ee3f165c8e4b332a7e680.

Also, what benefit is had by using modulus on rand(), rather than just
modulus on an incrementing counter?

Could you explain the value of this patch, given your other one that
does aggregation? If both were accepted, I think I would always use
the aggregation one in preference to this one.

Thanks,

Jeff

On 26.8.2012 00:19, Jeff Janes wrote:
> On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> Hi,
>>
>> attached is a patch that adds support for random sampling in pgbench, when
>> it's executed with "-l" flag. You can do for example this:
>>
>> $ pgbench -l -T 120 -R 1 db
>>
>> and then only 1% of transactions will be written into the log file. If you
>> omit the tag, all the transactions are written (i.e. it's backward
>> compatible).
>
> Hi Tomas,
>
> You use the rand() function. Isn't that function not thread-safe?
> Or, if it is thread-safe, does it accomplish that with a mutex? That
> was a problem with a different rand function used in pgbench that
> Robert Haas fixed a while ago, 4af43ee3f165c8e4b332a7e680.

Hi Jeff,

Aha! Good catch. I've used rand() which seems to be neither reentrant or
thread-safe (unless the man page is incorrect). Anyway, using pg_erand48
or getrand seems like an appropriate fix.

> Also, what benefit is had by using modulus on rand(), rather than just
> modulus on an incrementing counter?

Hmm, I was thinking about that too, but I wasn't really sure how would
that behave with multiple SQL files etc. But now I see the files are
actually chosen randomly, so using a counter seems like a good idea.

> Could you explain the value of this patch, given your other one that
> does aggregation? If both were accepted, I think I would always use
> the aggregation one in preference to this one.

The difference is that the sample contains information that is simply
unavailable in the aggregated output. For example when using multiple
files, you can compute per-file averages from the sample, but the
aggregated output contains just a single line for all files combined.

Tomas

On 26.8.2012 02:48, Tomas Vondra wrote:
> On 26.8.2012 00:19, Jeff Janes wrote:
>> On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>>> Hi,
>>>
>>> attached is a patch that adds support for random sampling in pgbench, when
>>> it's executed with "-l" flag. You can do for example this:
>>>
>>> $ pgbench -l -T 120 -R 1 db
>>>
>>> and then only 1% of transactions will be written into the log file. If you
>>> omit the tag, all the transactions are written (i.e. it's backward
>>> compatible).
>>
>> Hi Tomas,
>>
>> You use the rand() function. Isn't that function not thread-safe?
>> Or, if it is thread-safe, does it accomplish that with a mutex? That
>> was a problem with a different rand function used in pgbench that
>> Robert Haas fixed a while ago, 4af43ee3f165c8e4b332a7e680.
>
> Hi Jeff,
>
> Aha! Good catch. I've used rand() which seems to be neither reentrant or
> thread-safe (unless the man page is incorrect). Anyway, using pg_erand48
> or getrand seems like an appropriate fix.
>
>> Also, what benefit is had by using modulus on rand(), rather than just
>> modulus on an incrementing counter?
>
> Hmm, I was thinking about that too, but I wasn't really sure how would
> that behave with multiple SQL files etc. But now I see the files are
> actually chosen randomly, so using a counter seems like a good idea.

Attached is an improved patch, with a call to rand() replaced with
getrand().

I was thinking about the counter but I'm not really sure how to handle
cases like "39%" - I'm not sure a plain (counter % 100 < 37) is not a
good sampling, because it always keeps continuous sequences of
transactions. Maybe there's a clever way to use a counter, but let's
stick to a getrand() unless we can prove is't causing issues. Especially
considering that a lot of data won't be be written at all with low
sampling rates.

kind regards
Tomas

On Sun, Aug 26, 2012 at 1:04 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> Attached is an improved patch, with a call to rand() replaced with
> getrand().
>
> I was thinking about the counter but I'm not really sure how to handle
> cases like "39%" - I'm not sure a plain (counter % 100 < 37) is not a
> good sampling, because it always keeps continuous sequences of
> transactions. Maybe there's a clever way to use a counter, but let's
> stick to a getrand() unless we can prove is't causing issues. Especially
> considering that a lot of data won't be be written at all with low
> sampling rates.

I like this patch, and I think sticking with a random number is a good
idea. But I have two suggestions. Number one, I think the sampling
rate should be stored as a float, not an integer, because I can easily
imagine wanting a sampling rate that is not an integer percentage -
especially, one that is less than one percent, like half a percent or
a tenth of a percent. Also, I suggest that the command-line option
should be a long option rather than a single character option. That
will be more mnemonic and avoid using up too many single letter
options, of which there is a limited supply. So to sample every
hundredth result, you could do something like this:

pgbench --latency-sample-rate 0.01

Another option I personally think would be useful is an option to
record only those latencies that are above some minimum bound, like
this:

pgbench --latency-only-if-more-than $MICROSECONDS

The problem with recording all the latencies is that it tends to have
a material impact on throughput. Your patch should address that for
the case where you just want to characterize the latency, but it would
also be nice to have a way of recording the outliers.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 30 Srpen 2012, 17:46, Robert Haas wrote:
> On Sun, Aug 26, 2012 at 1:04 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> Attached is an improved patch, with a call to rand() replaced with
>> getrand().
>>
>> I was thinking about the counter but I'm not really sure how to handle
>> cases like "39%" - I'm not sure a plain (counter % 100 < 37) is not a
>> good sampling, because it always keeps continuous sequences of
>> transactions. Maybe there's a clever way to use a counter, but let's
>> stick to a getrand() unless we can prove is't causing issues. Especially
>> considering that a lot of data won't be be written at all with low
>> sampling rates.
>
> I like this patch, and I think sticking with a random number is a good
> idea. But I have two suggestions. Number one, I think the sampling
> rate should be stored as a float, not an integer, because I can easily
> imagine wanting a sampling rate that is not an integer percentage -
> especially, one that is less than one percent, like half a percent or
> a tenth of a percent. Also, I suggest that the command-line option
> should be a long option rather than a single character option. That
> will be more mnemonic and avoid using up too many single letter
> options, of which there is a limited supply. So to sample every
> hundredth result, you could do something like this:
>
> pgbench --latency-sample-rate 0.01

Right, I was thinking about that too. I'll do that in the next version of
the patch.

> Another option I personally think would be useful is an option to
> record only those latencies that are above some minimum bound, like
> this:
>
> pgbench --latency-only-if-more-than $MICROSECONDS
>
> The problem with recording all the latencies is that it tends to have
> a material impact on throughput. Your patch should address that for
> the case where you just want to characterize the latency, but it would
> also be nice to have a way of recording the outliers.

That sounds like a pretty trivial patch. I've been thinking about yet
another option - histograms (regular or with exponential bins).

What I'm not sure about is which of these options should be allowed at the
same time - to me, combinations like 'sampling + aggregation' don't make
much sense. Maybe except 'latency-only-if-more-than + aggregation'.

Tomas

On Thu, Aug 30, 2012 at 3:48 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> That sounds like a pretty trivial patch. I've been thinking about yet
> another option - histograms (regular or with exponential bins).

I thought about that, too, but I think high-outliers is a lot more
useful. At least for the kinds of things I worry about.

> What I'm not sure about is which of these options should be allowed at the
> same time - to me, combinations like 'sampling + aggregation' don't make
> much sense. Maybe except 'latency-only-if-more-than + aggregation'.

Maybe, but perhaps not even. I don't have a problem with saying that
the user is allowed to pick at most one method of reducing the output
volume. I think we could say: either sample, or take high outliers,
or aggregate. If we want to make some of those work in combination,
fine, but I don't think it's absolutely required. What WILL be
required is a clear error message telling you what you did wrong if
you use an unsupported combination.

BTW, I think that using any of these options should automatically
enable -l, rather than requiring it to be specified separately.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 30 Srpen 2012, 23:44, Robert Haas wrote:
> On Thu, Aug 30, 2012 at 3:48 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> That sounds like a pretty trivial patch. I've been thinking about yet
>> another option - histograms (regular or with exponential bins).
>
> I thought about that, too, but I think high-outliers is a lot more
> useful. At least for the kinds of things I worry about.

OK, let's fix the current patches first. We can add more options later.

>
>> What I'm not sure about is which of these options should be allowed at
>> the
>> same time - to me, combinations like 'sampling + aggregation' don't make
>> much sense. Maybe except 'latency-only-if-more-than + aggregation'.
>
> Maybe, but perhaps not even. I don't have a problem with saying that
> the user is allowed to pick at most one method of reducing the output
> volume. I think we could say: either sample, or take high outliers,
> or aggregate. If we want to make some of those work in combination,
> fine, but I don't think it's absolutely required. What WILL be
> required is a clear error message telling you what you did wrong if
> you use an unsupported combination.

I'll allow a single option - we can enable combinations that make sense
later.

>
> BTW, I think that using any of these options should automatically
> enable -l, rather than requiring it to be specified separately.

Good idea.

Tomas

Dne 31.08.2012 00:01, Tomas Vondra napsal:
> On 30 Srpen 2012, 23:44, Robert Haas wrote:
>> On Thu, Aug 30, 2012 at 3:48 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>>> That sounds like a pretty trivial patch. I've been thinking about
>>> yet
>>> another option - histograms (regular or with exponential bins).
>>
>> I thought about that, too, but I think high-outliers is a lot more
>> useful. At least for the kinds of things I worry about.
>
> OK, let's fix the current patches first. We can add more options
> later.

So, here is a fixed version of the patch. I've made these changes:

1) The option is now '--sampling-rate' instead of '-R' and accepts
float arguments. I've decided not to use 0.01 = 1% but use 1 = 1%, so it
accepts values between 0 and 100. I find this more natural.

2) I've removed one of the two new variables, the remaining one is used
both to check whether the sampling is enabled and keep the value.

3) I've decided not to enable "-l" whenever the "--sampling-rate" is
used. Again, I find this a bit less magic behavior.

4) I've fixed some formatting issues - if you notice another one that I
don't see, let me know.

Tomas

On 03.09.2012 01:40, Tomas Vondra wrote:
> So, here is a fixed version of the patch. I've made these changes:

Committed with some minor kibitzing.

> 1) The option is now '--sampling-rate' instead of '-R' and accepts float
> arguments. I've decided not to use 0.01 = 1% but use 1 = 1%, so it
> accepts values between 0 and 100. I find this more natural.

I find a fraction, ie. 0.01 = 1% more natural, so I changed it back to
that. I realize this is bike-shedding, but I believe Robert also found
this more natural, as that's what he suggested upthread.

I edited the comments and variable names a little bit, and also added a
small paragraph to the pgbench user manual about this under
"Per-Transaction Logging", in addition to the explanation under
"Benchmarking Options"

- Heikki