On 5/19/21 6:32 AM, Fabien COELHO wrote: > > >> Confirmed, thanks for looking. I can reproduce it on my machine with >> -m32. It's somewhat annoying that the buildfarm didn't pick it up >> sooner :-( >> >> On Wed, 19 May 2021 at 08:28, Michael Paquier <mich...@paquier.xyz> >> wrote: >>> >>> On Wed, May 19, 2021 at 09:06:16AM +0200, Fabien COELHO wrote: >>>> I see two simple approaches: >>>> >>>> (1) use another PRNG inside pgbench, eg Knuth's which was used in some >>>> previous submission and is very simple and IMHO better than the rand48 >>>> stuff. >>>> >>>> (2) extend pg_*rand48() to provide an unsigned 64 bits out of the >>>> 48 bits >>>> state. >>> >>> Or, (3) remove this test? I am not quite sure what there is to gain >>> with this extra test considering all the other tests with permute() >>> already present in this script. >> >> Yes, I think removing the test is the best option. It was originally >> added because there was a separate code path for larger permutation >> sizes that needed testing, but that's no longer the case so the test >> really isn't adding anything. > > Hmmm… > > It is the one test which worked in actually detecting an issue, so I > would not say that it is not adding anything, on the contrary, it did > prove its value! The permute function is expected to be deterministic > on different platforms and architectures, and it is not. > > I agree that removing the test will hide the issue effectively:-) but > ISTM more appropriate to solve the underlying issue and keep the test. > > I'd agree with a two phases approach: drop the test in the short term > and deal with the PRNG later. I'm sooooo unhappy with this 48 bit PRNG > that I may be motivated enough to attempt to replace it, or at least > add a better (faster?? larger state?? same/better quality?) alternative. >
Yeah, this does seem to be something that should be fixed rather than hidden. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
