Skip to content

Commit e6c3ba7

Browse files
committed
Fix portability problem in pgbench.
The pgbench regression test supposed that srandom() with a specific value would result in deterministic output from random(), as required by POSIX. It emerges however that OpenBSD is too smart to be constrained by mere standards, so their random() emits nondeterministic output anyway. While a workaround does exist, what seems like a better fix is to stop relying on the platform's srandom()/random() altogether, so that what you get from --random-seed=N is not merely deterministic but platform independent. Hence, use a separate pg_jrand48() random sequence in place of random(). Also adjust the regression test case that's supposed to detect nondeterminism so that it's more likely to detect it; the original choice of random_zipfian parameter tended to produce the same output all the time even if the underlying behavior wasn't deterministic. In passing, improve pgbench's docs about random_zipfian(). Back-patch to v11 where this code was introduced. Fabien Coelho and Tom Lane Discussion: https://postgr.es/m/4615.1547792324@sss.pgh.pa.us
1 parent 19184fc commit e6c3ba7

File tree

3 files changed

+58
-29
lines changed

3 files changed

+58
-29
lines changed

doc/src/sgml/ref/pgbench.sgml

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,15 +1604,24 @@ f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) /
16041604
in (1, 1000), a rejection method is used, based on
16051605
"Non-Uniform Random Variate Generation", Luc Devroye, p. 550-551,
16061606
Springer 1986. The distribution is not defined when the parameter's
1607-
value is 1.0. The drawing performance is poor for parameter values
1607+
value is 1.0. The function's performance is poor for parameter values
16081608
close and above 1.0 and on a small range.
16091609
</para>
16101610
<para>
1611-
<replaceable>parameter</replaceable>
1612-
defines how skewed the distribution is. The larger the <replaceable>parameter</replaceable>, the more
1613-
frequently values to the beginning of the interval are drawn.
1611+
<replaceable>parameter</replaceable> defines how skewed the distribution
1612+
is. The larger the <replaceable>parameter</replaceable>, the more
1613+
frequently values closer to the beginning of the interval are drawn.
16141614
The closer to 0 <replaceable>parameter</replaceable> is,
1615-
the flatter (more uniform) the access distribution.
1615+
the flatter (more uniform) the output distribution.
1616+
The distribution is such that, assuming the range starts from 1,
1617+
the ratio of the probability of drawing <replaceable>k</replaceable>
1618+
versus drawing <replaceable>k+1</replaceable> is
1619+
<literal>((<replaceable>k</replaceable>+1)/<replaceable>k</replaceable>)**<replaceable>parameter</replaceable></literal>.
1620+
For example, <literal>random_zipfian(1, ..., 2.5)</literal> produces
1621+
the value <literal>1</literal> about <literal>(2/1)**2.5 =
1622+
5.66</literal> times more frequently than <literal>2</literal>, which
1623+
itself is produced <literal>(3/2)*2.5 = 2.76</literal> times more
1624+
frequently than <literal>3</literal>, and so on.
16161625
</para>
16171626
</listitem>
16181627
</itemizedlist>

src/bin/pgbench/pgbench.c

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ int64 latency_limit = 0;
185185
char *tablespace = NULL;
186186
char *index_tablespace = NULL;
187187

188-
/* random seed used when calling srandom() */
188+
/* random seed used to initialize base_random_sequence */
189189
int64 random_seed = -1;
190190

191191
/*
@@ -287,6 +287,9 @@ typedef struct RandomState
287287
unsigned short xseed[3];
288288
} RandomState;
289289

290+
/* Various random sequences are initialized from this one. */
291+
static RandomState base_random_sequence;
292+
290293
/*
291294
* Connection state machine states.
292295
*/
@@ -833,16 +836,28 @@ strtodouble(const char *str, bool errorOK, double *dv)
833836

834837
/*
835838
* Initialize a random state struct.
839+
*
840+
* We derive the seed from base_random_sequence, which must be set up already.
836841
*/
837842
static void
838843
initRandomState(RandomState *random_state)
839844
{
840-
random_state->xseed[0] = random();
841-
random_state->xseed[1] = random();
842-
random_state->xseed[2] = random();
845+
random_state->xseed[0] = (unsigned short)
846+
(pg_jrand48(base_random_sequence.xseed) & 0xFFFF);
847+
random_state->xseed[1] = (unsigned short)
848+
(pg_jrand48(base_random_sequence.xseed) & 0xFFFF);
849+
random_state->xseed[2] = (unsigned short)
850+
(pg_jrand48(base_random_sequence.xseed) & 0xFFFF);
843851
}
844852

845-
/* random number generator: uniform distribution from min to max inclusive */
853+
/*
854+
* Random number generator: uniform distribution from min to max inclusive.
855+
*
856+
* Although the limits are expressed as int64, you can't generate the full
857+
* int64 range in one call, because the difference of the limits mustn't
858+
* overflow int64. In practice it's unwise to ask for more than an int32
859+
* range, because of the limited precision of pg_erand48().
860+
*/
846861
static int64
847862
getrand(RandomState *random_state, int64 min, int64 max)
848863
{
@@ -5126,20 +5141,22 @@ printResults(TState *threads, StatsData *total, instr_time total_time,
51265141
}
51275142
}
51285143

