Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested.

From: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: kuroda(dot)hayato(at)fujitsu(dot)com, Zhihong Yu <zyu(at)yugabyte(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested.
Date: 2021-03-23 00:05:33
Message-ID: 20f5bda1-c8d8-61f0-6163-f022ec01b67e@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2021/03/22 23:59, Fujii Masao wrote:
>
>
> On 2021/03/20 13:40, Masahiro Ikeda wrote:
>> Sorry, there is no evidence we should call it.
>> I thought that to call FreeWaitEventSet() is a manner after using
>> CreateWaitEventSet() and the performance impact to call it seems to be too
>> small.
>>
>> (Please let me know if my understanding is wrong.)
>> The reason why I said this is a manner because I thought it's no problem
>> even without calling FreeWaitEventSet() before the process exits regardless
>> of fast or immediate shutdown. Since the "WaitEventSet *wes" is a process
>> local resource,
>> this will be released by OS even if FreeWaitEventSet() is not called.
>>
>> I'm now changing my mind to skip it is better because the code can be
>> simpler and,
>> it's unimportant for the above reason especially when the immediate shutdown is
>> requested which is a bad manner itself.
>
> Thanks for explaining this! So you're thinking that v3 patch should be chosen?
> Probably some review comments I posted upthread need to be applied to
> v3 patch, e.g., rename of SignalHandlerForUnsafeExit() function.

Yes. I attached the v5 patch based on v3 patch.
I renamed SignalHandlerForUnsafeExit() and fixed the following comment.

> "SIGTERM or SIGQUIT of our parent postmaster" should be
> "SIGTERM, SIGQUIT, or detect ungraceful death of our parent
> postmaster"?

>> BTW, the SysLoggerMain() create the WaitEventSet, but it didn't call
>> FreeWaitEventSet().
>> Is it better to call FreeWaitEventSet() before exiting too?
>
> Yes if which could cause actual issue. Otherwise I don't have strong
> reason to do that for now..

OK. AFAIK, this doesn't lead critical problems like memory leak.

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION

Attachment Content-Type Size
v5-0001-pgstat_avoid_writing_on_sigquit.patch text/x-patch 7.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2021-03-23 00:15:02 Re: shared-memory based stats collector
Previous Message Tom Lane 2021-03-22 23:53:39 Re: [HACKERS] Custom compression methods