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

On Thu, Oct 27, 2016 at 7:29 AM, Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
> From: pgsql-hackers-owner@postgresql.org
>> [mailto:pgsql-hackers-owner@postgresql.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
whichare 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