RE: pgbench - doCustom cleanup

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: "Jamison, Kirk" <k(dot)jamison(at)jp(dot)fujitsu(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: pgbench - doCustom cleanup
Date: 2018-10-13 17:34:30
Message-ID: alpine.DEB.2.21.1810111444260.26702@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

Attachment Content-Type Size
pgbench-state-change-3.patch text/x-diff 17.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2018-10-13 18:10:16 Re: Changes in Brazil DST's period
Previous Message Jung, Jinho 2018-10-13 16:12:03 Regarding query minimizer (simplifier)