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-20 04:40:45
Message-ID: b5c42502b3984a0d5bc19f7ec39093e2@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>> When only the stats collector exits by SIGQUIT, with the patch
>>> FreeWaitEventSet() is also skipped. Is this ok?
>>
>> Thanks, I fixed it.
>
> I'm still not sure if FreeWaitEventSet() is actually necessary in that
> exit case. Could you tell me why you thought FreeWaitEventSet() is
> necessary in the case?

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.

BTW, the SysLoggerMain() create the WaitEventSet, but it didn't call
FreeWaitEventSet().
Is it better to call FreeWaitEventSet() before exiting too?

> Since it's not good to do many things in a signal handler, even when
> FreeWaitEventSet() is really necessary, it should be called at
> subsequent
> startup of the stats collector instead of calling it in the handler
> at the end? BTW, I found bgworker calls FreeWaitEventSet() via
> ShutdownLatchSupport() at its startup. But I'm also not sure if this
> is really necessary at the startup...

OK, I understood that I need to change the signal handler's
implementation
if FreeWaitEventSet() is necessary.

In my understanding from the following commit message, the ordinary
bgworker
which not access the shared memory doesn't use the latch which
postmaster
installed. So, ShutdownLatchSupport() is called at the startup. Though?

2021/3/1 commit: 83709a0d5a46559db016c50ded1a95fd3b0d3be6
```
The signal handler is now installed in all postmaster children by
InitializeLatchSupport(). Those wishing to disconnect from it should
call ShutdownLatchSupport().
```

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2021-03-20 04:47:47 Re: Fix DROP TABLESPACE on Windows with ProcSignalBarrier?
Previous Message Jan Wieck 2021-03-20 04:39:10 Re: pg_upgrade failing for 200+ million Large Objects