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
