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
da...@pgmasters.net
From f2dafe53a5da8916c3b0390b9ebf657b931013f5 Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Thu, 7 Mar 2019 11:30:19 +0200
Subject: [PATCH] Add exclusive backup deprecation notes to documentation.

Update the exclusive backup documentation to explain the limitations of
the exclusive backup mode and make it clear that the feature is
deprecated.

Update the log message when the backup_label is found but recovery
cannot proceed.
---
 doc/src/sgml/backup.sgml          | 46 +++++++++++++++++++++++++++++++
 src/backend/access/transam/xlog.c | 10 +++++--
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index a73fd4d044..ab5c81d382 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -948,6 +948,17 @@ SELECT * FROM pg_stop_backup(false, true);
    </sect3>
    <sect3 id="backup-lowlevel-base-backup-exclusive">
     <title>Making an exclusive low level backup</title>
+
+    <note>
+     <para>
+      The exclusive backup method is deprecated and should be avoided in favor
+      of the non-exclusive backup method, e.g.
+      <application>pg_basebackup</application>.  This method is deprecated
+      because it has serious risks which are enumerated at the end of this
+      section.
+     </para>
+    </note>
+
     <para>
      The process for an exclusive backup is mostly the same as for a
      non-exclusive one, but it differs in a few key steps. This type of backup
@@ -1054,6 +1065,41 @@ SELECT pg_stop_backup();
    </listitem>
   </orderedlist>
     </para>
+
+    <note>
+     <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,
+      then <filename>backup_label</filename> will be left in the data directory
+      and <productname>PostgreSQL</productname> probably will not start. A log
+      message recommends that <filename>backup_label</filename> be removed if
+      not restoring from a backup.
+     </para>
+
+     <para>
+      However, if <filename>backup_label</filename> is present because a 
restore
+      is actually in progress, then removing it will result in corruption.  For
+      this reason it is not recommended to automate the removal of
+      <filename>backup_label</filename>.
+     </para>
+
+     <para>
+      Another issue with exclusive backup mode is that it will continue until
+      <function>pg_stop_backup</function> is called, even if the calling 
process
+      is no longer performing the backup.  The next time
+      <function>pg_start_backup</function> is called it will fail unless
+      <function>pg_stop_backup</function> is called manually first.
+     </para>
+
+     <para>
+      Finally, only one exclusive backup may be run at a time.  However, it is
+      possible to run an exclusive backup at the same time as any number of
+      non-exclusive backups.
+     </para>
+    </note>
    </sect3>
    <sect3 id="backup-lowlevel-base-backup-data">
    <title>Backing up the data directory</title>
diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
index ecd12fc53a..e12a04414b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6405,14 +6405,20 @@ StartupXLOG(void)
                                if (!ReadRecord(xlogreader, checkPoint.redo, 
LOG, false))
                                        ereport(FATAL,
                                                        (errmsg("could not find 
redo location referenced by checkpoint record"),
-                                                        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"
+                                                        "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.",
+                                                        DataDir, DataDir, 
DataDir, DataDir)));
                        }
                }
                else
                {
                        ereport(FATAL,
                                        (errmsg("could not locate required 
checkpoint record"),
-                                        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"
+                                                        "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.",
+                                                        DataDir, DataDir, 
DataDir, DataDir)));
                        wasShutdown = false;    /* keep compiler quiet */
                }
 
-- 
2.17.2 (Apple Git-113)

Reply via email to