Re: PATCH: pgbench - aggregation of info written into log
Tomas Vondra <tv@fuzzy.cz>
From: Tomas Vondra <tv@fuzzy.cz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: <pgsql-hackers@postgresql.org>
Date: 2012-09-02T22:46:00Z
Lists: pgsql-hackers
Attachments
- pgbench-aggregated-v2.diff (text/plain) patch v2
Dne 30.08.2012 18:02, Robert Haas napsal: > On Fri, Aug 24, 2012 at 5:25 PM, Tomas Vondra <tv@fuzzy.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