Fabien COELHO <coe...@cri.ensmp.fr> writes:
>>> Here is a POC which defines an internal interface for a PRNG, and use it
>>> within pgbench, with several possible implementations which default to
>>> rand48.
>> I seriously dislike this patch. pgbench's random support is quite
>> overengineered already IMO, and this proposes to add a whole batch of
>> new code and new APIs to fix a very small bug.
> My intention is rather to discuss postgres' PRNG, in passing. Full success
> on this point:-)
Our immediate problem is to fix a portability failure, which we need to
back-patch into at least one released branch, ergo conservatism is
warranted. I had in mind something more like the attached.
regards, tom lane
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index b5c75ce..d5b543f 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -185,7 +185,7 @@ int64 latency_limit = 0;
char *tablespace = NULL;
char *index_tablespace = NULL;
-/* random seed used when calling srandom() */
+/* random seed used to initialize base_random_sequence */
int64 random_seed = -1;
/*
@@ -287,6 +287,9 @@ typedef struct RandomState
unsigned short xseed[3];
} RandomState;
+/* Various random sequences are initialized from this one. */
+static RandomState base_random_sequence;
+
/*
* Connection state machine states.
*/
@@ -598,6 +601,8 @@ static const BuiltinScript builtin_script[] =
/* Function prototypes */
+static void initRandomState(RandomState *random_state);
+static int64 getrand(RandomState *random_state, int64 min, int64 max);
static void setNullValue(PgBenchValue *pv);
static void setBoolValue(PgBenchValue *pv, bool bval);
static void setIntValue(PgBenchValue *pv, int64 ival);
@@ -833,16 +838,28 @@ strtodouble(const char *str, bool errorOK, double *dv)
/*
* Initialize a random state struct.
+ *
+ * We derive the seed from base_random_sequence, which must be set up already.
*/
static void
initRandomState(RandomState *random_state)
{
- random_state->xseed[0] = random();
- random_state->xseed[1] = random();
- random_state->xseed[2] = random();
+ random_state->xseed[0] = (unsigned short)
+ getrand(&base_random_sequence, 0, 0xFFFF);
+ random_state->xseed[1] = (unsigned short)
+ getrand(&base_random_sequence, 0, 0xFFFF);
+ random_state->xseed[2] = (unsigned short)
+ getrand(&base_random_sequence, 0, 0xFFFF);
}
-/* random number generator: uniform distribution from min to max inclusive */
+/*
+ * Random number generator: uniform distribution from min to max inclusive.
+ *
+ * Although the limits are expressed as int64, you can't generate the full
+ * int64 range in one call, because the difference of the limits mustn't
+ * overflow int64. In practice it's unwise to ask for more than an int32
+ * range, because of the limited precision of pg_erand48().
+ */
static int64
getrand(RandomState *random_state, int64 min, int64 max)
{
@@ -5126,12 +5143,14 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
}
}
-/* call srandom based on some seed. NULL triggers the default behavior. */
+/*
+ * Set up a random seed according to seed parameter (NULL means default),
+ * and initialize base_random_sequence for use in initializing other sequences.
+ */
static bool
set_random_seed(const char *seed)
{
- /* srandom expects an unsigned int */
- unsigned int iseed;
+ uint64 iseed;
if (seed == NULL || strcmp(seed, "time") == 0)
{
@@ -5139,7 +5158,7 @@ set_random_seed(const char *seed)
instr_time now;
INSTR_TIME_SET_CURRENT(now);
- iseed = (unsigned int) INSTR_TIME_GET_MICROSEC(now);
+ iseed = (uint64) INSTR_TIME_GET_MICROSEC(now);
}
else if (strcmp(seed, "rand") == 0)
{
@@ -5155,7 +5174,7 @@ set_random_seed(const char *seed)
/* parse seed unsigned int value */
char garbage;
- if (sscanf(seed, "%u%c", &iseed, &garbage) != 1)
+ if (sscanf(seed, UINT64_FORMAT "%c", &iseed, &garbage) != 1)
{
fprintf(stderr,
"unrecognized random seed option \"%s\": expecting an unsigned integer, \"time\" or \"rand\"\n",
@@ -5165,10 +5184,14 @@ set_random_seed(const char *seed)
}
if (seed != NULL)
- fprintf(stderr, "setting random seed to %u\n", iseed);
- srandom(iseed);
- /* no precision loss: 32 bit unsigned int cast to 64 bit int */
+ fprintf(stderr, "setting random seed to " UINT64_FORMAT "\n", iseed);
random_seed = iseed;
+
+ /* Fill base_random_sequence with low-order bits of seed */
+ base_random_sequence.xseed[0] = iseed & 0xFFFF;
+ base_random_sequence.xseed[1] = (iseed >> 16) & 0xFFFF;
+ base_random_sequence.xseed[2] = (iseed >> 32) & 0xFFFF;
+
return true;
}
@@ -5862,10 +5885,9 @@ main(int argc, char **argv)
/* set default seed for hash functions */
if (lookupVariable(&state[0], "default_seed") == NULL)
{
- uint64 seed = ((uint64) (random() & 0xFFFF) << 48) |
- ((uint64) (random() & 0xFFFF) << 32) |
- ((uint64) (random() & 0xFFFF) << 16) |
- (uint64) (random() & 0xFFFF);
+ uint64 seed =
+ ((uint64) getrand(&base_random_sequence, 0, 0xFFFFFFFF)) |
+ (((uint64) getrand(&base_random_sequence, 0, 0xFFFFFFFF)) << 32);
for (i = 0; i < nclients; i++)
if (!putVariableInt(&state[i], "startup", "default_seed", (int64) seed))
