On Thu, Mar 7, 2019 at 5:35 PM Michael Paquier <mich...@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?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

Reply via email to