From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Subject: | Re: Reword docs of feature "Remove temporary files after backend crash" |
Date: | 2021-10-10 13:33:32 |
Message-ID: | CALj2ACWdDyWM=n=cLRf+KMHDa=-6n+G1qAvNMH5fQBji4FPEjA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Oct 10, 2021 at 9:12 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
> On 2021/10/10 1:25, Bharath Rupireddy wrote:
> > On Sat, Oct 9, 2021 at 9:42 PM Justin Pryzby <pryzby(at)telsasoft(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2021-10-10 14:04:31 | Re: Skipping logical replication transactions on subscriber side |
Previous Message | Peter Eisentraut | 2021-10-10 12:51:38 | Re: Time to upgrade buildfarm coverage for some EOL'd OSes? |