From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | "Jamison, Kirk" <k(dot)jamison(at)jp(dot)fujitsu(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgbench - doCustom cleanup |
Date: | 2018-11-20 22:48:39 |
Message-ID: | 20181120224839.ji6hyx4s25t5quox@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
pgbench-state-change-7.patch | text/x-diff | 29.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-11-20 23:03:14 | Re: heap_sync seems rather oblivious to partitioned tables (wal_level=minimal) |
Previous Message | Alvaro Herrera | 2018-11-20 22:43:19 | Re: pgbench - doCustom cleanup |