Re: Add exclusive backup deprecation notes to documentation - Mailing list pgsql-hackers

From David Steele
Subject Re: Add exclusive backup deprecation notes to documentation
Date
Msg-id d80dacb2-06c8-9ba9-3828-0e203f6cf74a@pgmasters.net
Whole thread Raw
In response to Re: Add exclusive backup deprecation notes to documentation  (Magnus Hagander <magnus@hagander.net>)
Responses Re: Add exclusive backup deprecation notes to documentation
List pgsql-hackers
On 3/8/19 6:08 AM, Magnus Hagander wrote:
> On Thu, Mar 7, 2019 at 5:35 PM Michael Paquier <michael@paquier.xyz 
> <mailto:michael@paquier.xyz>> wrote:
> 
>     On Thu, Mar 07, 2019 at 11:33:20AM +0200, David Steele wrote:
>      > OK, here's a new version that splits the deprecation notes from the
>      > discussion of risks.  I also fixed the indentation.
> 
>     The documentation part looks fine to me.  Just one nit regarding the
>     error hint.
> 
>      > -     errhint("If you are not restoring from a backup, try
>     removing the file \"%s/backup_label\".", DataDir)));
>      > +     errhint("If you are restoring from a backup, touch
>     \"%s/recovery.signal\" and add recovery options to
>     \"%s/postgresql.auto.conf\".\n"
> 
>     Here do we really want to recommend adding options to
>     postgresql.auto.conf?  This depends a lot on the solution integration
>     so I think that this hint could actually confuse some users because it
>     implies that they kind of *have* to do so, which is not correct.  I
>     would recommend to be a bit more generic and just use "and add
>     necessary recovery configuration".
> 
> 
> Agreed, I think we should never tell people to "add recovery options to 
> postgresql.auto.conf". Becuase they should never do that manually. If we 
> want to suggest people use postgresql.auto.conf surely they should be 
> using ALTER SYSTEM SET. Which of course doesn't work in this case, since 
> postgrsql isn't running yet.
> 
> So yeah either that, or say "add to postgresql.conf" without the auto?

I went with Michael's suggestion.  Attached is a new patch.

I also think we should set a flag and throw the error below this if/else 
block.  This is a rather large message and maintaining two copies of it 
is not ideal.

Please note that there have been objections to the patch later in this 
thread by Peter and Robert.  I'm not very interested in watering down 
the documentation changes as Peter suggests, but I think at the very 
least we should commit the added hints in the error message.  For many 
users this error will be their first point of contact with the 
backup_label issue/behavior.

Regards,
-- 
-David
david@pgmasters.net

Attachment

pgsql-hackers by date:

Previous
From: Alexander Kuzmenkov
Date:
Subject: Re: Removing unneeded self joins
Next
From: Michael Paquier
Date:
Subject: Re: Offline enabling/disabling of data checksums