> 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 <michael@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.
--
Fabien.
