Re: PATCH: pgbench - aggregation of info written into log

Dne 30.08.2012 18:02, Robert Haas napsal:
> On Fri, Aug 24, 2012 at 5:25 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> This patch is a bit less polished (and more complex) than the other
>> pgbench patch I've sent a while back, and I'm not sure how to handle
>> the
>> Windows branch. That needs to be fixed during the commit fest.
>
> What's the problem with the Windows branch?
>
> Could you un-cuddle your brances to follow the PostgreSQL style, omit
> braces around single-statement blocks, and remove the spurious
> whitespace changes?

Done, or at least I don't see other formatting issues. Let me know if I
missed something.

> Instead of having both use_log_agg and naggseconds, I think you can
> get by with just having a single variable that is zero if aggregation
> is not in use and is otherwise the aggregation period. On a related
> note, you can't specify -A without an associated value, so there is
> no
> point in documenting a "default". As with your other patch, I
> suggest
> using a long option name like --latency-aggregate-interval and then
> making the name of the variable in the code match the option name,
> with s/-/_/g, for clarity.

Fixed. I've kept use_log_agg only and I've removed the default. Also
I've added one more check (that the total duration is a multiple of
the aggregation interval).

And just as with the sampling patch, I believe the "-l" should not be
enabled by default, but required. But if more people ask to enable it
whenever the aggregation or sampling is used, I'm fine with it.

Tomas