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

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