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

On Sat, Mar 24, 2018 at 5:28 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>
> This is a patch about which multiple people have expressed concerns.
> You're trying to jam a heavily redesigned version in at the last
> minute without adequate review. Please don't do that.
>
>
TBH this is not a heavily redesigned version. There were two parts to the
original patch:

1. Replacing 8-byte xl_prev with 2-byte xl_walid in order to reduce the
size of the WAL record header
2. Changing XLogCtlInsert.CurrBytePos to use atomic operations in order to
reduce the spinlock contention.

Most people expressed concerns regarding 1, but there were none regarding
2. Now it's possible that the entire attention got diverted to 1 and nobody
really studied 2 in detail. Or may be 2 is mostly non-contentious, given
the results of micro benchmarks.

So what I've done in v2 is to just deal with part 2 i.e. replace
access to CurrBytePos
with atomic operations, based on the following suggestion by Simon.

On 2/1/18 19:21, Simon Riggs wrote:
> 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.

This gives us the same level of guarantee that xl_prev used to offer, yet
help us use atomic operations. I'll be happy if we can look at that
particular change and see if there are any holes there.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services