On Wed, Nov 2, 2016 at 1:41 AM, Fabien COELHO <coe...@cri.ensmp.fr> wrote:
>
>> The log file generated by pgbench -l option is fixed file name
>> 'pgbench_log.<pid>.<thread id>'. And it's a little complicated for the
>> script that runs pgbench repeatedly to identify the log file name.
>> Attached patch make it possible to specify the log file name. I think
>> it's useful for the use who want to run pgbench repeatedly in script
>> and collects and analyze the result.
>>
>> The one thing I concern is that this patch changes -l option so that
>> it requires argument.
>> But changing its behavior would be good rather than adding new option.
>>
>> Please give me feedback.
>
>
> Patch applies but does not compile, because "logfilename" is not declared.
> I guess "logfile" was meant instead.
>
> I understand and agree that in some case having only a predefined file
> prefix in the current directory as the only option can be a hindrance for
> scripts which use pgbench and rely on the log.
>
> I'm not at ease either with changing the behavior of such an option, as some
> people may be happy with it and some script may be using it. I would suggest
> not to do so.
>
> Moreover, what is provided is not a file name, but a prefix used to build
> file names.
>
> So I would suggest to:
> - fix the compilation issue
> - leave -l/--log as it is, i.e. use "pgbench_log" as a prefix
> - add --log-prefix=... (long option only) for changing this prefix
>
Thank you for reviewing this patch!
I agree. It's better to add the separated option to specify the prefix
of log file instead of changing the existing behaviour. Attached
latest patch incorporated review comments.
Please review it.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 285608d..100cdb0 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -614,6 +614,16 @@ pgbench <optional> <replaceable>options</> </optional> <replaceable>dbname</>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--log-prefix=<replaceable>prefix</></option></term>
+ <listitem>
+ <para>
+ Prefix of transaction log file. If this option is not given,
+ <replaceable>pgbench_log</replaceable> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
@@ -1121,15 +1131,17 @@ END;
With the <option>-l</> option but without the <option>--aggregate-interval</option>,
<application>pgbench</> writes the time taken by each transaction
to a log file. The log file will be named
- <filename>pgbench_log.<replaceable>nnn</></filename>, where
+ <filename><replaceable>pgbench_log</replaceable>.<replaceable>nnn</></filename>,
+ where <replaceable>pgbench_log</replaceable> is the prefix of log file that can
+ be changed by specifying <option>--log-prefix</option>, and where
<replaceable>nnn</> is the PID of the <application>pgbench</application> process.
If the <option>-j</> option is 2 or higher, creating multiple worker
threads, each will have its own log file. The first worker will use the
same name for its log file as in the standard single worker case.
The additional log files for the other workers will be named
- <filename>pgbench_log.<replaceable>nnn</>.<replaceable>mmm</></filename>,
- where <replaceable>mmm</> is a sequential number for each worker starting
- with 1.
+ <filename><replaceable>pgbench_log</replaceable>.<replaceable>nnn</>.<replaceable>mmm</></filename>,
+ where <replaceable>mmm</> is a sequential number
+ for each worker starting with 1.
</para>
<para>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index d44cfda..32ffdff 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -180,6 +180,7 @@ char *pghost = "";
char *pgport = "";
char *login = NULL;
char *dbName;
+char *logfile_prefix;
const char *progname;
#define WSEP '@' /* weight separator */
@@ -511,6 +512,7 @@ usage(void)
" --aggregate-interval=NUM aggregate data over NUM seconds\n"
" --progress-timestamp use Unix epoch timestamps for progress\n"
" --sampling-rate=NUM fraction of transactions to log (e.g., 0.01 for 1%%)\n"
+ " --log-prefix=PREFIX prefix of transaction time log file (default: pgbench_log)\n"
"\nCommon options:\n"
" -d, --debug print debugging output\n"
" -h, --host=HOSTNAME database server host or socket directory\n"
@@ -3643,6 +3645,7 @@ main(int argc, char **argv)
{"sampling-rate", required_argument, NULL, 4},
{"aggregate-interval", required_argument, NULL, 5},
{"progress-timestamp", no_argument, NULL, 6},
+ {"log-prefix", required_argument, NULL, 7},
{NULL, 0, NULL, 0}
};
@@ -3990,6 +3993,9 @@ main(int argc, char **argv)
progress_timestamp = true;
benchmarking_option_set = true;
break;
+ case 7:
+ logfile_prefix = pg_strdup(optarg);
+ break;
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -4087,6 +4093,12 @@ main(int argc, char **argv)
exit(1);
}
+ if (!use_log && logfile_prefix)
+ {
+ fprintf(stderr, "log file prefix (--log-prefix) is allowed only when actually logging transactions\n");
+ exit(1);
+ }
+
if (duration > 0 && agg_interval > duration)
{
fprintf(stderr, "number of seconds for aggregation (%d) must not be higher than test duration (%d)\n", agg_interval, duration);
@@ -4388,11 +4400,13 @@ threadRun(void *arg)
if (use_log)
{
char logpath[64];
+ char *prefix = logfile_prefix ? logfile_prefix : "pgbench_log";
if (thread->tid == 0)
- snprintf(logpath, sizeof(logpath), "pgbench_log.%d", main_pid);
+ snprintf(logpath, sizeof(logpath), "%s.%d", prefix, main_pid);
else
- snprintf(logpath, sizeof(logpath), "pgbench_log.%d.%d", main_pid, thread->tid);
+ snprintf(logpath, sizeof(logpath), "%s.%d.%d", prefix, main_pid, thread->tid);
+
thread->logfile = fopen(logpath, "w");
if (thread->logfile == NULL)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
