On Tue, Nov 10, 2020 at 3:04 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > >> 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. > > > > The difference is whether the config file is processed at the next > > wakeup (by force-reply-request or SIGTERM) of walreceiver or > > immediately. If server-reload happened frequently, say, several times > > per second(?), we should consider to reduce the useless reloading, but > > actually that's not the case. > > So, attached is the patch that makes walreceiver use both standard > SIGTERM and SIGHUP handlers. Currently I've not found any actual > issues by making walreceiver use standard SIGHUP handler, yet. >
I think it makes sense to replace WalRcv->latch with MyProc->procLatch(as they point to the same latch) in the functions that are called in the walreceiver process. However, we are using walrcv->latch with spinlock, say in WalRcvForceReply() and RequestXLogStreaming() both are called from the startup process. See commit 45f9d08684, that made the access to the walreceiver's latch(WalRcv->latch) by the other process(startup) spinlock protected And looks like, in general it's a standard practice to set latch to wake up the process if waiting in case a SIGHUP signal is received and reload the relevant config variables. Going by the above analysis, the v3 patch looks good to me. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
