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 via email to