On 04.09.2020 20:13, Andres Freund wrote:
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.
Do you mean by general helper function something like this?
void
swap_lsn(XLogRecPtr old_value, XLogRecPtr new_value, bool to_largest)
{
while (true) {
XLogRecPtr currwrite;
currwrite = pg_atomic_read_u64(old_value);
if (to_largest)
if (currwrite > new_value)
break; /* already done by somebody else */
else
if (currwrite < new_value)
break; /* already done by somebody else */
if (pg_atomic_compare_exchange_u64(old_value,
currwrite, new_value))
break; /* already done by somebody else */
}
}
which will be called like
swap_lsn(XLogCtl->LogwrtRqst.Write, EndPos, true);
Greetings,
Andres Freund
This CF entry was inactive for a while. Alvaro, are you going to
continue working on it?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company