Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

On 09-06-2018 16:31, Fabien COELHO wrote:
> Hello Marina,

Hello!

>> v9-0002-Pgbench-errors-use-the-Variables-structure-for-cl.patch
>> - a patch for the Variables structure (this is used to reset client 
>> variables during the repeating of transactions after 
>> serialization/deadlock failures).
> 
> About this second patch:
> 
> This extracts the variable holding structure, so that it is somehow
> easier to reset them to their initial state on transaction failures,
> the management of which is the ultimate aim of this patch series.
> 
> It is also cleaner this way.
> 
> Patch applies cleanly on top of the previous one (there is no real
> interactions with it). It compiles cleanly. Global & pgbench "make
> check" are both ok.

:-)

> The structure typedef does not need a name. "typedef struct { } V...".

Ok!

> I tend to disagree with naming things after their type, eg "array".
> I'd suggest "vars" instead. "nvariables" could be "nvars" for
> consistency with that and "vars_sorted", and because
> "foo.variables->nvariables" starts looking heavy.
> 
> I'd suggest but "Variables" type declaration just after "Variable"
> type declaration in the file.

Thank you, I agree and I'll fix all this.

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company