Re: LogwrtResult contended spinlock

Hi,

On 2020-09-04 10:05:45 -0700, Andres Freund wrote:
> 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.

Re general routine: On second thought, it might actually be worth having
it. Even just for LSNs - there's plenty places where it's useful to
ensure a variable is at least a certain size.  I think I would be in
favor of a general helper function.

Greetings,

Andres Freund