From: | Beena Emerson <memissemerson(at)gmail(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Specifying the log file name of pgbench -l option |
Date: | 2016-11-02 06:57:39 |
Message-ID: | CAOG9ApEddb72nRHCufhQR=k2wyTfXZ1qAZ3STgs9V7COzycfYw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello,
On Wed, Nov 2, 2016 at 4:16 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>
> Hello Masahiko,
>
> 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
>>>
>>
>> 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.
>>
>
> Patch applies, compiles and works for me.
>
It works for me as well.
>
> This new option is for benchmarking, so "benchmarking_option_set" should
> be set to true.
>
> To simplify the code, I would suggest to initialize explicitely
> "logfile_prefix" to the default value which is then overriden when the
> option is set, which allows to get rid of the "prefix" variable later.
>
> There is no reason to change the documentation by breaking a line at
> another place if the text is not changed (where ... with 1).
>
> I would suggest to simplify a little bit the documentation:
> "prefix of log file ..." ->
> "default log file prefix that can be changed with <option>...</>"
>
> Otherwise the explanations seem clear enough to me. If a native English
> speaker could check them though, it would be nice.
For the explanation of the option --log-prefix, I feel it would be better
to say "Custom prefix for transaction log file. Default is pgbench_log"
- <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
It could just say "the default prefix pgbench_log can be changed with
option --log-prefix, and " we need not use where again.
Also the error message is made similar to that of aggregate-interval but I
think the one in sampling-rate is better:
$ pgbench --log-prefix=chk -t 20
log file prefix (--log-prefix) is allowed only when actually logging
transactions
pgbench --sampling-rate=1 -t 20
log sampling (--sampling-rate) is allowed only when logging transactions
(-l)
Thanks,
Beena Emerson
Have a Great Day!
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Sharma | 2016-11-02 07:18:34 | Re: Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated. |
Previous Message | Craig Ringer | 2016-11-02 06:54:47 | Re: Using a latch between a background worker process and a thread |