Re: pgbench - minor fix for meta command only scripts

On Mon, Sep 26, 2016 at 1:01 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 09/24/2016 12:45 PM, Fabien COELHO wrote:
>
>>
>> Attached are some small changes to your version:
>>
>> I have added the sleep_until fix.
>>
>> I have fixed a bug introduced in the patch by changing && by || in the
>> (min_sec > 0 && maxsock != -1) condition which was inducing errors with
>> multi-threads & clients...
>>
>> I have factored out several error messages in "commandFailed", in place of
>> the "metaCommandFailed", and added the script number as well in the error
>> messages. All messages are now specific to the failed command.
>>
>> I have added two states to the machine:
>>
>> - CSTATE_CHOOSE_SCRIPT which simplifies threadRun, there is now one call
>> to chooseScript instead of two before.
>>
>> - CSTATE_END_COMMAND which manages is_latencies and proceeding to the
>> next command, thus merging the three instances of updating the stats
>> that were in the first version.
>>
>> The later state means that processing query results is included in the per
>> statement latency, which is an improvement because before I was getting
>> some transaction latency significantly larger that the apparent sum of the
>> per-statement latencies, which did not make much sense...
>>
>
> Ok. I agree that makes more sense.
>
> I have added & updated a few comments.
>>
>
> Thanks! Committed.
>
> There are some places where the break could be a pass through
>> instead, not sure how desirable it is, I'm fine with break.
>>
>
> I left them as "break". Pass-throughs are error-prone, and make it more
> difficult to read, IMHO. The compiler will optimize it into a pass-through
> anyway, if possible and worthwhile, so there should be no performance
> difference.

Since this commit (12788ae49e1933f463bc5), if I use the --rate to throttle
the transaction rate, it does get throttled to about the indicated speed,
but the pg_bench consumes the entire CPU.

At the block of code starting
if (min_usec > 0 && maxsock != -1)

If maxsock == -1, then there is no sleep happening.

Cheers,

Jeff