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.
