On 2020/10/06 1:18, Bharath Rupireddy wrote:
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?
Unless I'm wrong, regarding MyLatch, the behavior is not different
whether in EXEC_BACKEND or not. In both cases, SwitchToSharedLatch()
is called and MyLatch is set to the shared latch, i.e., MyProc->procLatch.
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)).
Yes, and then the startup process calls SwitchToSharedLatch() and
repoint MyLatch to the shared one.
Others may have better thoughts though.
Okay, I will reconsider the patch and post it separately later if necessary.
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.
+1 Or it's also ok to make each patch separately.
Anyway, thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
