Re: pgbench - doCustom cleanup
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: "Jamison, Kirk" <k.jamison@jp.fujitsu.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2018-11-20T22:48:39Z
Lists: pgsql-hackers
Attachments
- pgbench-state-change-7.patch (text/x-diff) patch
On 2018-Nov-20, Fabien COELHO wrote: > Hmm. It is somehow, but the aim of the refactoring is to make *ALL* state > transitions to happen in doCustom's switch (st->state) and nowhere else, > which is defeated by creating the separate function. > > Although it improves readability at one level, it does not help figuring out > what happens to states, which is my primary concern: The idea is that > reading doCustom is enough to build and check the automaton, which I had to > do repeatedly while reviewing Marina's patches. I adopted your patch, then while going over it I decided to go with my separation proposal anyway, and re-did it. Looking at the state of the code before this patch, I totally understand that you want to get away from the current state of affairs. However, I don't think my proposal makes matters worse; actually it makes them better. Please give the code a honest look and think whether the separation of machine state advances is really worse with my proposal. I also renamed some things. doCustom() was a pretty bad name, as was CSTATE_START_THROTTLE. > Also the added doCustom comment, which announces this property becomes false > under the refactoring function: > > All state changes are performed within this function called by threadRun. > > So I would suggest not to create this function. I agree the state advances in threadRun were real messy. Thanks for getting rid of the goto. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
pgbench: doExecuteCommand -> executeMetaCommand
- 9938d1163310 12.0 landed
-
Rework the pgbench state machine code for clarity
- 3bac77c48f16 12.0 landed
-
instr_time.h: add INSTR_TIME_SET_CURRENT_LAZY
- 6f7d02aa60b7 12.0 landed
-
pgbench: introduce a RandomState struct
- 409231919443 12.0 cited