Re: pgbench progress report improvements

On Sat, Sep 21, 2013 at 4:55 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>>> - Use progress option both under init & bench.
>>>
>>> Activate progress report by default, every 5 seconds.
>>> When initializing, --quiet reverts to the old every 100,000 insertions
>>> behavior...
>>
>> Patch (1): Change the default from --progress=0 to --progress=5
>>
>> This has a disadvantage of causing two extra gettimeofday() calls per
>> transaction. That's not cheap on all platforms; a user comparing pgbench
>> results across versions will need to make a point of disabling this again to
>> make his results comparable. Given that threat and uncertainty about which
>> default would be more popular, I think we should drop this part.
>
> The gettimeofday call time is very small compared to network and database
> (disk!) accesses involved in a pgbench run. On my small laptop, gettimeofday
> takes about 0.00000002 seconds (0.02 µs -- micro seconds) per call, which is
> typically under 1/1000 of the fastest local-socket read-only one-table
> cache-hit prepared trivial transaction.
>
> This is a little different when initializing, but I put a guard on
> gettimeofday in that case.
>
> If this is a blocker, I can put 0, but I really do not think it is necessary
> because of the performance impact.

I really don't think it's a good idea to change the default behavior.
We've had many discussions about how the overhead of gettimeofday()
can sometimes be surprisingly large; I don't think we should assume
it's guaranteed to be small in this case. Also, changing established
defaults will annoy users who like to present defaults; I don't see
any reason to assume that your preferences will be universal. In
doubtful cases we should favor leaving the behavior the way it's been
in previous releases, because backward-compatibility is very
desirable.

>> Patch (2): Make --initialize mode respect --progress.
>>
>> The definition you've chosen for --quiet makes that option contrary to its
>> own
>> name: message-per-100k-tuples is typically more voluminous than your
>> proposed
>> new default of message-per-5s. In fact, since --quiet currently switches
>> from
>> message-per-100k-tuples to message-per-5s, your patch gives it the exact
>> opposite of its present effect.
>>
>> During the 9.3 development cycle, we _twice_ made changes pertaining to
>> --initialize message frequency:
>>
>> http://www.postgresql.org/message-id/flat/20120620(dot)170427(dot)347012755716399568(dot)t-ishii(at)sraoss(dot)co(dot)jp
>>
>> http://www.postgresql.org/message-id/flat/1346472039(dot)18010(dot)10(dot)camel(at)vanquo(dot)pezone(dot)net
>>
>> That gives me pause about working through yet another change; we keep
>> burning
>> time on this minor issue.
>
> I totally agree that this "quiet" is not convincing!
>
> My actual opinion is that quiet should just mean quiet:-), i.e. no progress
> report.
>
> I tried to preserve the row-counting behavior because I thought that someone
> would object otherwise, but I would be really in favor of dropping the
> row-counting report behavior altogether and only keep the time triggered
> report.

-1 for changing this again. Frankly, I might have come down in a
different place if I had understood exactly how this was going to end
up being commited; it ended up being quite different from what I was
expecting. But I really think that relitigating this just so we can
break backward compatibility again one release later is not a good use
of anyone's time, or a good idea in general.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company