Re: Specifying the log file name of pgbench -l option

Поиск
Список
Период
Сортировка
От Beena Emerson
Тема Re: Specifying the log file name of pgbench -l option
Дата
Msg-id CAOG9ApEddb72nRHCufhQR=k2wyTfXZ1qAZ3STgs9V7COzycfYw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Specifying the log file name of pgbench -l option  (Fabien COELHO <coelho@cri.ensmp.fr>)
Ответы Re: Specifying the log file name of pgbench -l option
Список pgsql-hackers
Hello,

On Wed, Nov 2, 2016 at 4:16 AM, Fabien COELHO <coelho@cri.ensmp.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!

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Craig Ringer
Дата:
Сообщение: Re: Using a latch between a background worker process and a thread
Следующее
От: Ashutosh Sharma
Дата:
Сообщение: Re: Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.