Re: pgbench - doCustom cleanup

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

In response to

Responses

Browse pgsql-hackers by date

  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