Re: [RFC] Should we fix postmaster to avoid slow shutdown?

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [RFC] Should we fix postmaster to avoid slow shutdown?
Date: 2016-10-27 09:05:04
Message-ID: CAFjFpReLusCq1f1W6d_hQERfJvau+UWkU_sznT=uG8_qSDg=ag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 27, 2016 at 7:29 AM, Tsunakawa, Takayuki
<tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
> From: pgsql-hackers-owner(at)postgresql(dot)org
>> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Ashutosh Bapat
>> In pgstat_quickdie(), I think a call to sigaddset(&BlockSig, SIGQUIT) is
>> missing before PG_SETMASK(). Although there are some SIGQUIT handlers which
>> do not have that call. But I guess, it will be safer to have it.
>
> I didn't add it because pgstat_quickdie() just exits, like some other postmaster children. I thought those processes which are concerned about their termination processing call sigaddset(SIGQUIT), so I went after the processes who aren't. Is this really necessary?
>

Ok. In that case, I think we shouldn't even call PG_SETMASK() similar
to pgarch_exit(). Attached patch removes PG_SETMASK(). Let me know if
it looks good.

>> Also, many other SIGQUIT handlers like bgworker_quickdie() call
>> on_exit_reset() followed by exit(2) instead of just exit(1) in
>> pgstat_quickdie(). Why is this difference?
>
> As Robert and Tom said, either exit(1) or exit(2) is OK because reaper() handles non-zero exit code the same.

Yes, per reaper().
2955 /*
2956 * Was it the statistics collector? If so, just try to start a new
2957 * one; no need to force reset of the rest of the system.
(If fail,
2958 * we'll try again in future cycles of the main loop.)
2959 */
2960 if (pid == PgStatPID)
2961 {
2962 PgStatPID = 0;
2963 if (!EXIT_STATUS_0(exitstatus))
2964 LogChildExit(LOG, _("statistics collector process"),
2965 pid, exitstatus);
2966 if (pmState == PM_RUN)
2967 PgStatPID = pgstat_start();
2968 continue;
2969 }

> Regarding on_proc_reset(), stats collector is not attached to the shared memory and does not register on_proc_exit() callbacks. These situations are the same as the archiver process, so I followed it.
>

Right. I got confused because of on_shmem_exit() call in
pgstat_initialize. But that's per backend code and not executed within
stats collector.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
01_pgstat_avoid_writing_on_sigquit_v2.patch text/x-patch 4.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2016-10-27 09:06:04 Re: Push down more full joins in postgres_fdw
Previous Message Michael Paquier 2016-10-27 08:08:47 Re: WAL consistency check facility