5129-
/* call srandom based on some seed. NULL triggers the default behavior. */
5144+
/*
5145+
* Set up a random seed according to seed parameter (NULL means default),
5146+
* and initialize base_random_sequence for use in initializing other sequences.
5147+
*/
51305148
static bool
51315149
set_random_seed(const char *seed)
51325150
{
5133-
/* srandom expects an unsigned int */
5134-
unsigned int iseed;
5151+
uint64 iseed;
51355152

51365153
if (seed == NULL || strcmp(seed, "time") == 0)
51375154
{
51385155
/* rely on current time */
51395156
instr_time now;
51405157

51415158
INSTR_TIME_SET_CURRENT(now);
5142-
iseed = (unsigned int) INSTR_TIME_GET_MICROSEC(now);
5159+
iseed = (uint64) INSTR_TIME_GET_MICROSEC(now);
51435160
}
51445161
else if (strcmp(seed, "rand") == 0)
51455162
{
@@ -5155,7 +5172,7 @@ set_random_seed(const char *seed)
51555172
/* parse seed unsigned int value */
51565173
char garbage;
51575174

5158-
if (sscanf(seed, "%u%c", &iseed, &garbage) != 1)
5175+
if (sscanf(seed, UINT64_FORMAT "%c", &iseed, &garbage) != 1)
51595176
{
51605177
fprintf(stderr,
51615178
"unrecognized random seed option \"%s\": expecting an unsigned integer, \"time\" or \"rand\"\n",
@@ -5165,10 +5182,14 @@ set_random_seed(const char *seed)
51655182
}
51665183

51675184
if (seed != NULL)
5168-
fprintf(stderr, "setting random seed to %u\n", iseed);
5169-
srandom(iseed);
5170-
/* no precision loss: 32 bit unsigned int cast to 64 bit int */
5185+
fprintf(stderr, "setting random seed to " UINT64_FORMAT "\n", iseed);
51715186
random_seed = iseed;
5187+
5188+
/* Fill base_random_sequence with low-order bits of seed */
5189+
base_random_sequence.xseed[0] = iseed & 0xFFFF;
5190+
base_random_sequence.xseed[1] = (iseed >> 16) & 0xFFFF;
5191+
base_random_sequence.xseed[2] = (iseed >> 32) & 0xFFFF;
5192+
51725193
return true;
51735194
}
51745195

@@ -5862,10 +5883,9 @@ main(int argc, char **argv)
58625883
/* set default seed for hash functions */
58635884
if (lookupVariable(&state[0], "default_seed") == NULL)
58645885
{
5865-
uint64 seed = ((uint64) (random() & 0xFFFF) << 48) |
5866-
((uint64) (random() & 0xFFFF) << 32) |
5867-
((uint64) (random() & 0xFFFF) << 16) |
5868-
(uint64) (random() & 0xFFFF);
5886+
uint64 seed =
5887+
((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) |
5888+
(((uint64) pg_jrand48(base_random_sequence.xseed) & 0xFFFFFFFF) << 32);
58695889

58705890
for (i = 0; i < nclients; i++)
58715891
if (!putVariableInt(&state[i], "startup", "default_seed", (int64) seed))

src/bin/pgbench/t/001_pgbench_with_server.pl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,11 +259,11 @@ sub pgbench
259259
[
260260
qr{setting random seed to 5432\b},
261261

262-
# After explicit seeding, the four * random checks (1-3,20) should be
263-
# deterministic, but not necessarily portable.
264-
qr{command=1.: int 1\d\b}, # uniform random: 12 on linux
265-
qr{command=2.: int 1\d\d\b}, # exponential random: 106 on linux
266-
qr{command=3.: int 1\d\d\d\b}, # gaussian random: 1462 on linux
262+
# After explicit seeding, the four random checks (1-3,20) are
263+
# deterministic
264+
qr{command=1.: int 13\b}, # uniform random
265+
qr{command=2.: int 116\b}, # exponential random
266+
qr{command=3.: int 1498\b}, # gaussian random
267267
qr{command=4.: int 4\b},
268268
qr{command=5.: int 5\b},
269269
qr{command=6.: int 6\b},
@@ -276,7 +276,7 @@ sub pgbench
276276
qr{command=15.: double 15\b},
277277
qr{command=16.: double 16\b},
278278
qr{command=17.: double 17\b},
279-
qr{command=20.: int \d\b}, # zipfian random: 1 on linux
279+
qr{command=20.: int 1\b}, # zipfian random
280280
qr{command=21.: double -27\b},
281281
qr{command=22.: double 1024\b},
282282
qr{command=23.: double 1\b},
@@ -471,7 +471,7 @@ sub pgbench
471471
\set ur random(1000, 1999)
472472
\set er random_exponential(2000, 2999, 2.0)
473473
\set gr random_gaussian(3000, 3999, 3.0)
474-
\set zr random_zipfian(4000, 4999, 2.5)
474+
\set zr random_zipfian(4000, 4999, 1.5)
475475
INSERT INTO seeded_random(seed, rand, val) VALUES
476476
(:random_seed, 'uniform', :ur),
477477
(:random_seed, 'exponential', :er),

0 commit comments

Comments
 (0)