On Mon, Oct 5, 2020 at 8:04 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > On 2020/10/05 19:45, Bharath Rupireddy wrote: > > Hi, > > > > Autoprewarm module is using it's own SIGHUP(apw_sigterm_handler, > > got_sigterm) and SIGTERM(apw_sighup_handler, got_sighup) handlers which are > > similar to standard signal handlers(except for a difference [1]). Isn't it > > good to remove them and use standard SignalHandlerForConfigReload and > > SignalHandlerForShutdownRequest? > > > > Attaching the patch for the above changes. > > > > Looks like the commit[2] replaced custom handlers with standard handlers. > > > > Thoughts? > > +1 > > The patch looks good to me. >
Thanks. > > ISTM that we can also replace StartupProcSigHupHandler() in startup.c > with SignalHandlerForConfigReload() by making the startup process use > the general shared latch instead of its own one. POC patch attached. > Thought? > I'm not quite sure whether it breaks something or not. I see that WakeupRecovery() with XLogCtl->recoveryWakeupLatch latch from the startup process is also being used in the walreceiver process. I may be wrong, but have some concern if the behaviour is different in case of EXEC_BACKEND and Windows? Another concern is that we are always using XLogCtl->recoveryWakeupLatch in shared mode, this makes sense as this latch is also being used in walrecevier. But sometimes, MyLatch is created in non-shared mode as well(see InitLatch(MyLatch)). Others may have better thoughts though. > > Probably we can also replace sigHupHandler() in syslogger.c with > SignalHandlerForConfigReload()? This would be separate patch, though. > +1 to replace sigHupHandler() with SignalHandlerForConfigReload() as the latch and the functionality are pretty much the same. WalReceiverMai(): I think we can also replace WalRcvShutdownHandler() with SignalHandlerForShutdownRequest() because walrcv->latch point to &MyProc->procLatch which in turn point to MyLatch. Thoughts? If okay, we can combine these into a single patch. I will post an updated patch soon. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
