Re: LogwrtResult contended spinlock

On Mon, 2024-02-12 at 17:44 -0800, Jeff Davis wrote:
> It looks like there's some renewed interest in this patch:

After rebasing (attached as 0001), I'm seeing some test failures. It
looks like the local LogwrtResult is not being updated in as many
places, and that's hitting the Assert that I recently added. The fix is
easy (attached as 0002).

Though it looks like we can remove the non-shared LogwrtResult
entirely. Andres expressed some concern here:

https://www.postgresql.org/message-id/20210130020211.rtu5ir3dpjrbiats@alap3.anarazel.de

But then seemed to favor removing it here:

https://www.postgresql.org/message-id/20240213001150.4uqzh7tinuhvoopl@awork3.anarazel.de

I'm inclined to think we can get rid of the non-shared copy.

A few other comments:

* Should GetFlushRecPtr()/GetXLogWriteRecPtr() use a read memory
barrier?
* Why did you add pg_memory_barrier() right before a spinlock
acquisition?
* Is it an invariant that Write >= Flush at all times? Are there
guaranteed to be write barriers in the right place to ensure that?

I would also like it if we could add a new "Copy" pointer indicating
how much WAL data has been copied to the WAL buffers. That would be set
by WaitXLogInsertionsToFinish() so that subsequent calls are cheap.
Attached a patch (0003) for illustration purposes. It adds to the size
of XLogCtlData, but it's fairly large already, so I'm not sure if
that's a problem. If we do add this, there would be an invariant that
Copy >= Write at all times.

Regards,
Jeff Davis