From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: pgsql-hackers@postgresql.org, Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
Cc: Teodor Sigaev <teodor@sigaev.ru>
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date: Thu, 12 Jul 2018 11:57:01 +0300
Message-ID: <cb2cde10e4e7a10a38b48e9cae8fbd28@postgrespro.ru> (raw)
In-Reply-To: <20180711180417.3ytmmwmonsr5lra7@alvherre.pgsql>
References: <20180711180417.3ytmmwmonsr5lra7@alvherre.pgsql>
On 11-07-2018 21:04, Alvaro Herrera wrote:
> Just a quick skim while refreshing what were those error reporting API
> changes about ...
Thank you!
> On 2018-May-21, Marina Polyakova wrote:
>
>> v9-0001-Pgbench-errors-use-the-RandomState-structure-for-.patch
>> - a patch for the RandomState structure (this is used to reset a
>> client's
>> random seed during the repeating of transactions after
>> serialization/deadlock failures).
>
> LGTM, though I'd rename the random_state struct members so that it
> wouldn't look as confusing. Maybe that's just me.
IIUC, do you like "xseed" instead of "data"?
typedef struct RandomState
{
- unsigned short data[3];
+ unsigned short xseed[3];
} RandomState;
Or do you want to rename "random_state" in the structures RetryState /
CState / TState? Thanks to Fabien Coelho' comments in [1], TState can
contain several RandomStates for different purposes, something like
this:
/*
* Thread state
*/
typedef struct
{
...
/*
* Separate randomness for each thread. Each thread option uses its own
* random state to make all of them independent of each other and
therefore
* deterministic at the thread level.
*/
RandomState choose_script_rs; /* random state for selecting a script */
RandomState throttling_rs; /* random state for transaction throttling
*/
RandomState sampling_rs; /* random state for log sampling */
...
} TState;
>> 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).
>
> Please don't allocate Variable structs one by one. First time allocate
> some decent number (say 8) and then enlarge by duplicating size. That
> way you save realloc overhead. We use this technique everywhere else,
> no reason do different here. Other than that, LGTM.
Ok!
[1]
https://www.postgresql.org/message-id/alpine.DEB.2.21.1806090810090.5307%40lancre> While reading your patch, it occurs to me that a run is not
> deterministic
> at the thread level under throttling and sampling, because the random
> state is sollicited differently depending on when transaction ends.
> This
> suggest that maybe each thread random_state use should have its own
> random
> state.
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: m.polyakova@postgrespro.ru, alvherre@2ndquadrant.com, coelho@cri.ensmp.fr, i.kurbangaliev@postgrespro.ru, teodor@sigaev.ru
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
In-Reply-To: <cb2cde10e4e7a10a38b48e9cae8fbd28@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox