Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 bharath.rupireddyforpostgres@gmail.com
Cc: pgsql-hackers@postgresql.org
References: <ce414f11-1418-511c-c840-a3cdd7f2b232@oss.nttdata.com>
 <6b561c73-27ce-7079-b6e5-440e49653810@oss.nttdata.com>
 <CALj2ACUdvTiD3zpGMsO6HcMOLYDs=LXRkX72NUCExVdna=qP=Q@mail.gmail.com>
 <20201105.121206.819769369617763972.horikyota.ntt@gmail.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <bc313fdc-2beb-52d1-b8a7-ef1e4cfb92bf@oss.nttdata.com>
Date: Sat, 7 Nov 2020 02:30:44 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.4.0
MIME-Version: 1.0
In-Reply-To: <20201105.121206.819769369617763972.horikyota.ntt@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2020/11/05 12:12, Kyotaro Horiguchi wrote:
> At Wed, 4 Nov 2020 21:16:29 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in
>> On Wed, Nov 4, 2020 at 2:36 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>>
>>> Regarding other two patches, I think that it's better to use MyLatch
>>> rather than MyProc->procLatch or walrcv->latch in WaitLatch() and
>>> ResetLatch(), like other code does. Attached are the updated versions
>>> of the patches. Thought?
>>>
>>
>> +1 for replacing MyProc->procLatch with MyLatch in the autoprewarm
>> module, and the patch looks good to me.
> 
> Looks good to me, too.

Thanks for the review! I pushed the patch.

> 
>> I'm not quite sure to replace all the places in the walreceiver
>> process, for instance in WalRcvForceReply() we are using spinlock to
>> acquire the latch pointer and . Others may have better thoughts on
>> this.
> 
> The SIGTERM part looks good. The only difference between
> WalRcvSigHupHandler and SignalHandlerForConfigReload is whether latch
> is set or not.  I don't think it's a problem that config-reload causes
> an extra wakeup.  Couldn't we do the same thing for SIGHUP?

I agree that we can use even standard SIGHUP handler in walreceiver.
I'm not sure why SetLatch() was not called in walreceiver's SIGHUP
handler so far.

Regards,


-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION