Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested. - Mailing list pgsql-hackers

From Masahiro Ikeda
Subject Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested.
Date
Msg-id b5c42502b3984a0d5bc19f7ec39093e2@oss.nttdata.com
Whole thread Raw
In response to Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested.  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: make the stats collector shutdown without writing the statsfiles if the immediate shutdown is requested.
List 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



pgsql-hackers by date:

Previous
From: Jan Wieck
Date:
Subject: Re: pg_upgrade failing for 200+ million Large Objects
Next
From: Thomas Munro
Date:
Subject: Re: Fix DROP TABLESPACE on Windows with ProcSignalBarrier?