From: | Greg Smith <greg(at)2ndquadrant(dot)com> |
---|---|
To: | Florian Pflug <fgp(at)phlo(dot)org> |
Cc: | PostgreSQL-development hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Patch to show individual statement latencies in pgbench output |
Date: | 2010-08-03 19:16:25 |
Message-ID: | 4C586B09.3080203@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Florian Pflug wrote:
> I created the patch to tune the wal_writer for the synchronous_commit=off case - the idea being that the COMMIT should be virtually instantaneous if the wal_writer writes old wal buffers out fast enough.
>
As I was saying, being able to see the COMMIT times for purposes such as
that is something I consider valuable about using this instrumentation
that's not really targeted by pg_stat_statements, the other way built-in
way people might try to get it.
> I haven't yet used pgbench's log output feature, so I can't judge how useful the additional of per-statement data to that log is, and what the format should be. However, if you think it's useful and can come up with a sensible format, I'd be happy to add that feature to the patch.
>
Let's worry about that in the future. Maybe it's a good add-on, but
it's more than I have time to get into during this CF personally.
> That was a leftover of the trimming and comment skipping logic, which my patch moves to process_command.
>
I think there's still a trimming error here--line 195 of the new patch
is now removing the declaration of "i" just before it sets it to zero?
On the coding standard side, I noticed all your for loops are missing a
space between the for and the (; that should get fixed.
Finally, now that the rest of the patch is looking in good shape and is
something I think is worth considering to commit, it's time to work on
the documentation SGML.
Also: when generating multiple versions of a patch like this, standard
practice is to add something like "-vX" to the naming, so those of us
trying to review can keep them straight. So next one would be
"pgbench_statementlatency_v3.patch" or something like that. It's a good
habit to get into from first version of a patch you submit. Presuming
that's going to be the only version is optimistic for all but the
smallest of patches, and sometimes not even them...
--
Greg Smith 2ndQuadrant US Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.us
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2010-08-03 19:32:00 | Re: tracking inherited columns (was: patch for check constraints using multiple inheritance) |
Previous Message | Robert Haas | 2010-08-03 19:16:00 | Re: (9.1) btree_gist support for searching on "not equals" |