| From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
|---|---|
| To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec> |
| Subject: | Re: LogwrtResult contended spinlock |
| Date: | 2024-04-05 18:22:40 |
| Message-ID: | 43269689137ce4f80224b58a4e162fb891a6fc4e.camel@j-davis.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, 2024-04-05 at 13:54 +0200, Alvaro Herrera wrote:
> Couldn't push: I tested with --disable-atomics --disable-spinlocks
> and
> the tests fail because the semaphore for the atomic variables is not
> always initialized. This is weird -- it's like a client process is
> running at a time when StartupXLOG has not initialized the variable
> ...
> so the initialization in the other place was not completely wrong.
It looks like you solved the problem and pushed the first patch. Looks
good to me.
Patch 0002 also looks good to me, after a mostly-trivial rebase (just
remember to initialize logCopyResult).
Minor comments:
* You could consider using a read barrier before reading the Copy ptr,
or using the pg_atomic_read_membarrier_u64() variant. I don't see a
correctness problem, but it might be slightly more clear and I don't
see a lot of downside.
* Also, I assume that the Max() call in
pg_atomic_monotonic_advance_u64() is just for clarity? Surely the
currval cannot be less than _target when it returns. I'd probably just
do Assert(currval >= _target) and then return currval.
Regards,
Jeff Davis
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2024-04-05 18:40:46 | Re: Security lessons from liblzma |
| Previous Message | Alvaro Herrera | 2024-04-05 18:15:16 | Re: [HACKERS] make async slave to wait for lsn to be replayed |