On Sun, Oct 10, 2021 at 9:12 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> On 2021/10/10 1:25, Bharath Rupireddy wrote:
> > On Sat, Oct 9, 2021 at 9:42 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >>
> >> I doubt there's much confusion here - crashes are treated the same. I think
> >> the fix would be to say "server crash" rather than backend crash.
> >
> > IIUC, the "server crash" includes any backend, auxiliary process,
> > postmaster crash i.e. database cluster/instance crash. The commit
> > cd91de0d1 especially added the temp file cleanup support if any
> > backend or auxiliary process (except syslogger and stats collector)
>
> Also the startup process should be in this exception list?
Yes, if the startup process fails, neither restart_after_crash nor
remove_temp_files_after_crash code path is hit.
> > crashes. The temp file cleanup in postmaster crash does exist prior to
> > the commit cd91de0d1.
> >
> > When we add the description about the new GUC introduced by the commit
> > cd91de0d1, let's be clear as to which crash triggers the new temp file
> > cleanup path.
>
> If we really want to add this information, the description of
> restart_after_crash seems more proper place to do that in.
> remove_temp_files_after_crash works only when the processes are
> reinitialized because restart_after_crash is enabled.
IMO, we can add the new description as proposed in my v1 patch (after
adding startup process to the exception list) to both the GUCs
restart_after_crash and remove_temp_files_after_crash. And, in
remove_temp_files_after_crash GUC description we can just add a note
saying "this GUC is effective only when restart_after_crash is on".
Also, I see that the restart_after_crash and
remove_temp_files_after_crash descriptions in guc.c say "Remove
temporary files after backend crash.". I think we can also modify them
to "Remove temporary files after the backend or auxiliary process
(except startup, syslogger and stats collector) crash.
Thoughts?
Regards,
Bharath Rupireddy.
