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>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org, Jaime Casanova <jcasanov@systemguards.com.ec>
Date: 2024-04-08T16:13:22Z
Lists: pgsql-hackers 
On Mon, 2024-04-08 at 10:24 +0200, Alvaro Herrera wrote:
> My trouble with the "copy" term is that we don't use that term
> anywhere
> in relation to WAL.

I got the term from CopyXLogRecordToWAL().

> This "copy" is in
> reality just the insertion, after it's finished.  The "Result" suffix
> is intended to convey that it's the point where the operation has
> successfully finished.

That's a convincing point.

> Maybe we can add some commentary to make this clearer.

For now, I'd just suggest a comment on GetXLogInsertRecPtr() explaining
what it's returning and how that's different from logInsertResult.

In the future, it would be nice to clarify where variable names like
reservedUpto and CurrBytePos fit in. Also the external documentation
for pg_current_wal_insert_lsn() is a bit ambiguous. 

> Lastly, I just noticed that I forgot credits and discussion link in
> that
> commit message.  I would have attributed authorship to Bharath
> though,
> because I had not realized that this patch had actually been written
> by
> you initially[1].  My apologies.

No worries. Thank you for reviewing and committing it.

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