Re: Patch to show individual statement latencies in pgbench output

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

In response to

Responses

Browse pgsql-hackers by date

  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"