Re: LogwrtResult contended spinlock - Mailing list pgsql-hackers

From Andres Freund
Subject Re: LogwrtResult contended spinlock
Date
Msg-id 20200904171355.bldopurt65m2aqn4@alap3.anarazel.de
Whole thread Raw
In response to Re: LogwrtResult contended spinlock  (Andres Freund <andres@anarazel.de>)
Responses Re: LogwrtResult contended spinlock
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: LogwrtResult contended spinlock
Next
From: Bruce Momjian
Date:
Subject: Re: [PATCH] Comments related to " buffer descriptors“ cache line size"