On 2021/03/26 9:25, Masahiro Ikeda wrote:
On 2021/03/25 21:23, Fujii Masao wrote:
On 2021/03/25 9:58, Andres Freund wrote:
Outside of very
narrow circumstances a normal exit shouldn't use _exit(1). Neither 1 no
_exit() (as opposed to exit()) seem appropriate. This seems like a bad
idea.
So you're thinking that the stats collector should do proc_exit(0)
or something even when it receives immediate shutdown request?
One idea to avoid using _exit(1) is to change the SIGQUIT handler
so that it just sets the flag. Then if the stats collector detects that
the flag is set in the main loop, it gets out of the loop,
skips writing the permanent stats file and then exits with exit(0).
That is, normal and immediate shutdown requests are treated
almost the same way in the stats collector. Only the difference of
them is whether it saves the stats to the file or not. Thought?
Although I don't oppose the idea to change from _exit(1) to proc_exit(0), the
reason why I used _exit(1) is that I thought the behavior to skip writing the
stat counters is not normal exit. Actually, other background processes use
_exit(2) instead of _exit(0) or proc_exit(0) in immediate shutdown although
the status code is different because they touch shared memory.
True.
Also, won't this lead to postmaster now starting to complain about
pgstat having crashed in an immediate shutdown? Right now only exit
status 0 is expected:
if (pid == PgStatPID)
{
PgStatPID = 0;
if (!EXIT_STATUS_0(exitstatus))
LogChildExit(LOG, _("statistics collector process"),
pid, exitstatus);
if (pmState == PM_RUN || pmState == PM_HOT_STANDBY)
PgStatPID = pgstat_start();
continue;
}
No. In immediate shutdown case, pmdie() sets "Shutdown" variable to
ImmdiateShutdown and HandleChildCrash() doesn't complain that in that case
because of the following.
/*
* We only log messages and send signals if this is the first process
* crash and we're not doing an immediate shutdown; otherwise, we're only
* here to update postmaster's idea of live processes. If we have already
* signaled children, nonzero exit status is to be expected, so don't
* clutter log.
*/
take_action = !FatalError && Shutdown != ImmediateShutdown;
IIUC, in immediate shutdown case (and other cases too?), HandleChildCrash() is
never invoked due to the exit of pgstat. My understanding of Andres-san's
comment is that is it ok to show like the following log message?
```
LOG: statistics collector process (PID 64991) exited with exit code 1
```
Surely, other processes don't output the log like it. So, I agree to suppress
the log message.
Yes. I was mistakenly thinking that HandleChildCrash() was called
even when the stats collector exits with non-zero code, like other processes.
But that's not true.
How should we do this? HandleChildCrash() calls LogChildExit()
when FatalError = false and Shutdown != ImmediateShutdown.
We should use the same conditions for the stats collector as follows?
if (pid == PgStatPID)
{
PgStatPID = 0;
- if (!EXIT_STATUS_0(exitstatus))
+ if (!EXIT_STATUS_0(exitstatus) && !FatalError &&
+ Shutdown != ImmediateShutdown)
LogChildExit(LOG, _("statistics collector
process"),
pid, exitstatus);
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
