Florian Pflug wrote:
> I created the patch to tune the wal_writer for the synchronous_commit=off case - the idea being that the COMMIT
shouldbe 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
thatlog is, and what the format should be. However, if you think it's useful and can come up with a sensible format,
I'dbe 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@2ndQuadrant.com www.2ndQuadrant.us