Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()

On 1 February 2018 at 18:55, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Feb 1, 2018 at 1:00 AM, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> wrote:
>> IMHO we're missing a point here. When xl_prev is changed to a 2-byte value
>> (as the patch suggests), the risk only comes when an old WAL file is
>> recycled for some future WAL file and the old and the future WAL file's
>> segment numbers share the same low order 16-bits. Now as the patch stands,
>> we take precaution to never reuse a WAL file with conflicting low order
>> bits.
>
> You and Simon seem to be assuming that the only case we need to worry
> about is when the old segment had an xl_prev value at the same offset,
> and you may be right, but if that is guaranteed, I don't understand
> why it's guaranteed. Why couldn't that offset in the old WAL file
> been in the middle of a WAL record and thus subject to containing any
> random garbage?

Yes, it would be about 99% of the time.

But you have it backwards - we are not assuming that case. That is the
only case that has risk - the one where an old WAL record starts at
exactly the place the latest one stops. Otherwise the rest of the WAL
record will certainly fail the CRC check, since it will effectively
have random data in it, as you say.

I didn't even include that aspect in the calculation of the risk.

> If we know that, at the same offset in the previous WAL file, there
> was definitely an xl_prev pointer, then it seems like some trick like
> what you're proposing might work, although I'm not sure of the
> details. But if that could've been something else -- like the middle
> of a WAL record -- then that doesn't really help. Our chance of a
> false match is pretty much just 2^-nbits.

The full 8 bytes of the xl_prev pointer aren't working for a living.
More isn't better, always.

If we really can't persuade you of that, it doesn't sink the patch. We
can have the WAL pointer itself - it wouldn't save space but it would
at least alleviate the spinlock.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services