Re: cleanup temporary files after crash

On Wed, Mar 10, 2021 at 1:31 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Tue, Mar 09, 2021 at 02:28:43AM +0100, Tomas Vondra wrote:
> > Let's move this patch forward. Based on the responses, I agree the
> > default behavior should be to remove the temp files, and I think we
> > should have the GUC (on the off chance that someone wants to preserve
> > the temporary files for debugging or whatever other reason).
>
> Thanks for taking care of this. I am having some second-thoughts
> about changing this behavior by default, still that's much more useful
> this way.

+1 for having it on by default.

I was also just looking at this patch and came here to say LGTM except
for two cosmetic things, below.

> > I propose to rename the GUC to remove_temp_files_after_crash, I think
> > "remove" is a bit clearer than "cleanup". I've also reworded the sgml
> > docs a little bit.
>
> "remove" sounds fine to me.

+1

> > Attached is a patch with those changes. Barring objections, I'll get
> > this committed in the next couple days.
>
> + When set to on, <productname>PostgreSQL</productname> will automatically
> Nit: using a <literal> markup for the "on" value.

Maybe should say "which is the default", like other similar things?

> +#remove_temp_files_after_crash = on # remove temporary files after
> +# # backend crash?
> The indentation of the second line is incorrect here (Incorrect number
> of spaces in tabs perhaps?), and there is no need for the '#' at the
> beginning of the line.

Yeah, that's wrong. For some reason that one file uses a tab size of
8, unlike the rest of the tree (I guess because people will read that
file in software with the more common setting of 8). If you do :set
tabstop=8 in vim, suddenly it all makes sense, but it is revealed that
this patch has it wrong, as you said. (Perhaps this file should have
some of those special Vim/Emacs control messages so we don't keep
getting this wrong?)