From: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Fujii Masao <fujii(at)postgresql(dot)org>, pgsql-committers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pgsql: Get rid of the dedicated latch for signaling the startup process |
Date: | 2020-12-17 09:17:54 |
Message-ID: | 5dd3bbe0-3845-52d4-ce69-083102ba0094@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
On 2020/11/05 23:32, Fujii Masao wrote:
>
>
> On 2020/11/05 6:02, Fujii Masao wrote:
>>
>>
>> On 2020/11/05 5:36, Heikki Linnakangas wrote:
>>> On 04/11/2020 15:17, Heikki Linnakangas wrote:
>>>> On 04/11/2020 14:03, Fujii Masao wrote:
>>>>> Or ISTM that WakeupRecovery() should set the latch only when the latch
>>>>> has not been reset to NULL yet.
>>>>
>>>> Got to be careful with race conditions, if the latch is set to NULL at
>>>> the same time that WakeupRecovery() is called.
>>>
>>> I don't think commit 113d3591b8 got this quite right:
>>>
>>>> void
>>>> WakeupRecovery(void)
>>>> {
>>>> if (XLogCtl->recoveryWakeupLatch)
>>>> SetLatch(XLogCtl->recoveryWakeupLatch);
>>>> }
>>>
>>> If XLogCtl->recoveryWakeupLatch is set to NULL between the if and the SetLatch, you'll still get a segfault. That's highly unlikely to happen in practice because the compiler will optimize that into a single load instruction, but could happen with -O0. I think you'd need to do the access only once, using a volatile pointer, to make that safe.
>
> On second thought, since fetching the latch pointer might not be atomic,
> maybe we need to use spinlock like WalRcvForceReply() does. But using
> spinlock in every calls of WakeupRecovery() might cause performance
> overhead. So I'm thinking to use spinlock only when it's necessary, i.e.,
> when the latch may be reset to NULL concurrently. Attached is the POC
> patch implementing that. Thought?
Previously I added this patch to next CommitFest. But I reverted the commit
ac22929a26 and 113d3591b8 because those changes have other issue. So this
patch is no longer necessary, and I dropped it from next CommitFest.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2020-12-18 01:51:05 | pgsql: pg_stat_statements: Track time at which all statistics were last |
Previous Message | Fujii Masao | 2020-12-17 09:09:39 | pgsql: Revert "Get rid of the dedicated latch for signaling the startup |