Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robert(dot)haas(at)enterprisedb(dot)com>
Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
Date: 2020-11-27 06:56:52
Message-ID: 9e3f88cd-0cc4-ac9e-4102-505ac8213fc9@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020/11/27 12:13, Bharath Rupireddy wrote:
> On Wed, Nov 25, 2020 at 8:08 PM Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com <mailto:bharath(dot)rupireddyforpostgres(at)gmail(dot)com>> wrote:
> >
> > > When I read the patch again, I found that, with the patch, the shutdown
> > > of worker_spi causes to report the following FATAL message.
> > >
> > >      FATAL:  terminating connection due to administrator command
> > >
> > > Isn't this message confusing because it's not a connection? If so,
> > > we need to update ProcessInterrupts() so that the proper message is
> > > reported like other bgworkers do.
> > >
> >
> > This is also true for all the bgworker that use the die() handler. How about doing it the way bgworker_die() does in ProcessInterrupts()? This would give meaningful information. Thoughts? If okay, I can make a separate patch.
> >
>
> Attaching the patch that improved the message for bg workers in ProcessInterrupts(). For instance, now it looks like *FATAL:  terminating background worker "worker_spi" due to administrator command* or *FATAL:  terminating background worker "parallel worker" due to administrator command *and so on for other bg workers.*
> *
>
> Please review the patch.

Thanks for the patch! It looks good to me.

>
> I'm also mentioning the 2 previous patches posted in [1]. One of the patch is for using die() instead of handle_sigterm() in test_shm_mq/worker.c and another is for replacing custom SIGTERM handler worker_spi_sigterm() with die() and custom SIGHUP handler worker_spi_sighup() with standard SignalHandlerForConfigReload()

Yeah, I pushed them. Thanks!

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message tsunakawa.takay@fujitsu.com 2020-11-27 07:01:16 RE: Disable WAL logging to speed up data loading
Previous Message Luc Vlaming 2020-11-27 06:56:19 Re: Parallel plans and "union all" subquery