> 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.
In addition to this, I'd suggest to add checking -q is only possible
with -i option since without -i, -q is meaningless.
>> 2) Shouldn't a long option for '-q' be provided? Something like
> 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.
If we do so, probably '-q' is not appropeate option name any more,
since the only difference between old logging and new one is, the
former is printed every 10000 lines while the latter is every 5
seconds, which is not really "quiet". What do you think?
SRA OSS, Inc. Japan
In response to
pgsql-hackers by date
|Next:||From: Amit kapila||Date: 2013-01-06 04:56:11|
|Subject: Re: Proposal for Allow postgresql.conf values to be
changed via SQL [review]|
|Previous:||From: Tomas Vondra||Date: 2013-01-06 04:03:38|
|Subject: Re: PATCH: Split stats file per database WAS: autovacuum
stress-testing our system|