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

On 2021/03/18 19:16, Masahiro Ikeda wrote:
> As you said, the temporary stats files don't removed if the stats collector is killed with SIGQUIT.
> So, if the user change the GUC parameter "stats_temp_directory" after immediate shutdown,
> temporary stats file can't be removed forever.
>
> But, I think this case is rarely happened and unimportant. Actually, pgstat_write_statsfiles()
> didn't check error of unlink() and the same problem is occurred if the server is crashed now.
> The documentation said following. I think it's enough.

Thanks for considering this! Understood.

> I don't have any strong opinion this behaivor is useless too.
>
> Since the reinitialized phase is not executed when only the stats collector is crashed
> (since it didn't touch the shared memory), if the permanent stats file is exists, the
> stats collector can use it. But, IIUC, the case is rare.
>
> The case is happened by operation mistake which a operator sends the SIGQUIT signal to
> the stats collector. Please let me know if there are more important case.
>
> But, if SIGKILL is sent by operator, the stats can't be rescure now because the permanent stats
> files can't be written before exiting. Since the case which can rescure the stats is rare,
> I think it's ok to initialize the stats even if SIGQUIT is sent.

Sounds reasonable.

>> 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?

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...

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION