Horton

Re: LogwrtResult contended spinlock

Jeff Davis <pgsql@j-davis.com>

View thread View on postgr.es
From: Jeff Davis <pgsql@j-davis.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org, Jaime Casanova <jcasanov@systemguards.com.ec>
Date: 2024-02-16T20:54:25Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Remove bogus assertion in pg_atomic_monotonic_advance_u64

  2. Add XLogCtl->logInsertResult

  3. Operate XLogCtl->log{Write,Flush}Result with atomics

  4. Split XLogCtl->LogwrtResult into separate struct members

  5. Introduce atomic read/write functions with full barrier semantics.

  6. Reduce the number of GetFlushRecPtr() calls done by walsenders.

  7. Remove most volatile qualifiers from xlog.c