Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(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 03:13:34
Message-ID: CALj2ACX4rQRHuczPnkr6Ki+PxdKR33aj9LYRFFO3t8Xgx=8eVw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Nov 25, 2020 at 8:08 PM Bharath Rupireddy <
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.

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()

[1]
https://www.postgresql.org/message-id/CALj2ACWWy1YcngpCUn09AsXMfOzwjfNqbVosfoRY0vhhVWhVBw%40mail.gmail.com

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v1-0001-Improve-error-message-in-ProcessInterrupts-for-bg-worker.patch application/octet-stream 1.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Nancarrow 2020-11-27 03:14:48 Re: Parallel plans and "union all" subquery
Previous Message Kyotaro Horiguchi 2020-11-27 03:05:45 Re: Allow some recovery parameters to be changed with reload