Re: Patch to show individual statement latencies in pgbench output

Greg Smith <greg@2ndquadrant.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