On 09/24/2016 12:45 PM, Fabien COELHO wrote:
Although I cannot be absolutely sure that the refactoring does not introduce any new bug, I'm convinced that it will be much easier to find them:-)
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.
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.
- Heikki -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers