Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
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 t...@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.
Tomasdiff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 00cab73..578aeb3 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -130,6 +130,11 @@ intforeign_keys = 0;
intunlogged_tables = 0;
/*
+ * use log sampling (rate = 1 = 100%, 0 = don't use)
+ */
+double use_sample_rate = 0.0;
+
+/*
* tablespace selection
*/
char *tablespace = NULL;
@@ -364,6 +369,8 @@ usage(void)
-f FILENAME read transaction script from FILENAME\n
-j NUM number of threads (default: 1)\n
-l write transaction times to log file\n
+--sampling-rate NUM\n
+ sampling rate of the log (e.g. 0.01 for 1%%
sample)\n
-M simple|extended|prepared\n
protocol for submitting queries to server
(default: simple)\n
-n do not run VACUUM before tests\n
@@ -877,21 +884,26 @@ top:
instr_time diff;
double usec;
- INSTR_TIME_SET_CURRENT(now);
- diff = now;
- INSTR_TIME_SUBTRACT(diff, st-txn_begin);
- usec = (double) INSTR_TIME_GET_MICROSEC(diff);
+ /* either no sampling or is within the sample */
+ if ((use_sample_rate == 0.0) ||
(pg_erand48(thread-random_state) = use_sample_rate))
+ {
+
+ INSTR_TIME_SET_CURRENT(now);
+ diff = now;
+ INSTR_TIME_SUBTRACT(diff, st-txn_begin);
+ usec = (double) INSTR_TIME_GET_MICROSEC(diff);
#ifndef WIN32
- /* This is more than we really ought to know about
instr_time */
- fprintf(logfile, %d %d %.0f %d %ld %ld\n,
- st-id, st-cnt, usec, st-use_file,
- (long) now.tv_sec, (long) now.tv_usec);
+ /* This is more than we really ought to know
about instr_time */
+ fprintf(logfile, %d %d %.0f %d %ld %ld\n,
+ st-id, st-cnt, usec,
st-use_file,
+ (long) now.tv_sec, (long)
now.tv_usec);
#else
- /* On Windows, instr_time doesn't provide a timestamp
anyway */
- fprintf(logfile, %d %d %.0f %d 0 0\n,
- st-id, st-cnt, usec, st-use_file);
+ /* On Windows, instr_time doesn't provide a
timestamp anyway */
+ fprintf(logfile, %d %d %.0f %d 0 0\n,
+ st-id, st-cnt, usec,
st-use_file);
#endif
+ }
}
if (commands[st-state]-type == SQL_COMMAND)
@@ -1918,6 +1930,7 @@ main(int argc, char **argv)
{index-tablespace, required_argument, NULL, 3},
{tablespace, required_argument, NULL, 2},
{unlogged-tables, no_argument, unlogged_tables, 1},
+ {sampling-rate, required_argument, NULL, 4},
{NULL, 0, NULL, 0}
};
@@ -2123,6 +2136,14 @@ main(int argc, char **argv)
case 3: /* index-tablespace */
index_tablespace = optarg;
break;
+ case 4:
+ use_sample_rate = atof(optarg)/100;
+ if (use_sample_rate = 0.0 ||
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On Sun, Aug 26, 2012 at 1:04 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On 30 Srpen 2012, 17:46, Robert Haas wrote: On Sun, Aug 26, 2012 at 1:04 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On 30 Srpen 2012, 23:44, Robert Haas wrote: On Thu, Aug 30, 2012 at 3:48 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On Thu, Aug 30, 2012 at 3:48 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
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 t...@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
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 00cab73..12c1338 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -145,6 +145,9 @@ char *index_tablespace = NULL;
#define naccounts 10
bool use_log;/* log transaction latencies to
a file */
+bool use_log_sampling; /* sample the log randomly */
+intnsample_rate = 100; /* default log sampling rate */
+
bool is_connect; /* establish connection for
each transaction */
bool is_latencies; /* report per-command latencies */
intmain_pid; /* main process id used
in log filename */
@@ -364,6 +367,7 @@ usage(void)
-f FILENAME read transaction script from FILENAME\n
-j NUM number of threads (default: 1)\n
-l write transaction times to log file\n
+-R NUM log sampling rate in pct (default: 100)\n
-M simple|extended|prepared\n
protocol for submitting queries to server
(default: simple)\n
-n do not run VACUUM before tests\n
@@ -877,21 +881,25 @@ top:
instr_time diff;
double usec;
- INSTR_TIME_SET_CURRENT(now);
- diff = now;
- INSTR_TIME_SUBTRACT(diff, st-txn_begin);
- usec = (double) INSTR_TIME_GET_MICROSEC(diff);
+ /* either no sampling or is within the sample */
+ if ((! use_log_sampling) || (getrand(thread, 0, 99)
nsample_rate)) {
+
+ INSTR_TIME_SET_CURRENT(now);
+ diff = now;
+ INSTR_TIME_SUBTRACT(diff, st-txn_begin);
+ usec = (double) INSTR_TIME_GET_MICROSEC(diff);
#ifndef WIN32
- /* This is more than we really ought to know about
instr_time */
- fprintf(logfile, %d %d %.0f %d %ld %ld\n,
- st-id, st-cnt, usec, st-use_file,
- (long) now.tv_sec, (long) now.tv_usec);
+ /* This is more than we really ought to know
about instr_time */
+ fprintf(logfile, %d %d %.0f %d %ld %ld\n,
+ st-id, st-cnt, usec,
st-use_file,
+ (long) now.tv_sec, (long)
now.tv_usec);
#else
- /* On Windows, instr_time doesn't provide a timestamp
anyway */
- fprintf(logfile, %d %d %.0f %d 0 0\n,
- st-id, st-cnt, usec, st-use_file);
+ /* On Windows, instr_time doesn't provide a
timestamp anyway */
+ fprintf(logfile, %d %d %.0f %d 0 0\n,
+
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PATCH: pgbench - random sampling of transaction written into log
On 26.8.2012 00:19, Jeff Janes wrote: On Fri, Aug 24, 2012 at 2:16 PM, Tomas Vondra t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
