MIME-Version: 1.0
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>
 <bc313fdc-2beb-52d1-b8a7-ef1e4cfb92bf@oss.nttdata.com>
In-Reply-To: <bc313fdc-2beb-52d1-b8a7-ef1e4cfb92bf@oss.nttdata.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sat, 7 Nov 2020 19:31:21 +0530
Message-ID: 
 <CALj2ACWz1R+gRt2zJCSsWZeCgQhnM6-E-4XFotqqUwbyYk7ZEA@mail.gmail.com>
Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Nov 6, 2020 at 11:00 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> >> 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.
>

The main reason for having SetLatch() in
SignalHandlerForConfigReload() is to wake up the calling process if
waiting in WaitLatchOrSocket() or WaitLatch() and reload the new
config file and use the reloaded config variables. Maybe we should
give a thought on the scenarios in which the walreceiver process
waits, and what happens in case the latch is set when SIGHUP is
received.

And also, I think it's worth having a look at the commit 40f908bdcdc7
that introduced WalRcvSigHupHandler() and 597a87ccc that replaced
custom latch with procLatch.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com