On Mon, Sep 26, 2016 at 1:01 AM, Heikki Linnakangas <hlinnaka@iki.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.
