RE: pgbench - doCustom cleanup

Hello Kirk,

> I have decided to take a look into this patch.

Thanks for the feedback.

> I noticed in your refactored code that CSTATE_CHOOSE_SCRIPT may now also 
> change to CSTATE_FINISHED when timer is exceeded. (Before, it only 
> changes to either CSTATE_START_THROTTLE or CSTATE_START_TX) But the 
> change has not been indicated YET in the typedef enum part for 
> CSTATE_CHOOSE_SCRIPT.

Indeed, the comment is inaccurate and need updating.

> As for the behavioral change, it is assumed that there will be 1 script 
> per transaction run. After code reading, I interpreted the "modified" 
> state changes below:
>
> 1. CSTATE_CHOOSE_SCRIPT may now switch CSTATE_FINISHED
> Previously, it may only switch to CSTATE_THROTTLE or CSTATE_START_TX.

Yes, I have added a shortcut there.

> 2. CSTATE_START_THROTTLE (Similar to #1) may now switch to CSTATE_FINISHED.
> Previously, it may only switch to CSTATE_THROTTLE or CSTATE_START_TX.

This was somehow already the case before the patch in some cases, but I 
moved the decision after the while throttle so that it catches more cases.


> 3. CSTATE_START_TX: Transactions, once started, cannot be interrupted 
> while running, and now proceeds to first command. Interrupt may only be 
> allowed either after tx error or when tx run ends.

Yes, I removed all interruptions.

> 4. CSTATE_SKIP_COMMAND
> No particular change in code logic, but clarified in the typedef that this state can move forward several commands until it meets next commands or finishes script.

Yes.

> 5. CSTATE_END_TX: either starts over with new script (CSTATE_CHOOSE_SCRIPT) or finishes (CSTATE_FINISHED).
> Previously, it either defaults to CSTATE_START_THROTTLE when choosing a new script, or finishes.

Nope, it was already jumping between CHOOSE & FINISHED, however I 
simplified the logic there to avoid doCustom to stay in one client
by always returning.

Attached is a v3, where I have updated inaccurate comments.

-- 
Fabien.