Re: LogwrtResult contended spinlock

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndquadrant.com>
Date: 2020-09-04T17:05:45Z
Lists: pgsql-hackers
Hi,

On 2020-09-03 14:34:52 -0400, Alvaro Herrera wrote:
> Looking at patterns like this
> 
> 	if (XLogCtl->LogwrtRqst.Write < EndPos)
> 		XLogCtl->LogwrtRqst.Write = EndPos;
> 
> It seems possible to implement with
> 
>     do {
>     	XLogRecPtr	currwrite;
> 
>         currwrite = pg_atomic_read_u64(LogwrtRqst.Write);
> 	if (currwrite > EndPos)
>             break;  // already done by somebody else
>         if (pg_atomic_compare_exchange_u64(LogwrtRqst.Write,
> 	                                   currwrite, EndPos))
>             break;  // successfully updated
>     } while (true);
> 
> This assumes that LogwrtRqst.Write never goes backwards, so it doesn't
> seem good material for a general routine.
> 
> This *seems* correct to me, though this is muddy territory to me.  Also,
> are there better ways to go about this?

Hm, I was thinking that we'd first go for reading it without a spinlock,
but continuing to write it as we currently do.

But yea, I can't see an issue with what you propose here. I personally
find do {} while () weird and avoid it when not explicitly useful, but
that's extremely minor, obviously.

Greetings,

Andres Freund



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