Re: Patch to show individual statement latencies in pgbench output

Greg Smith <greg(at)2ndquadrant(dot)com> writes:
> Florian Pflug wrote:
>> Attached is an updated version (v4).

> I've attached a v5.

BTW, I discovered a rather nasty shortcoming of this patch on platforms
without ENABLE_THREAD_SAFETY. It doesn't work, and what's worse, it
*looks* like it's working, because it gives you plausible-looking
numbers. But actually the numbers are only averages across the first
worker thread. The other threads are in sub-processes where they can't
affect the contents of the parent's arrays.

Since platforms without ENABLE_THREAD_SAFETY are only a small minority
these days, this is probably not sufficient reason to reject the patch.
What I plan to do instead is reject the combination of -r with -j larger
than 1 on such platforms:

if (is_latencies)
{
/*
* is_latencies only works with multiple threads in thread-based
* implementations, not fork-based ones, because it supposes that the
* parent can see changes made to the command data structures by child
* threads. It seems useful enough to accept despite this limitation,
* but perhaps we should FIXME someday.
*/
#ifndef ENABLE_THREAD_SAFETY
if (nthreads > 1)
{
fprintf(stderr, "-r does not work with -j larger than 1 on this platform.\n");
exit(1);
}
#endif

It could be fixed with enough effort, by having the child threads pass
back their numbers through the child-to-parent pipes. I'm not
interested in doing that myself though.

If anyone thinks this problem makes it uncommittable, speak up now.

regards, tom lane