public inbox for [email protected]  

help / color / mirror / Atom feed

From: Fujii Masao <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
Date: Thu, 12 Nov 2020 13:36:07 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACXPEW6wpx+-s5dqcDV2Y+G0U-wu79ZHr7mPRF8G_YwB-Q@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CALj2ACWz1R+gRt2zJCSsWZeCgQhnM6-E-4XFotqqUwbyYk7ZEA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CALj2ACXPEW6wpx+-s5dqcDV2Y+G0U-wu79ZHr7mPRF8G_YwB-Q@mail.gmail.com>



On 2020/11/10 21:30, Bharath Rupireddy wrote:
> On Tue, Nov 10, 2020 at 3:04 PM Fujii Masao <[email protected]> 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.

Thanks for the analysis! I pushed the patch.

Regards,

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

view thread (38+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox