Add exclusive backup deprecation notes to documentation

David Steele wrote:

This patch attempts to document the limitations of the exclusive mode.

Thanks!

+   <para>
+     The primary issue with the exclusive method is that the
+     <filename>backup_label</filename> file is written into the data directory
+     when <function>pg_start_backup</function> is called and remains until
+     <function>pg_stop_backup</function> is called.  If
+     <productname>PostgreSQL</productname> or the host terminates abnormally

There should be a comma at the end of this line.

+     then <filename>backup_label</filename> will be left in the data directory
+     and <productname>PostgreSQL</productname> will not start. A log message

You should say "*may* not start", because it will if the WAL segment is still there.

+     recommends that <filename>backup_label</filename> be removed if not
+     restoring from a backup.
+   </para>

Yours,
Laurenz Albe

On 2/28/19 6:08 PM, Laurenz Albe wrote:

David Steele wrote:

This patch attempts to document the limitations of the exclusive mode.

Thanks!

+   <para>
+     The primary issue with the exclusive method is that the
+     <filename>backup_label</filename> file is written into the data directory
+     when <function>pg_start_backup</function> is called and remains until
+     <function>pg_stop_backup</function> is called.  If
+     <productname>PostgreSQL</productname> or the host terminates abnormally

There should be a comma at the end of this line.

Fixed.

+     then <filename>backup_label</filename> will be left in the data directory
+     and <productname>PostgreSQL</productname> will not start. A log message

You should say "*may* not start", because it will if the WAL segment is still there.

You are correct. It's still pretty likely though so I went with "probably".

I added some extra language to the warning that gets emitted in the log.
Users are more like to see that than the documentation.

I also addressed a comment from another thread by adding pg_basebackup
as .e.g. rather than or.

Thanks,
--
-David
david@pgmasters.net

David Steele wrote:

I added some extra language to the warning that gets emitted in the log.
Users are more like to see that than the documentation.

I also addressed a comment from another thread by adding pg_basebackup
as .e.g. rather than or.

Looks good to me.

Yours,
Laurenz Albe

El vie., 1 de mar. de 2019 a la(s) 06:21, David Steele
(david@pgmasters.net) escribió:

I also addressed a comment from another thread by adding pg_basebackup
as .e.g. rather than or.

Thanks David,

This looks very good!

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

El vie., 1 de mar. de 2019 a la(s) 06:21, David Steele
(david@pgmasters.net) escribió:

I added some extra language to the warning that gets emitted in the log.
Users are more like to see that than the documentation.

I also addressed a comment from another thread by adding pg_basebackup
as .e.g. rather than or.

More awake, I gave this last patch a second read. Wording is good now.
No objections there at all.

I do think that paragraph 2 and 3 should be merged as it seems the
idea on the third is a continuation of what's in the second.

But even without that change, I believe this patch is good for commit.

Regards,

--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Please follow the 1-space indentation in the documentation files.

I think the style of mentioning all the problems in a note before the
actual description is a bit backwards.

The layout of the section should be:

- This is what it does.

- Here are some comparisons with other methods.

- For this or that reason, it's deprecated.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 3/1/19 1:13 PM, Peter Eisentraut wrote:

Please follow the 1-space indentation in the documentation files.

Whoops. Will fix.

I think the style of mentioning all the problems in a note before the
actual description is a bit backwards.

In the case of an important note like this I think it should be right at
the top where people will see it. Not everyone reads to the end.

--
-David
david@pgmasters.net

On Fri, Mar 1, 2019 at 2:07 PM David Steele <david@pgmasters.net> wrote:

On 3/1/19 1:13 PM, Peter Eisentraut wrote:

Please follow the 1-space indentation in the documentation files.

Whoops. Will fix.

I think the style of mentioning all the problems in a note before the
actual description is a bit backwards.

In the case of an important note like this I think it should be right at
the top where people will see it. Not everyone reads to the end

Maybe have the first note say "This method is deprecated bceause it has
serious risks (see bellow)" and then list the actual risks at the end?

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

Magnus Hagander wrote:

Maybe have the first note say "This method is deprecated bceause it has serious
risks (see bellow)" and then list the actual risks at the end?

Good idea. That may attract the attention of the dogs among the readers.

Yours,
Laurenz Albe

On 3/1/19 3:14 PM, Laurenz Albe wrote:

Magnus Hagander wrote:

Maybe have the first note say "This method is deprecated bceause it has serious
risks (see bellow)" and then list the actual risks at the end?

Good idea. That may attract the attention of the dogs among the readers.

OK, here's a new version that splits the deprecation notes from the
discussion of risks. I also fixed the indentation.

Thanks,
--
-David
david@pgmasters.net

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".

+		"If you are not restoring from a backup, try removing the file \"%s/backup_label\".\n"
+		"Be careful: removing \"%s/backup_label\" will result in a corrupt cluster if restoring from a backup.",

Fine for these two ones.

+ DataDir, DataDir, DataDir, DataDir)));

:)
--
Michael

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

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

On 2019-03-07 10:33, David Steele wrote:

On 3/1/19 3:14 PM, Laurenz Albe wrote:

Magnus Hagander wrote:

Maybe have the first note say "This method is deprecated bceause it has serious
risks (see bellow)" and then list the actual risks at the end?

Good idea. That may attract the attention of the dogs among the readers.

OK, here's a new version that splits the deprecation notes from the
discussion of risks. I also fixed the indentation.

The documentation changes appear to continue the theme from the other
thread that the exclusive backup mode is terrible and everyone should
feel bad about it. I don't think there is consensus about that.

I do welcome a more precise description of the handling of backup_label
and a better hint in the error message. I think we haven't gotten to
the final shape there yet, especially for the latter. I suggest to
focus on that.

The other changes repeat points already made in nearby documentation.

I think it would be helpful to frame the documentation in a way to
suggest that the nonexclusive mode is more for automation and more
sophisticated tools and the exclusive mode is more for manual or simple
scripted use.

If we do think that the exclusive mode will be removed in PG13, then I
don't think we need further documentation changes. It already says it's
deprecated, and we don't need to justify that at length. But again, I'm
not convinced that that will happen.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Mar 18, 2019 at 8:33 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

The documentation changes appear to continue the theme from the other
thread that the exclusive backup mode is terrible and everyone should
feel bad about it. I don't think there is consensus about that.

+1.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Greetings,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 2019-03-07 10:33, David Steele wrote:

On 3/1/19 3:14 PM, Laurenz Albe wrote:

Magnus Hagander wrote:

Maybe have the first note say "This method is deprecated bceause it has serious
risks (see bellow)" and then list the actual risks at the end?

Good idea. That may attract the attention of the dogs among the readers.

OK, here's a new version that splits the deprecation notes from the
discussion of risks. I also fixed the indentation.

The documentation changes appear to continue the theme from the other
thread that the exclusive backup mode is terrible and everyone should
feel bad about it. I don't think there is consensus about that.

I don't view it as up for much debate. The exclusive backup mode is
quite bad.

I do welcome a more precise description of the handling of backup_label
and a better hint in the error message. I think we haven't gotten to
the final shape there yet, especially for the latter. I suggest to
focus on that.

There isn't a way to handle the backup_label in a sane way when it's
created by the server in the data directory, which is why the
non-exclusive mode explicitly doesn't do that.

I think it would be helpful to frame the documentation in a way to
suggest that the nonexclusive mode is more for automation and more
sophisticated tools and the exclusive mode is more for manual or simple
scripted use.

I don't agree with this at all, that's not the reason the two exist nor
were they ever developed with the intent that one is for the 'simple'
case and one is for the 'automated' case. Trying to wedge them into
that framework strikes me as simply trying to sweep the serious issues
under the rug and I don't agree with that- if we are going to continue
to have this, we need to make it clear what the issues are. Sadly, we
will still have users who don't actually read the docs that carefully
and get bit by the exclusive backup mode because they didn't appreciate
the issues, but we will continue to have that until we finally remove
the exclusive mode.

If we do think that the exclusive mode will be removed in PG13, then I
don't think we need further documentation changes. It already says it's
deprecated, and we don't need to justify that at length. But again, I'm
not convinced that that will happen.

There were at least a few comments made on this thread that it wasn't
made clear enough in the documentation that it's deprecated. Saying
that we already deprecated it doesn't change that and doesn't do
anything to actually address those concerns. Continuing to carry
forward these two modes makes further progress in this area difficult
and unlikely to happen, and that's disappointing.

Thanks!

Stephen

On Mon, Mar 18, 2019 at 11:13 PM Stephen Frost <sfrost@snowman.net> wrote:

I don't view it as up for much debate.

In other words, you're not willing to listen to what other people
think about this issue.

I can't say I haven't noticed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Mar 18, 2019 at 11:13 PM Stephen Frost <sfrost@snowman.net> wrote:

I don't view it as up for much debate.

In other words, you're not willing to listen to what other people
think about this issue.

I have listened, but unfortunately the discussion just revolves around
"oh, it isn't actually all that bad", which, no, isn't something that's
going to sway my opinion.

I suppose some might view that as being principled.

Thanks!

Stephen

Hi,

On 2019-03-18 23:35:07 -0400, Stephen Frost wrote:

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Mar 18, 2019 at 11:13 PM Stephen Frost <sfrost@snowman.net> wrote:

I don't view it as up for much debate.

In other words, you're not willing to listen to what other people
think about this issue.

I have listened, but unfortunately the discussion just revolves around
"oh, it isn't actually all that bad", which, no, isn't something that's
going to sway my opinion.

I think you're right about the original issue. But at some point you
just gotta settle for not everyone agreeing with you (and me). Just
pushing forward hard won't achieve anything. Nobody says you need to
agree with people saying "it's not all that bad", but that doesn't mean
you should just try to push forward at full speed.

Greetings,

Andres Freund

Greetings,

* Andres Freund (andres@anarazel.de) wrote:

On 2019-03-18 23:35:07 -0400, Stephen Frost wrote:

* Robert Haas (robertmhaas@gmail.com) wrote:

On Mon, Mar 18, 2019 at 11:13 PM Stephen Frost <sfrost@snowman.net> wrote:

I don't view it as up for much debate.

In other words, you're not willing to listen to what other people
think about this issue.

I have listened, but unfortunately the discussion just revolves around
"oh, it isn't actually all that bad", which, no, isn't something that's
going to sway my opinion.

I think you're right about the original issue. But at some point you
just gotta settle for not everyone agreeing with you (and me). Just
pushing forward hard won't achieve anything. Nobody says you need to
agree with people saying "it's not all that bad", but that doesn't mean
you should just try to push forward at full speed.

This thread didn't exactly pop out of nowhere.. It's a result of the
prior discussion, with the goal of at least improving the situation by
documenting the issues and trying to make it clearer that the exclusive
mode is deprecated to hopefully provide a chance that we'll actually
remove it at some point in the future as being dangerous.

That's a pretty large step back from "let's rip it out for v12", and yet
people are now pushing back against even that, which is definitely
frustrating, and saying that I'm still pushing forward 'at full speed'
definitely comes across as not really considering that this started out
with "rip it out for v12" and has now regressed back to a documentation
patch.

Thanks!

Stephen