On 6.1.2013 03:03, Tatsuo Ishii wrote: > As a committer, I have looked into the patch. I noticed two things: > > 1) In the help you put '-q' option into "Common options" section. I > think this should be moved to "Initialization options" section because > the option is only applied while initializing.
Good point, moved. > 2) Shouldn't a long option for '-q' be provided? Something like > '--quiet-progress-logging'? I don't think so. Currently pgbench has either short or long option, not both (for the same thing). I believe we should stick to this and either choose "-q" or "--quiet-logging" but not both. > 3) No patches for docs found (doc/src/sgml/pgbench.sgml) I've added a brief description of the "-q" option into the docs. IMHO it's enough but feel free to add some more details. There's one more thing I've just noticed - the original version of the patch simply removed the old logging, but this one keeps both old and quiet logging. But the old logging still uses this: fprintf(stderr, "%d of %d tuples (%d%%) done.\n", .... while the new logging does this fprintf(stderr, "%d of %d tuples (%d%%) done (elapsed %.2f s, remaining %.2f s).\n", i.e. it prints additional info about elapsed/estimated time. Do we want to keep it this way (i.e. not to mess with the old logging) or do we want to add these new fields to the old logging too? I suggest to add it to the old logging, to keep the log messages the same, the only difference being the logging frequency. Tomas
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c index e376452..31764fe 100644 --- a/contrib/pgbench/pgbench.c +++ b/contrib/pgbench/pgbench.c @@ -39,6 +39,7 @@ #include "portability/instr_time.h" #include <ctype.h> +#include <math.h> #ifndef WIN32 #include <sys/time.h> @@ -102,6 +103,7 @@ extern int optind; #define MAXCLIENTS 1024 #endif +#define LOG_STEP_SECONDS 5 /* seconds between log messages */ #define DEFAULT_NXACTS 10 /* default nxacts */ int nxacts = 0; /* number of transactions per client */ @@ -150,6 +152,7 @@ char *index_tablespace = NULL; #define naccounts 100000 bool use_log; /* log transaction latencies to a file */ +bool use_quiet; /* quiet logging onto stderr */ bool is_connect; /* establish connection for each transaction */ bool is_latencies; /* report per-command latencies */ int main_pid; /* main process id used in log filename */ @@ -359,6 +362,7 @@ usage(void) " -n do not run VACUUM after initialization\n" " -F NUM fill factor\n" " -s NUM scaling factor\n" + " -q quiet logging (one message each 5 seconds)\n" " --foreign-keys\n" " create foreign key constraints between tables\n" " --index-tablespace=TABLESPACE\n" @@ -1362,6 +1366,11 @@ init(bool is_no_vacuum) char sql[256]; int i; + /* used to track elapsed time and estimate of the remaining time */ + instr_time start, diff; + double elapsed_sec, remaining_sec; + int log_interval = 1; + if ((con = doConnect()) == NULL) exit(1); @@ -1430,6 +1439,8 @@ init(bool is_no_vacuum) } PQclear(res); + INSTR_TIME_SET_CURRENT(start); + for (i = 0; i < naccounts * scale; i++) { int j = i + 1; @@ -1441,10 +1452,33 @@ init(bool is_no_vacuum) exit(1); } - if (j % 100000 == 0) + /* If we want to stick with the original logging, print a message each + * 100k inserted rows. */ + if ((! use_quiet) && (j % 100000 == 0)) fprintf(stderr, "%d of %d tuples (%d%%) done.\n", - j, naccounts * scale, - (int) (((int64) j * 100) / (naccounts * scale))); + j, naccounts * scale, + (int) (((int64) j * 100) / (naccounts * scale))); + /* let's not call the timing for each row, but only each 100 rows */ + else if (use_quiet && (j % 100 == 0)) + { + INSTR_TIME_SET_CURRENT(diff); + INSTR_TIME_SUBTRACT(diff, start); + + elapsed_sec = INSTR_TIME_GET_DOUBLE(diff); + remaining_sec = (scale * naccounts - j) * elapsed_sec / j; + + /* have we reached the next interval (or end)? */ + if ((j == scale * naccounts) || (elapsed_sec >= log_interval * LOG_STEP_SECONDS)) { + + fprintf(stderr, "%d of %d tuples (%d%%) done (elapsed %.2f s, remaining %.2f s).\n", + j, naccounts * scale, + (int) (((int64) j * 100) / (naccounts * scale)), elapsed_sec, remaining_sec); + + /* skip to the next interval */ + log_interval = (int)ceil(elapsed_sec/LOG_STEP_SECONDS); + } + } + } if (PQputline(con, "\\.\n")) { @@ -1987,7 +2021,7 @@ main(int argc, char **argv) state = (CState *) pg_malloc(sizeof(CState)); memset(state, 0, sizeof(CState)); - while ((c = getopt_long(argc, argv, "ih:nvp:dSNc:j:Crs:t:T:U:lf:D:F:M:", long_options, &optindex)) != -1) + while ((c = getopt_long(argc, argv, "ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:", long_options, &optindex)) != -1) { switch (c) { @@ -2095,6 +2129,9 @@ main(int argc, char **argv) case 'l': use_log = true; break; + case 'q': + use_quiet = true; + break; case 'f': ttype = 3; filename = pg_strdup(optarg); diff --git a/doc/src/sgml/pgbench.sgml b/doc/src/sgml/pgbench.sgml index 91530ab..58686b1 100644 --- a/doc/src/sgml/pgbench.sgml +++ b/doc/src/sgml/pgbench.sgml @@ -190,6 +190,17 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</> </varlistentry> <varlistentry> + <term><option>-q</option></term> + <listitem> + <para> + Switch logging to quiet mode, producing only one progress message per 5 + seconds. The default logging prints one message each 100000 rows, which + often outputs many lines per second (especially on good hardware). + </para> + </listitem> + </varlistentry> + + <varlistentry> <term><option>--foreign-keys</option></term> <listitem> <para>
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers