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 <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.
>
Yeah, this does seem to be something that should be fixed rather than
hidden.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com