Thread: pgsql: Set random seed for pgbench.
Set random seed for pgbench. Setting random could increase reproducibility of test in some cases. Patch suggests three providers for seed: time (default), strong random generator (if available) and unsigned constant. Seed could be set from command line or enviroment variable. Author: Fabien Coelho Reviewed by: Chapman Flack Discussion: https://www.postgresql.org/message-id/flat/20160407082711.q7iq3ykffqxcszkv@alap3.anarazel.de Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/64f85894ad2730fb1449a8e81dd8026604e9a546 Modified Files -------------- doc/src/sgml/ref/pgbench.sgml | 42 +++++++++++++++++ src/bin/pgbench/pgbench.c | 69 +++++++++++++++++++++++++--- src/bin/pgbench/t/001_pgbench_with_server.pl | 69 +++++++++++++++++++++++++--- src/bin/pgbench/t/002_pgbench_no_server.pl | 2 + 4 files changed, 170 insertions(+), 12 deletions(-)
Teodor Sigaev <teodor@sigaev.ru> writes:
> Set random seed for pgbench.
So this patch has ignored the possibility of not having pg_strong_random.
That's moving the portability goalposts in a way that I do not find
acceptable for such a marginal feature. What behavior do you think we
should have for platforms without that --- accept the seed=rand option
but then error out, or not recognize the option at all?
BTW, I also note that the patch is constructing error messages from
sentence fragments, which is verboten per translatability policy.
While that's perhaps not too important as long as we don't have message
translations for pgbench, it still seems like something to fix now not
later.
regards, tom lane
Hello Tom, >> Set random seed for pgbench. > > So this patch has ignored the possibility of not having pg_strong_random. I assumed that pg_strong_random is always available, but may fail with an error if a strong random source is not available, in which case the error message should be enough for the user to figure out that they cannot use the 'rand' value and must rely on 'time' or an integer. The error message can be improved in this case. > That's moving the portability goalposts in a way that I do not find > acceptable for such a marginal feature. What behavior do you think we > should have for platforms without that --- accept the seed=rand option > but then error out, or not recognize the option at all? The first option is currently implemented and documented, and seems fine. > BTW, I also note that the patch is constructing error messages from > sentence fragments, which is verboten per translatability policy. > While that's perhaps not too important as long as we don't have message > translations for pgbench, it still seems like something to fix now not > later. The elements of the structure could be translatable independently: one part says there is an error, the other provides the context in which this error was triggered. This could be made a little clearer by using parentheses for instance. Otherwise to comply with translability policy the function can be made to return a bool and then the caller show the context. In the attached, I improved the error message on 'rand' error and printed the error context outside of the function. Thanks for the review. -- Fabien.
Attachment
Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> So this patch has ignored the possibility of not having pg_strong_random.
> I assumed that pg_strong_random is always available,
... which is wrong. Every other call of it is wrapped in
#ifdef HAVE_STRONG_RANDOM, and so must this one be.
We can use the same error message though, I suppose.
Adjusted and pushed.
regards, tom lane
>> I assumed that pg_strong_random is always available,
>
> ... which is wrong. Every other call of it is wrapped in
> #ifdef HAVE_STRONG_RANDOM, and so must this one be.
Indeed, I clearly misunderstood its usage pattern. I looked at its source
("src/port/pg_strong_random.c") where the function is always defined and
is documented as returning false if it does not find a strong random
source, so I though that checking for this was enough. But indeed its
compilation fails if no source is provided.
Thanks for the fix.
--
Fabien.
On Sat, Mar 31, 2018 at 07:43:38PM +0200, Fabien COELHO wrote:
> Indeed, I clearly misunderstood its usage pattern. I looked at its source
> ("src/port/pg_strong_random.c") where the function is always defined and is
> documented as returning false if it does not find a strong random source, so
> I though that checking for this was enough. But indeed its
> compilation fails if no source is provided.
I have not check in details this thread so I may be saying something
stupid... But if you are looking for a frontend implementation for
strong randoms, please extract pg_frontend_random in fe-auth-scram.c and
move it to its own file for example in src/common as a frontend-only
file. When working on SCRAM, I recall mentioning that but Heikki has
kept the code in its current shape for simplicity.
As far as I can see, there is no reason to issue an error in
set_random_seed() either in pgbench code.
--
Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> I have not check in details this thread so I may be saying something
> stupid... But if you are looking for a frontend implementation for
> strong randoms, please extract pg_frontend_random in fe-auth-scram.c and
> move it to its own file for example in src/common as a frontend-only
> file. When working on SCRAM, I recall mentioning that but Heikki has
> kept the code in its current shape for simplicity.
I don't really see the point of doing more work than we've done there.
No modern platform should be without pg_strong_random, and if you've
actually had to use --disable-strong-random, it will not be astonishing
that you lose some functionality.
Also, while I personally do not see a use-case why somebody would need
cryptographically strong random seeds in pgbench, if somebody did need
that then they wouldn't thank us for silently falling back to a
not-strong generation method if the platform is misconfigured somehow.
regards, tom lane