pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedRe: LogwrtResult contended spinlock
3+ messages / 2 participants
[nested] [flat]
* Re: LogwrtResult contended spinlock
@ 2020-09-03 18:34 Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-04 17:05 ` Re: LogwrtResult contended spinlock Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Alvaro Herrera @ 2020-09-03 18:34 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndQuadrant.com>
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?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: LogwrtResult contended spinlock
2020-09-03 18:34 Re: LogwrtResult contended spinlock Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2020-09-04 17:05 ` Andres Freund <andres@anarazel.de>
2020-09-04 17:13 ` Re: LogwrtResult contended spinlock Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2020-09-04 17:05 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndquadrant.com>
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
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: LogwrtResult contended spinlock
2020-09-03 18:34 Re: LogwrtResult contended spinlock Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-04 17:05 ` Re: LogwrtResult contended spinlock Andres Freund <andres@anarazel.de>
@ 2020-09-04 17:13 ` Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 3+ messages in thread
From: Andres Freund @ 2020-09-04 17:13 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndquadrant.com>
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
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2020-09-04 17:13 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 18:34 Re: LogwrtResult contended spinlock Alvaro Herrera <alvherre@2ndquadrant.com>
2020-09-04 17:05 ` Andres Freund <andres@anarazel.de>
2020-09-04 17:13 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox