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

On 2021/03/26 9:54, Fujii Masao wrote:
> 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:
>>>> 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);

Thanks, I agree the above code if it's ok that the exit status of the stats
collector is not 0 in immediate shutdown case.

Regards,

--
Masahiro Ikeda
NTT DATA CORPORATION