Thread: 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
Attachment
On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra <tv@fuzzy.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@fuzzy.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@fuzzy.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
Attachment
On Sun, Aug 26, 2012 at 1:04 PM, Tomas Vondra <tv@fuzzy.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@fuzzy.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@fuzzy.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@fuzzy.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@fuzzy.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
Attachment
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