Subject: Re: Postgres restart in the middle of exclusive backup and the presence of backup_label file

From: "Bossart, Nathan" <[email-masked]>
Date: Fri, 7 Jan 2022 00:48:21 +0000

On 12/2/21, 1:34 PM, "Bossart, Nathan" <[email-masked]> wrote:
> On 12/2/21, 9:50 AM, "David Steele" <[email-masked]> wrote:
>> On 12/2/21 12:38, David Steele wrote:
>>> On 12/2/21 11:00, Andrew Dunstan wrote:
>>>>
>>>> Should we really be getting rid of
>>>> PostgreSQL::Test::Cluster::backup_fs_hot() ?
>>>
>>> Agreed, it would be better to update backup_fs_hot() to use exclusive
>>> mode and save out backup_label instead.
>>
>> Oops, of course I meant non-exclusive mode.
>
> +1.  I'll fix that in the next revision.

I finally got around to looking into this, and I think I found why it
was done this way in 2018.  backup_fs_hot() runs pg_start_backup(),
closes the session, copies the data, and then runs pg_stop_backup() in
a different session.  This doesn't work with non-exclusive mode
because the backup will be aborted when the session that runs
pg_start_backup() is closed.  pg_stop_backup() will fail with a
"backup is not in progress" error.  Furthermore,
010_logical_decoding_timelines.pl seems to be the only test that uses
backup_fs_hot().

After a quick glance, I didn't see an easy way to hold a session open
while the test does other things.  If there isn't one, modifying
backup_fs_hot() to work with non-exclusive mode might be more trouble
than it is worth.

Nathan

---

From: "Euler Taveira" <[email-masked]>
Date: Thu, 06 Jan 2022 22:20:55 -0300

> backup_fs_hot() to work with non-exclusive mode might be more trouble
You can use IPC::Run to start psql in background. See examples in
src/test/recovery.

--
Euler Taveira
EDB   https://www.enterprisedb.com/

---

From: David Steele <[email-masked]>
Date: Fri, 7 Jan 2022 08:51:14 -0500

> You can use IPC::Run to start psql in background. See examples in

I don't think updating backup_fs_hot() is worth it here.

backup_fs_cold() works just fine for this case and if there is a need 
for backup_fs_hot() in the future it can be implemented as needed.

Regards,
-David

---

From: "Bossart, Nathan" <[email-masked]>
Date: Fri, 7 Jan 2022 18:17:29 +0000

> for backup_fs_hot() in the future it can be implemented as needed.

Thanks for the pointer on IPC::Run.  I had a feeling I was missing
something obvious!

I think I agree with David that it still isn't worth it for just this
one test.  Of course, it would be great to test the non-exclusive
backup logic as much as possible, but I'm not sure that this
particular test will provide any sort of meaningful coverage.

Nathan

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 16 Feb 2022 17:10:53 -0800

Here is a rebased patch.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 8fc744a0b22a1fff9f3470bfdc0e08d9dd5da5be Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 1 Dec 2021 23:50:49 +0000
Subject: [PATCH v2 1/1] remove exclusive backup mode

---
 doc/src/sgml/backup.sgml                      | 154 +-----
 doc/src/sgml/func.sgml                        |  30 +-
 src/backend/access/transam/xlog.c             | 466 ++----------------
 src/backend/access/transam/xlogfuncs.c        | 235 ++-------
 src/backend/catalog/system_functions.sql      |  14 +-
 src/backend/postmaster/postmaster.c           |  46 +-
 src/backend/replication/basebackup.c          |   6 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |   4 +
 src/bin/pg_rewind/filemap.c                   |   6 +-
 src/include/access/xlog.h                     |   3 +-
 src/include/catalog/pg_proc.dat               |  20 +-
 src/include/miscadmin.h                       |   4 -
 src/test/perl/PostgreSQL/Test/Cluster.pm      |  56 +--
 .../t/010_logical_decoding_timelines.pl       |   4 +-
 14 files changed, 113 insertions(+), 935 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 0d69851bb1..f241bcab9c 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -857,16 +857,8 @@ test ! -f /mnt/server/archivedir/00000001000000A900000065 &amp;&amp; cp pg_wal/0
     sequence, and that the success of a step is verified before
     proceeding to the next step.
    </para>
-   <para>
-    Low level base backups can be made in a non-exclusive or an exclusive
-    way. The non-exclusive method is recommended and the exclusive one is
-    deprecated and will eventually be removed.
-   </para>
-
-   <sect3 id="backup-lowlevel-base-backup-nonexclusive">
-    <title>Making a Non-Exclusive Low-Level Backup</title>
     <para>
-     A non-exclusive low level backup is one that allows other
+     A low level backup allows other
      concurrent backups to be running (both those started using
      the same backup API and those started using
      <xref linkend="app-pgbasebackup"/>).
@@ -884,7 +876,7 @@ test ! -f /mnt/server/archivedir/00000001000000A900000065 &amp;&amp; cp pg_wal/0
      rights to run pg_start_backup (superuser, or a user who has been granted
      EXECUTE on the function) and issue the command:
 <programlisting>
-SELECT pg_start_backup('label', false, false);
+SELECT pg_start_backup('label', false);
 </programlisting>
      where <literal>label</literal> is any string you want to use to uniquely
      identify this backup operation. The connection
@@ -905,10 +897,6 @@ SELECT pg_start_backup('label', false, false);
      issue an immediate checkpoint using as much I/O as available.
     </para>
 
-    <para>
-     The third parameter being <literal>false</literal> tells
-     <function>pg_start_backup</function> to initiate a non-exclusive base backup.
-    </para>
    </listitem>
    <listitem>
     <para>
@@ -926,7 +914,7 @@ SELECT pg_start_backup('label', false, false);
     <para>
      In the same connection as before, issue the command:
 <programlisting>
-SELECT * FROM pg_stop_backup(false, true);
+SELECT * FROM pg_stop_backup(true);
 </programlisting>
      This terminates backup mode. On a primary, it also performs an automatic
      switch to the next WAL segment.  On a standby, it is not possible to
@@ -985,142 +973,6 @@ SELECT * FROM pg_stop_backup(false, true);
    </listitem>
   </orderedlist>
     </para>
-   </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.
-      Prior to <productname>PostgreSQL</productname> 9.6, this was the only
-      low-level method available, but it is now recommended that all users
-      upgrade their scripts to use non-exclusive backups.
-     </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 can only be taken on a primary and does not allow concurrent
-     backups.  Moreover, because it creates a backup label file, as
-     described below, it can block automatic restart of the primary server
-     after a crash.  On the other hand, the erroneous removal of this
-     file from a backup or standby is a common mistake, which can result
-     in serious data corruption.  If it is necessary to use this method,
-     the following steps may be used.
-    </para>
-    <para>
-  <orderedlist>
-   <listitem>
-    <para>
-     Ensure that WAL archiving is enabled and working.
-    </para>
-   </listitem>
-   <listitem>
-    <para>
-     Connect to the server (it does not matter which database) as a user with
-     rights to run pg_start_backup (superuser, or a user who has been granted
-     EXECUTE on the function) and issue the command:
-<programlisting>
-SELECT pg_start_backup('label');
-</programlisting>
-     where <literal>label</literal> is any string you want to use to uniquely
-     identify this backup operation.
-     <function>pg_start_backup</function> creates a <firstterm>backup label</firstterm> file,
-     called <filename>backup_label</filename>, in the cluster directory with
-     information about your backup, including the start time and label string.
-     The function also creates a <firstterm>tablespace map</firstterm> file,
-     called <filename>tablespace_map</filename>, in the cluster directory with
-     information about tablespace symbolic links in <filename>pg_tblspc/</filename> if
-     one or more such link is present.  Both files are critical to the
-     integrity of the backup, should you need to restore from it.
-    </para>
-
-    <para>
-     By default, <function>pg_start_backup</function> can take a long time to finish.
-     This is because it performs a checkpoint, and the I/O
-     required for the checkpoint will be spread out over a significant
-     period of time, by default half your inter-checkpoint interval
-     (see the configuration parameter
-     <xref linkend="guc-checkpoint-completion-target"/>).  This is
-     usually what you want, because it minimizes the impact on query
-     processing.  If you want to start the backup as soon as
-     possible, use:
-<programlisting>
-SELECT pg_start_backup('label', true);
-</programlisting>
-     This forces the checkpoint to be done as quickly as possible.
-    </para>
-   </listitem>
-   <listitem>
-    <para>
-     Perform the backup, using any convenient file-system-backup tool
-     such as <application>tar</application> or <application>cpio</application> (not
-     <application>pg_dump</application> or
-     <application>pg_dumpall</application>).  It is neither
-     necessary nor desirable to stop normal operation of the database
-     while you do this. See
-     <xref linkend="backup-lowlevel-base-backup-data"/> for things to
-     consider during this backup.
-    </para>
-    <para>
-     As noted above, if the server crashes during the backup it may not be
-     possible to restart until the <filename>backup_label</filename> file has
-     been manually deleted from the <envar>PGDATA</envar> directory.  Note
-     that it is very important to never remove the
-     <filename>backup_label</filename> file when restoring a backup, because
-     this will result in corruption.  Confusion about when it is appropriate
-     to remove this file is a common cause of data corruption when using this
-     method; be very certain that you remove the file only on an existing
-     primary and never when building a standby or restoring a backup, even if
-     you are building a standby that will subsequently be promoted to a new
-     primary.
-    </para>
-   </listitem>
-   <listitem>
-    <para>
-     Again connect to the database as a user with rights to run
-     pg_stop_backup (superuser, or a user who has been granted EXECUTE on
-     the function), and issue the command:
-<programlisting>
-SELECT pg_stop_backup();
-</programlisting>
-     This function terminates backup mode and
-     performs an automatic switch to the next WAL segment. The reason for the
-     switch is to arrange for the last WAL segment written during the backup
-     interval to be ready to archive.
-    </para>
-   </listitem>
-   <listitem>
-    <para>
-     Once the WAL segment files active during the backup are archived, you are
-     done.  The file identified by <function>pg_stop_backup</function>'s result is
-     the last segment that is required to form a complete set of backup files.
-     If <varname>archive_mode</varname> is enabled,
-     <function>pg_stop_backup</function> does not return until the last segment has
-     been archived.
-     Archiving of these files happens automatically since you have
-     already configured <varname>archive_command</varname>. In most cases this
-     happens quickly, but you are advised to monitor your archive
-     system to ensure there are no delays.
-     If the archive process has fallen behind
-     because of failures of the archive command, it will keep retrying
-     until the archive succeeds and the backup is complete.
-    </para>
-
-    <para>
-     When using exclusive backup mode, it is absolutely imperative to ensure
-     that <function>pg_stop_backup</function> completes successfully at the
-     end of the backup.  Even if the backup itself fails, for example due to
-     lack of disk space, failure to call <function>pg_stop_backup</function>
-     will leave the server in backup mode indefinitely, causing future backups
-     to fail and increasing the risk of a restart failure during the time that
-     <filename>backup_label</filename> exists.
-    </para>
-   </listitem>
-  </orderedlist>
-    </para>
-   </sect3>
    <sect3 id="backup-lowlevel-base-backup-data">
    <title>Backing Up the Data Directory</title>
    <para>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index df3cd5987b..3d4c8bf851 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25683,8 +25683,7 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
         <function>pg_start_backup</function> (
           <parameter>label</parameter> <type>text</type>
           <optional>, <parameter>fast</parameter> <type>boolean</type>
-          <optional>, <parameter>exclusive</parameter> <type>boolean</type>
-          </optional></optional> )
+          </optional> )
         <returnvalue>pg_lsn</returnvalue>
        </para>
        <para>
@@ -25696,20 +25695,6 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
         it specifies executing <function>pg_start_backup</function> as quickly
         as possible.  This forces an immediate checkpoint which will cause a
         spike in I/O operations, slowing any concurrently executing queries.
-        The optional third parameter specifies whether to perform an exclusive
-        or non-exclusive backup (default is exclusive).
-       </para>
-       <para>
-        When used in exclusive mode, this function writes a backup label file
-        (<filename>backup_label</filename>) and, if there are any links in
-        the <filename>pg_tblspc/</filename> directory, a tablespace map file
-        (<filename>tablespace_map</filename>) into the database cluster's data
-        directory, then performs a checkpoint, and then returns the backup's
-        starting write-ahead log location.  (The user can ignore this
-        result value, but it is provided in case it is useful.)  When used in
-        non-exclusive mode, the contents of these files are instead returned
-        by the <function>pg_stop_backup</function> function, and should be
-        copied to the backup area by the user.
        </para>
        <para>
         This function is restricted to superusers by default, but other users
@@ -25723,7 +25708,6 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
          <primary>pg_stop_backup</primary>
         </indexterm>
         <function>pg_stop_backup</function> (
-          <parameter>exclusive</parameter> <type>boolean</type>
           <optional>, <parameter>wait_for_archive</parameter> <type>boolean</type>
           </optional> )
         <returnvalue>setof record</returnvalue>
@@ -25732,13 +25716,8 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
         <parameter>spcmapfile</parameter> <type>text</type> )
        </para>
        <para>
-        Finishes performing an exclusive or non-exclusive on-line backup.
-        The <parameter>exclusive</parameter> parameter must match the
-        previous <function>pg_start_backup</function> call.
-        In an exclusive backup, <function>pg_stop_backup</function> removes
-        the backup label file and, if it exists, the tablespace map file
-        created by <function>pg_start_backup</function>.  In a non-exclusive
-        backup, the desired contents of these files are returned as part of
+        Finishes performing an on-line backup.  The desired contents of the
+        backup label file and the tablespace map file are returned as part of
         the result of the function, and should be written to files in the
         backup area (not in the data directory).
        </para>
@@ -25771,8 +25750,7 @@ LOG:  Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560
         The result of the function is a single record.
         The <parameter>lsn</parameter> column holds the backup's ending
         write-ahead log location (which again can be ignored).  The second and
-        third columns are <literal>NULL</literal> when ending an exclusive
-        backup; after a non-exclusive backup they hold the desired contents of
+        third columns hold the desired contents of
         the label and tablespace map files.
        </para>
        <para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ce78ac413e..e12e86dc2d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -384,29 +384,6 @@ typedef union WALInsertLockPadded
 	char		pad[PG_CACHE_LINE_SIZE];
 } WALInsertLockPadded;
 
-/*
- * State of an exclusive backup, necessary to control concurrent activities
- * across sessions when working on exclusive backups.
- *
- * EXCLUSIVE_BACKUP_NONE means that there is no exclusive backup actually
- * running, to be more precise pg_start_backup() is not being executed for
- * an exclusive backup and there is no exclusive backup in progress.
- * EXCLUSIVE_BACKUP_STARTING means that pg_start_backup() is starting an
- * exclusive backup.
- * EXCLUSIVE_BACKUP_IN_PROGRESS means that pg_start_backup() has finished
- * running and an exclusive backup is in progress. pg_stop_backup() is
- * needed to finish it.
- * EXCLUSIVE_BACKUP_STOPPING means that pg_stop_backup() is stopping an
- * exclusive backup.
- */
-typedef enum ExclusiveBackupState
-{
-	EXCLUSIVE_BACKUP_NONE = 0,
-	EXCLUSIVE_BACKUP_STARTING,
-	EXCLUSIVE_BACKUP_IN_PROGRESS,
-	EXCLUSIVE_BACKUP_STOPPING
-} ExclusiveBackupState;
-
 /*
  * Session status of running backup, used for sanity checks in SQL-callable
  * functions to start and stop backups.
@@ -455,15 +432,12 @@ typedef struct XLogCtlInsert
 	bool		fullPageWrites;
 
 	/*
-	 * exclusiveBackupState indicates the state of an exclusive backup (see
-	 * comments of ExclusiveBackupState for more details). nonExclusiveBackups
-	 * is a counter indicating the number of streaming base backups currently
-	 * in progress. forcePageWrites is set to true when either of these is
-	 * non-zero. lastBackupStart is the latest checkpoint redo location used
-	 * as a starting point for an online backup.
+	 * runningBackups is a counter indicating the number of backups currently in
+	 * progress. forcePageWrites is set to true when either of these is
+	 * non-zero. lastBackupStart is the latest checkpoint redo location used as
+	 * a starting point for an online backup.
 	 */
-	ExclusiveBackupState exclusiveBackupState;
-	int			nonExclusiveBackups;
+	int			runningBackups;
 	XLogRecPtr	lastBackupStart;
 
 	/*
@@ -696,7 +670,6 @@ static void UpdateControlFile(void);
 static char *str_time(pg_time_t tnow);
 
 static void pg_start_backup_callback(int code, Datum arg);
-static void pg_stop_backup_callback(int code, Datum arg);
 
 static int	get_sync_bit(int method);
 
@@ -7006,7 +6979,7 @@ CreateRestartPoint(int flags)
 		 * Ensure minRecoveryPoint is past the checkpoint record.  Normally,
 		 * this will have happened already while writing out dirty buffers,
 		 * but not necessarily - e.g. because no buffers were dirtied.  We do
-		 * this because a non-exclusive base backup uses minRecoveryPoint to
+		 * this because a backup performed in recovery uses minRecoveryPoint to
 		 * determine which WAL files must be included in the backup, and the
 		 * file (or files) containing the checkpoint record must be included,
 		 * at a minimum. Note that for an ordinary restart of recovery there's
@@ -8009,32 +7982,14 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 }
 
 /*
- * do_pg_start_backup
- *
- * Utility function called at the start of an online backup. It creates the
- * necessary starting checkpoint and constructs the backup label file.
- *
- * There are two kind of backups: exclusive and non-exclusive. An exclusive
- * backup is started with pg_start_backup(), and there can be only one active
- * at a time. The backup and tablespace map files of an exclusive backup are
- * written to $PGDATA/backup_label and $PGDATA/tablespace_map, and they are
- * removed by pg_stop_backup().
+ * do_pg_start_backup is the workhorse of the user-visible pg_start_backup()
+ * function. It creates the necessary starting checkpoint and constructs the
+ * backup label and tablespace map.
  *
- * A non-exclusive backup is used for the streaming base backups (see
- * src/backend/replication/basebackup.c). The difference to exclusive backups
- * is that the backup label and tablespace map files are not written to disk.
- * Instead, their would-be contents are returned in *labelfile and *tblspcmapfile,
- * and the caller is responsible for including them in the backup archive as
- * 'backup_label' and 'tablespace_map'. There can be many non-exclusive backups
- * active at the same time, and they don't conflict with an exclusive backup
- * either.
- *
- * labelfile and tblspcmapfile must be passed as NULL when starting an
- * exclusive backup, and as initially-empty StringInfos for a non-exclusive
- * backup.
- *
- * If "tablespaces" isn't NULL, it receives a list of tablespaceinfo structs
- * describing the cluster's tablespaces.
+ * The backup label and tablespace map contents are returned in *labelfile and
+ * *tblspcmapfile, and the caller is responsible for including them in the
+ * backup archive as 'backup_label' and 'tablespace_map'. There can be many
+ * backups active at the same time.
  *
  * tblspcmapfile is required mainly for tar format in windows as native windows
  * utilities are not able to create symlinks while extracting files from tar.
@@ -8043,7 +7998,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
  * Returns the minimum WAL location that must be present to restore from this
  * backup, and the corresponding timeline ID in *starttli_p.
  *
- * Every successfully started non-exclusive backup must be stopped by calling
+ * Every successfully started backup must be stopped by calling
  * do_pg_stop_backup() or do_pg_abort_backup().
  *
  * It is the responsibility of the caller of this function to verify the
@@ -8054,7 +8009,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 				   StringInfo labelfile, List **tablespaces,
 				   StringInfo tblspcmapfile)
 {
-	bool		exclusive = (labelfile == NULL);
 	bool		backup_started_in_recovery = false;
 	XLogRecPtr	checkpointloc;
 	XLogRecPtr	startpoint;
@@ -8063,20 +8017,9 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 	char		strfbuf[128];
 	char		xlogfilename[MAXFNAMELEN];
 	XLogSegNo	_logSegNo;
-	struct stat stat_buf;
-	FILE	   *fp;
 
 	backup_started_in_recovery = RecoveryInProgress();
 
-	/*
-	 * Currently only non-exclusive backup can be taken during recovery.
-	 */
-	if (backup_started_in_recovery && exclusive)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("recovery is in progress"),
-				 errhint("WAL control functions cannot be executed during recovery.")));
-
 	/*
 	 * During recovery, we don't need to check WAL level. Because, if WAL
 	 * level is not sufficient, it's impossible to get here during recovery.
@@ -8115,30 +8058,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 	 * XLogInsertRecord().
 	 */
 	WALInsertLockAcquireExclusive();
-	if (exclusive)
-	{
-		/*
-		 * At first, mark that we're now starting an exclusive backup, to
-		 * ensure that there are no other sessions currently running
-		 * pg_start_backup() or pg_stop_backup().
-		 */
-		if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_NONE)
-		{
-			WALInsertLockRelease();
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("a backup is already in progress"),
-					 errhint("Run pg_stop_backup() and try again.")));
-		}
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
-	}
-	else
-		XLogCtl->Insert.nonExclusiveBackups++;
+	XLogCtl->Insert.runningBackups++;
 	XLogCtl->Insert.forcePageWrites = true;
 	WALInsertLockRelease();
 
 	/* Ensure we release forcePageWrites if fail below */
-	PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
+	PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0);
 	{
 		bool		gotUniqueStartpoint = false;
 		DIR		   *tblspcdir;
@@ -8373,122 +8298,19 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 						 LSN_FORMAT_ARGS(startpoint), xlogfilename);
 		appendStringInfo(labelfile, "CHECKPOINT LOCATION: %X/%X\n",
 						 LSN_FORMAT_ARGS(checkpointloc));
-		appendStringInfo(labelfile, "BACKUP METHOD: %s\n",
-						 exclusive ? "pg_start_backup" : "streamed");
+		appendStringInfo(labelfile, "BACKUP METHOD: streamed\n");
 		appendStringInfo(labelfile, "BACKUP FROM: %s\n",
 						 backup_started_in_recovery ? "standby" : "primary");
 		appendStringInfo(labelfile, "START TIME: %s\n", strfbuf);
 		appendStringInfo(labelfile, "LABEL: %s\n", backupidstr);
 		appendStringInfo(labelfile, "START TIMELINE: %u\n", starttli);
-
-		/*
-		 * Okay, write the file, or return its contents to caller.
-		 */
-		if (exclusive)
-		{
-			/*
-			 * Check for existing backup label --- implies a backup is already
-			 * running.  (XXX given that we checked exclusiveBackupState
-			 * above, maybe it would be OK to just unlink any such label
-			 * file?)
-			 */
-			if (stat(BACKUP_LABEL_FILE, &stat_buf) != 0)
-			{
-				if (errno != ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not stat file \"%s\": %m",
-									BACKUP_LABEL_FILE)));
-			}
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("a backup is already in progress"),
-						 errhint("If you're sure there is no backup in progress, remove file \"%s\" and try again.",
-								 BACKUP_LABEL_FILE)));
-
-			fp = AllocateFile(BACKUP_LABEL_FILE, "w");
-
-			if (!fp)
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not create file \"%s\": %m",
-								BACKUP_LABEL_FILE)));
-			if (fwrite(labelfile->data, labelfile->len, 1, fp) != 1 ||
-				fflush(fp) != 0 ||
-				pg_fsync(fileno(fp)) != 0 ||
-				ferror(fp) ||
-				FreeFile(fp))
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not write file \"%s\": %m",
-								BACKUP_LABEL_FILE)));
-			/* Allocated locally for exclusive backups, so free separately */
-			pfree(labelfile->data);
-			pfree(labelfile);
-
-			/* Write backup tablespace_map file. */
-			if (tblspcmapfile->len > 0)
-			{
-				if (stat(TABLESPACE_MAP, &stat_buf) != 0)
-				{
-					if (errno != ENOENT)
-						ereport(ERROR,
-								(errcode_for_file_access(),
-								 errmsg("could not stat file \"%s\": %m",
-										TABLESPACE_MAP)));
-				}
-				else
-					ereport(ERROR,
-							(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-							 errmsg("a backup is already in progress"),
-							 errhint("If you're sure there is no backup in progress, remove file \"%s\" and try again.",
-									 TABLESPACE_MAP)));
-
-				fp = AllocateFile(TABLESPACE_MAP, "w");
-
-				if (!fp)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not create file \"%s\": %m",
-									TABLESPACE_MAP)));
-				if (fwrite(tblspcmapfile->data, tblspcmapfile->len, 1, fp) != 1 ||
-					fflush(fp) != 0 ||
-					pg_fsync(fileno(fp)) != 0 ||
-					ferror(fp) ||
-					FreeFile(fp))
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not write file \"%s\": %m",
-									TABLESPACE_MAP)));
-			}
-
-			/* Allocated locally for exclusive backups, so free separately */
-			pfree(tblspcmapfile->data);
-			pfree(tblspcmapfile);
-		}
 	}
-	PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
+	PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0);
 
 	/*
-	 * Mark that start phase has correctly finished for an exclusive backup.
-	 * Session-level locks are updated as well to reflect that state.
-	 *
-	 * Note that CHECK_FOR_INTERRUPTS() must not occur while updating backup
-	 * counters and session-level lock. Otherwise they can be updated
-	 * inconsistently, and which might cause do_pg_abort_backup() to fail.
+	 * Mark that the start phase has correctly finished for the backup.
 	 */
-	if (exclusive)
-	{
-		WALInsertLockAcquireExclusive();
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
-
-		/* Set session-level lock */
-		sessionBackupState = SESSION_BACKUP_EXCLUSIVE;
-		WALInsertLockRelease();
-	}
-	else
-		sessionBackupState = SESSION_BACKUP_NON_EXCLUSIVE;
+	sessionBackupState = SESSION_BACKUP_RUNNING;
 
 	/*
 	 * We're done.  As a convenience, return the starting WAL location.
@@ -8502,43 +8324,15 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 static void
 pg_start_backup_callback(int code, Datum arg)
 {
-	bool		exclusive = DatumGetBool(arg);
-
 	/* Update backup counters and forcePageWrites on failure */
 	WALInsertLockAcquireExclusive();
-	if (exclusive)
-	{
-		Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
-	}
-	else
-	{
-		Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
-		XLogCtl->Insert.nonExclusiveBackups--;
-	}
 
-	if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
-		XLogCtl->Insert.nonExclusiveBackups == 0)
-	{
-		XLogCtl->Insert.forcePageWrites = false;
-	}
-	WALInsertLockRelease();
-}
-
-/*
- * Error cleanup callback for pg_stop_backup
- */
-static void
-pg_stop_backup_callback(int code, Datum arg)
-{
-	bool		exclusive = DatumGetBool(arg);
+	Assert(XLogCtl->Insert.runningBackups > 0);
+	XLogCtl->Insert.runningBackups--;
 
-	/* Update backup status on failure */
-	WALInsertLockAcquireExclusive();
-	if (exclusive)
+	if (XLogCtl->Insert.runningBackups == 0)
 	{
-		Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+		XLogCtl->Insert.forcePageWrites = false;
 	}
 	WALInsertLockRelease();
 }
@@ -8558,9 +8352,6 @@ get_backup_status(void)
  * Utility function called at the end of an online backup. It cleans up the
  * backup state and can optionally wait for WAL segments to be archived.
  *
- * If labelfile is NULL, this stops an exclusive backup. Otherwise this stops
- * the non-exclusive backup specified by 'labelfile'.
- *
  * Returns the last WAL location that must be present to restore from this
  * backup, and the corresponding timeline ID in *stoptli_p.
  *
@@ -8570,7 +8361,6 @@ get_backup_status(void)
 XLogRecPtr
 do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 {
-	bool		exclusive = (labelfile == NULL);
 	bool		backup_started_in_recovery = false;
 	XLogRecPtr	startpoint;
 	XLogRecPtr	stoppoint;
@@ -8584,7 +8374,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 	char		histfilename[MAXFNAMELEN];
 	char		backupfrom[20];
 	XLogSegNo	_logSegNo;
-	FILE	   *lfp;
 	FILE	   *fp;
 	char		ch;
 	int			seconds_before_warning;
@@ -8597,15 +8386,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 
 	backup_started_in_recovery = RecoveryInProgress();
 
-	/*
-	 * Currently only non-exclusive backup can be taken during recovery.
-	 */
-	if (backup_started_in_recovery && exclusive)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("recovery is in progress"),
-				 errhint("WAL control functions cannot be executed during recovery.")));
-
 	/*
 	 * During recovery, we don't need to check WAL level. Because, if WAL
 	 * level is not sufficient, it's impossible to get here during recovery.
@@ -8616,106 +8396,23 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 				 errmsg("WAL level not sufficient for making an online backup"),
 				 errhint("wal_level must be set to \"replica\" or \"logical\" at server start.")));
 
-	if (exclusive)
-	{
-		/*
-		 * At first, mark that we're now stopping an exclusive backup, to
-		 * ensure that there are no other sessions currently running
-		 * pg_start_backup() or pg_stop_backup().
-		 */
-		WALInsertLockAcquireExclusive();
-		if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_IN_PROGRESS)
-		{
-			WALInsertLockRelease();
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("exclusive backup not in progress")));
-		}
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STOPPING;
-		WALInsertLockRelease();
-
-		/*
-		 * Remove backup_label. In case of failure, the state for an exclusive
-		 * backup is switched back to in-progress.
-		 */
-		PG_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
-		{
-			/*
-			 * Read the existing label file into memory.
-			 */
-			struct stat statbuf;
-			int			r;
-
-			if (stat(BACKUP_LABEL_FILE, &statbuf))
-			{
-				/* should not happen per the upper checks */
-				if (errno != ENOENT)
-					ereport(ERROR,
-							(errcode_for_file_access(),
-							 errmsg("could not stat file \"%s\": %m",
-									BACKUP_LABEL_FILE)));
-				ereport(ERROR,
-						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-						 errmsg("a backup is not in progress")));
-			}
-
-			lfp = AllocateFile(BACKUP_LABEL_FILE, "r");
-			if (!lfp)
-			{
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not read file \"%s\": %m",
-								BACKUP_LABEL_FILE)));
-			}
-			labelfile = palloc(statbuf.st_size + 1);
-			r = fread(labelfile, statbuf.st_size, 1, lfp);
-			labelfile[statbuf.st_size] = '\0';
-
-			/*
-			 * Close and remove the backup label file
-			 */
-			if (r != 1 || ferror(lfp) || FreeFile(lfp))
-				ereport(ERROR,
-						(errcode_for_file_access(),
-						 errmsg("could not read file \"%s\": %m",
-								BACKUP_LABEL_FILE)));
-			durable_unlink(BACKUP_LABEL_FILE, ERROR);
-
-			/*
-			 * Remove tablespace_map file if present, it is created only if
-			 * there are tablespaces.
-			 */
-			durable_unlink(TABLESPACE_MAP, DEBUG1);
-		}
-		PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
-	}
-
 	/*
-	 * OK to update backup counters, forcePageWrites and session-level lock.
+	 * OK to update backup counters and forcePageWrites.
 	 *
 	 * Note that CHECK_FOR_INTERRUPTS() must not occur while updating them.
 	 * Otherwise they can be updated inconsistently, and which might cause
 	 * do_pg_abort_backup() to fail.
 	 */
 	WALInsertLockAcquireExclusive();
-	if (exclusive)
-	{
-		XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
-	}
-	else
-	{
-		/*
-		 * The user-visible pg_start/stop_backup() functions that operate on
-		 * exclusive backups can be called at any time, but for non-exclusive
-		 * backups, it is expected that each do_pg_start_backup() call is
-		 * matched by exactly one do_pg_stop_backup() call.
-		 */
-		Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
-		XLogCtl->Insert.nonExclusiveBackups--;
-	}
 
-	if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
-		XLogCtl->Insert.nonExclusiveBackups == 0)
+	/*
+	 * It is expected that each do_pg_start_backup() call is matched by exactly
+	 * one do_pg_stop_backup() call.
+	 */
+	Assert(XLogCtl->Insert.runningBackups > 0);
+	XLogCtl->Insert.runningBackups--;
+
+	if (XLogCtl->Insert.runningBackups == 0)
 	{
 		XLogCtl->Insert.forcePageWrites = false;
 	}
@@ -8980,10 +8677,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
  * The caller can pass 'arg' as 'true' or 'false' to control whether a warning
  * is emitted.
  *
- * NB: This is only for aborting a non-exclusive backup that doesn't write
- * backup_label. A backup started with pg_start_backup() needs to be finished
- * with pg_stop_backup().
- *
  * NB: This gets used as a before_shmem_exit handler, hence the odd-looking
  * signature.
  */
@@ -8993,18 +8686,16 @@ do_pg_abort_backup(int code, Datum arg)
 	bool		emit_warning = DatumGetBool(arg);
 
 	/*
-	 * Quick exit if session is not keeping around a non-exclusive backup
-	 * already started.
+	 * Quick exit if session does not have a running backup.
 	 */
-	if (sessionBackupState != SESSION_BACKUP_NON_EXCLUSIVE)
+	if (sessionBackupState != SESSION_BACKUP_RUNNING)
 		return;
 
 	WALInsertLockAcquireExclusive();
-	Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
-	XLogCtl->Insert.nonExclusiveBackups--;
+	Assert(XLogCtl->Insert.runningBackups > 0);
+	XLogCtl->Insert.runningBackups--;
 
-	if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
-		XLogCtl->Insert.nonExclusiveBackups == 0)
+	if (XLogCtl->Insert.runningBackups == 0)
 	{
 		XLogCtl->Insert.forcePageWrites = false;
 	}
@@ -9072,87 +8763,6 @@ GetOldestRestartPoint(XLogRecPtr *oldrecptr, TimeLineID *oldtli)
 	LWLockRelease(ControlFileLock);
 }
 
-/*
- * BackupInProgress: check if online backup mode is active
- *
- * This is done by checking for existence of the "backup_label" file.
- */
-bool
-BackupInProgress(void)
-{
-	struct stat stat_buf;
-
-	return (stat(BACKUP_LABEL_FILE, &stat_buf) == 0);
-}
-
-/*
- * CancelBackup: rename the "backup_label" and "tablespace_map"
- *				 files to cancel backup mode
- *
- * If the "backup_label" file exists, it will be renamed to "backup_label.old".
- * Similarly, if the "tablespace_map" file exists, it will be renamed to
- * "tablespace_map.old".
- *
- * Note that this will render an online backup in progress
- * useless. To correctly finish an online backup, pg_stop_backup must be
- * called.
- */
-void
-CancelBackup(void)
-{
-	struct stat stat_buf;
-
-	/* if the backup_label file is not there, return */
-	if (stat(BACKUP_LABEL_FILE, &stat_buf) < 0)
-		return;
-
-	/* remove leftover file from previously canceled backup if it exists */
-	unlink(BACKUP_LABEL_OLD);
-
-	if (durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, DEBUG1) != 0)
-	{
-		ereport(WARNING,
-				(errcode_for_file_access(),
-				 errmsg("online backup mode was not canceled"),
-				 errdetail("File \"%s\" could not be renamed to \"%s\": %m.",
-						   BACKUP_LABEL_FILE, BACKUP_LABEL_OLD)));
-		return;
-	}
-
-	/* if the tablespace_map file is not there, return */
-	if (stat(TABLESPACE_MAP, &stat_buf) < 0)
-	{
-		ereport(LOG,
-				(errmsg("online backup mode canceled"),
-				 errdetail("File \"%s\" was renamed to \"%s\".",
-						   BACKUP_LABEL_FILE, BACKUP_LABEL_OLD)));
-		return;
-	}
-
-	/* remove leftover file from previously canceled backup if it exists */
-	unlink(TABLESPACE_MAP_OLD);
-
-	if (durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, DEBUG1) == 0)
-	{
-		ereport(LOG,
-				(errmsg("online backup mode canceled"),
-				 errdetail("Files \"%s\" and \"%s\" were renamed to "
-						   "\"%s\" and \"%s\", respectively.",
-						   BACKUP_LABEL_FILE, TABLESPACE_MAP,
-						   BACKUP_LABEL_OLD, TABLESPACE_MAP_OLD)));
-	}
-	else
-	{
-		ereport(WARNING,
-				(errcode_for_file_access(),
-				 errmsg("online backup mode canceled"),
-				 errdetail("File \"%s\" was renamed to \"%s\", but "
-						   "file \"%s\" could not be renamed to \"%s\": %m.",
-						   BACKUP_LABEL_FILE, BACKUP_LABEL_OLD,
-						   TABLESPACE_MAP, TABLESPACE_MAP_OLD)));
-	}
-}
-
 /* Thin wrapper around ShutdownWalRcv(). */
 void
 XLogShutdownWalRcv(void)
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 12e2bf4135..148b506a22 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -39,7 +39,7 @@
 #include "utils/tuplestore.h"
 
 /*
- * Store label file and tablespace map during non-exclusive backups.
+ * Store label file and tablespace map during backups.
  */
 static StringInfo label_file;
 static StringInfo tblspc_map_file;
@@ -61,101 +61,40 @@ pg_start_backup(PG_FUNCTION_ARGS)
 {
 	text	   *backupid = PG_GETARG_TEXT_PP(0);
 	bool		fast = PG_GETARG_BOOL(1);
-	bool		exclusive = PG_GETARG_BOOL(2);
 	char	   *backupidstr;
 	XLogRecPtr	startpoint;
 	SessionBackupState status = get_backup_status();
+	MemoryContext oldcontext;
 
 	backupidstr = text_to_cstring(backupid);
 
-	if (status == SESSION_BACKUP_NON_EXCLUSIVE)
+	if (status == SESSION_BACKUP_RUNNING)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("a backup is already in progress in this session")));
 
-	if (exclusive)
-	{
-		startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
-										NULL, NULL);
-	}
-	else
-	{
-		MemoryContext oldcontext;
-
-		/*
-		 * Label file and tablespace map file need to be long-lived, since
-		 * they are read in pg_stop_backup.
-		 */
-		oldcontext = MemoryContextSwitchTo(TopMemoryContext);
-		label_file = makeStringInfo();
-		tblspc_map_file = makeStringInfo();
-		MemoryContextSwitchTo(oldcontext);
+	/*
+	 * Label file and tablespace map file need to be long-lived, since
+	 * they are read in pg_stop_backup.
+	 */
+	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+	label_file = makeStringInfo();
+	tblspc_map_file = makeStringInfo();
+	MemoryContextSwitchTo(oldcontext);
 
-		register_persistent_abort_backup_handler();
+	register_persistent_abort_backup_handler();
 
-		startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
-										NULL, tblspc_map_file);
-	}
+	startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
+									NULL, tblspc_map_file);
 
 	PG_RETURN_LSN(startpoint);
 }
 
-/*
- * pg_stop_backup: finish taking an on-line backup dump
- *
- * We write an end-of-backup WAL record, and remove the backup label file
- * created by pg_start_backup, creating a backup history file in pg_wal
- * instead (whence it will immediately be archived). The backup history file
- * contains the same info found in the label file, plus the backup-end time
- * and WAL location. Before 9.0, the backup-end time was read from the backup
- * history file at the beginning of archive recovery, but we now use the WAL
- * record for that and the file is for informational and debug purposes only.
- *
- * Note: different from CancelBackup which just cancels online backup mode.
- *
- * Note: this version is only called to stop an exclusive backup. The function
- *		 pg_stop_backup_v2 (overloaded as pg_stop_backup in SQL) is called to
- *		 stop non-exclusive backups.
- *
- * Permission checking for this function is managed through the normal
- * GRANT system.
- */
-Datum
-pg_stop_backup(PG_FUNCTION_ARGS)
-{
-	XLogRecPtr	stoppoint;
-	SessionBackupState status = get_backup_status();
-
-	if (status == SESSION_BACKUP_NON_EXCLUSIVE)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("non-exclusive backup in progress"),
-				 errhint("Did you mean to use pg_stop_backup('f')?")));
-
-	/*
-	 * Exclusive backups were typically started in a different connection, so
-	 * don't try to verify that status of backup is set to
-	 * SESSION_BACKUP_EXCLUSIVE in this function. Actual verification that an
-	 * exclusive backup is in fact running is handled inside
-	 * do_pg_stop_backup.
-	 */
-	stoppoint = do_pg_stop_backup(NULL, true, NULL);
-
-	PG_RETURN_LSN(stoppoint);
-}
-
 
 /*
- * pg_stop_backup_v2: finish taking exclusive or nonexclusive on-line backup.
- *
- * Works the same as pg_stop_backup, except for non-exclusive backups it returns
- * the backup label and tablespace map files as text fields in as part of the
- * resultset.
+ * pg_stop_backup: finish taking an on-line backup.
  *
- * The first parameter (variable 'exclusive') allows the user to tell us if
- * this is an exclusive or a non-exclusive backup.
- *
- * The second parameter (variable 'waitforarchive'), which is optional,
+ * The first parameter (variable 'waitforarchive'), which is optional,
  * allows the user to choose if they want to wait for the WAL to be archived
  * or if we should just return as soon as the WAL record is written.
  *
@@ -163,7 +102,7 @@ pg_stop_backup(PG_FUNCTION_ARGS)
  * GRANT system.
  */
 Datum
-pg_stop_backup_v2(PG_FUNCTION_ARGS)
+pg_stop_backup(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	TupleDesc	tupdesc;
@@ -173,8 +112,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
 	Datum		values[3];
 	bool		nulls[3];
 
-	bool		exclusive = PG_GETARG_BOOL(0);
-	bool		waitforarchive = PG_GETARG_BOOL(1);
+	bool		waitforarchive = PG_GETARG_BOOL(0);
 	XLogRecPtr	stoppoint;
 	SessionBackupState status = get_backup_status();
 
@@ -205,51 +143,29 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
 	MemSet(values, 0, sizeof(values));
 	MemSet(nulls, 0, sizeof(nulls));
 
-	if (exclusive)
-	{
-		if (status == SESSION_BACKUP_NON_EXCLUSIVE)
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("non-exclusive backup in progress"),
-					 errhint("Did you mean to use pg_stop_backup('f')?")));
-
-		/*
-		 * Stop the exclusive backup, and since we're in an exclusive backup
-		 * return NULL for both backup_label and tablespace_map.
-		 */
-		stoppoint = do_pg_stop_backup(NULL, waitforarchive, NULL);
-
-		nulls[1] = true;
-		nulls[2] = true;
-	}
-	else
-	{
-		if (status != SESSION_BACKUP_NON_EXCLUSIVE)
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("non-exclusive backup is not in progress"),
-					 errhint("Did you mean to use pg_stop_backup('t')?")));
+	if (status != SESSION_BACKUP_RUNNING)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("backup is not in progress"),
+				 errhint("Did you call pg_start_backup()?")));
 
-		/*
-		 * Stop the non-exclusive backup. Return a copy of the backup label
-		 * and tablespace map so they can be written to disk by the caller.
-		 */
-		stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
-
-		values[1] = CStringGetTextDatum(label_file->data);
-		values[2] = CStringGetTextDatum(tblspc_map_file->data);
-
-		/* Free structures allocated in TopMemoryContext */
-		pfree(label_file->data);
-		pfree(label_file);
-		label_file = NULL;
-		pfree(tblspc_map_file->data);
-		pfree(tblspc_map_file);
-		tblspc_map_file = NULL;
-	}
+	/*
+	 * Stop the backup. Return a copy of the backup label and tablespace map so
+	 * they can be written to disk by the caller.
+	 */
+	stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
 
-	/* Stoppoint is included on both exclusive and nonexclusive backups */
 	values[0] = LSNGetDatum(stoppoint);
+	values[1] = CStringGetTextDatum(label_file->data);
+	values[2] = CStringGetTextDatum(tblspc_map_file->data);
+
+	/* Free structures allocated in TopMemoryContext */
+	pfree(label_file->data);
+	pfree(label_file);
+	label_file = NULL;
+	pfree(tblspc_map_file->data);
+	pfree(tblspc_map_file);
+	tblspc_map_file = NULL;
 
 	tuplestore_putvalues(tupstore, tupdesc, values, nulls);
 
@@ -669,81 +585,6 @@ pg_wal_lsn_diff(PG_FUNCTION_ARGS)
 	PG_RETURN_NUMERIC(result);
 }
 
-/*
- * Returns bool with current on-line backup mode, a global state.
- */
-Datum
-pg_is_in_backup(PG_FUNCTION_ARGS)
-{
-	PG_RETURN_BOOL(BackupInProgress());
-}
-
-/*
- * Returns start time of an online exclusive backup.
- *
- * When there's no exclusive backup in progress, the function
- * returns NULL.
- */
-Datum
-pg_backup_start_time(PG_FUNCTION_ARGS)
-{
-	Datum		xtime;
-	FILE	   *lfp;
-	char		fline[MAXPGPATH];
-	char		backup_start_time[30];
-
-	/*
-	 * See if label file is present
-	 */
-	lfp = AllocateFile(BACKUP_LABEL_FILE, "r");
-	if (lfp == NULL)
-	{
-		if (errno != ENOENT)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not read file \"%s\": %m",
-							BACKUP_LABEL_FILE)));
-		PG_RETURN_NULL();
-	}
-
-	/*
-	 * Parse the file to find the START TIME line.
-	 */
-	backup_start_time[0] = '\0';
-	while (fgets(fline, sizeof(fline), lfp) != NULL)
-	{
-		if (sscanf(fline, "START TIME: %25[^\n]\n", backup_start_time) == 1)
-			break;
-	}
-
-	/* Check for a read error. */
-	if (ferror(lfp))
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not read file \"%s\": %m", BACKUP_LABEL_FILE)));
-
-	/* Close the backup label file. */
-	if (FreeFile(lfp))
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not close file \"%s\": %m", BACKUP_LABEL_FILE)));
-
-	if (strlen(backup_start_time) == 0)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
-
-	/*
-	 * Convert the time string read from file to TimestampTz form.
-	 */
-	xtime = DirectFunctionCall3(timestamptz_in,
-								CStringGetDatum(backup_start_time),
-								ObjectIdGetDatum(InvalidOid),
-								Int32GetDatum(-1));
-
-	PG_RETURN_DATUM(xtime);
-}
-
 /*
  * Promotes a standby server.
  *
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index fd1421788e..111eff6e14 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -377,14 +377,14 @@ BEGIN ATOMIC
 END;
 
 CREATE OR REPLACE FUNCTION
-  pg_start_backup(label text, fast boolean DEFAULT false, exclusive boolean DEFAULT true)
+  pg_start_backup(label text, fast boolean DEFAULT false)
   RETURNS pg_lsn STRICT VOLATILE LANGUAGE internal AS 'pg_start_backup'
   PARALLEL RESTRICTED;
 
 CREATE OR REPLACE FUNCTION pg_stop_backup (
-        exclusive boolean, wait_for_archive boolean DEFAULT true,
-        OUT lsn pg_lsn, OUT labelfile text, OUT spcmapfile text)
-  RETURNS SETOF record STRICT VOLATILE LANGUAGE internal as 'pg_stop_backup_v2'
+        wait_for_archive boolean DEFAULT true, OUT lsn pg_lsn,
+        OUT labelfile text, OUT spcmapfile text)
+  RETURNS SETOF record STRICT VOLATILE LANGUAGE internal as 'pg_stop_backup'
   PARALLEL RESTRICTED;
 
 CREATE OR REPLACE FUNCTION
@@ -603,11 +603,9 @@ AS 'unicode_is_normalized';
 -- available to superuser / cluster owner, if they choose.
 --
 
-REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean) FROM public;
 
-REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
-
-REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean, boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_create_restore_point(text) FROM public;
 
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 735fed490b..12d8ef9bfe 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -348,7 +348,6 @@ static PMState pmState = PM_INIT;
 typedef enum
 {
 	ALLOW_ALL_CONNS,			/* normal not-shutting-down state */
-	ALLOW_SUPERUSER_CONNS,		/* only superusers can connect */
 	ALLOW_NO_CONNS				/* no new connections allowed, period */
 } ConnsAllowedState;
 
@@ -2546,19 +2545,11 @@ canAcceptConnections(int backend_type)
 
 	/*
 	 * "Smart shutdown" restrictions are applied only to normal connections,
-	 * not to autovac workers or bgworkers.  When only superusers can connect,
-	 * we return CAC_SUPERUSER to indicate that superuserness must be checked
-	 * later.  Note that neither CAC_OK nor CAC_SUPERUSER can safely be
-	 * returned until we have checked for too many children.
+	 * not to autovac workers or bgworkers.
 	 */
 	if (connsAllowed != ALLOW_ALL_CONNS &&
 		backend_type == BACKEND_TYPE_NORMAL)
-	{
-		if (connsAllowed == ALLOW_SUPERUSER_CONNS)
-			result = CAC_SUPERUSER; /* allow superusers only */
-		else
-			return CAC_SHUTDOWN;	/* shutdown is pending */
-	}
+		return CAC_SHUTDOWN;	/* shutdown is pending */
 
 	/*
 	 * Don't start too many children.
@@ -2877,16 +2868,11 @@ pmdie(SIGNAL_ARGS)
 #endif
 
 			/*
-			 * If we reached normal running, we have to wait for any online
-			 * backup mode to end; otherwise go straight to waiting for client
-			 * backends to exit.  (The difference is that in the former state,
-			 * we'll still let in new superuser clients, so that somebody can
-			 * end the online backup mode.)  If already in PM_STOP_BACKENDS or
+			 * If we reached normal running, we go straight to waiting for
+			 * client backends to exit.  If already in PM_STOP_BACKENDS or
 			 * a later state, do not change it.
 			 */
-			if (pmState == PM_RUN)
-				connsAllowed = ALLOW_SUPERUSER_CONNS;
-			else if (pmState == PM_HOT_STANDBY)
+			if (pmState == PM_RUN || pmState == PM_HOT_STANDBY)
 				connsAllowed = ALLOW_NO_CONNS;
 			else if (pmState == PM_STARTUP || pmState == PM_RECOVERY)
 			{
@@ -3842,16 +3828,6 @@ PostmasterStateMachine(void)
 	/* If we're doing a smart shutdown, try to advance that state. */
 	if (pmState == PM_RUN || pmState == PM_HOT_STANDBY)
 	{
-		if (connsAllowed == ALLOW_SUPERUSER_CONNS)
-		{
-			/*
-			 * ALLOW_SUPERUSER_CONNS state ends as soon as online backup mode
-			 * is not active.
-			 */
-			if (!BackupInProgress())
-				connsAllowed = ALLOW_NO_CONNS;
-		}
-
 		if (connsAllowed == ALLOW_NO_CONNS)
 		{
 			/*
@@ -4044,18 +4020,6 @@ PostmasterStateMachine(void)
 		}
 		else
 		{
-			/*
-			 * Terminate exclusive backup mode to avoid recovery after a clean
-			 * fast shutdown.  Since an exclusive backup can only be taken
-			 * during normal running (and not, for example, while running
-			 * under Hot Standby) it only makes sense to do this if we reached
-			 * normal running. If we're still in recovery, the backup file is
-			 * one we're recovering *from*, and we must keep it around so that
-			 * recovery restarts from the right place.
-			 */
-			if (ReachedNormalRunning)
-				CancelBackup();
-
 			/*
 			 * Normal exit from the postmaster is here.  We don't need to log
 			 * anything here, since the UnlinkLockFiles proc_exit callback
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 0bf28b55d7..4682b28a85 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -195,10 +195,8 @@ static const struct exclude_list_item excludeFiles[] =
 	{RELCACHE_INIT_FILENAME, true},
 
 	/*
-	 * If there's a backup_label or tablespace_map file, it belongs to a
-	 * backup started by the user with pg_start_backup().  It is *not* correct
-	 * for this backup.  Our backup_label/tablespace_map is injected into the
-	 * tar separately.
+	 * backup_label and tablespace_map should not exist in in a running cluster
+	 * capable of doing an online backup, but exclude then just in case.
 	 */
 	{BACKUP_LABEL_FILE, false},
 	{TABLESPACE_MAP, false},
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 8c70e5b32b..5019a51009 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -185,6 +185,10 @@ isnt(slurp_file("$tempdir/backup/backup_label"),
 	'DONOTCOPY', 'existing backup_label not copied');
 rmtree("$tempdir/backup");
 
+# Now delete the bogus backup_label file since it will interfere with startup
+unlink("$pgdata/backup_label")
+  or BAIL_OUT("unable to unlink $pgdata/backup_label");
+
 $node->command_ok(
 	[
 		@pg_basebackup_defs, '-D',
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 7211090f47..fb52debf7a 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -140,9 +140,9 @@ static const struct exclude_list_item excludeFiles[] =
 	{"pg_internal.init", true}, /* defined as RELCACHE_INIT_FILENAME */
 
 	/*
-	 * If there's a backup_label or tablespace_map file, it belongs to a
-	 * backup started by the user with pg_start_backup().  It is *not* correct
-	 * for this backup.  Our backup_label is written later on separately.
+	 * If there is a backup_label or tablespace_map file, it indicates that
+	 * a recovery failed and this cluster probably can't be rewound, but
+	 * exclude them anyway if they are found.
 	 */
 	{"backup_label", false},	/* defined as BACKUP_LABEL_FILE */
 	{"tablespace_map", false},	/* defined as TABLESPACE_MAP */
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 4b45ac64db..00a3d14c76 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -275,8 +275,7 @@ extern void XLogShutdownWalRcv(void);
 typedef enum SessionBackupState
 {
 	SESSION_BACKUP_NONE,
-	SESSION_BACKUP_EXCLUSIVE,
-	SESSION_BACKUP_NON_EXCLUSIVE
+	SESSION_BACKUP_RUNNING,
 } SessionBackupState;
 
 extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7f1ee97f55..8f126e2f54 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6275,25 +6275,15 @@
   prosrc => 'pg_terminate_backend' },
 { oid => '2172', descr => 'prepare for taking an online backup',
   proname => 'pg_start_backup', provolatile => 'v', proparallel => 'r',
-  prorettype => 'pg_lsn', proargtypes => 'text bool bool',
+  prorettype => 'pg_lsn', proargtypes => 'text bool',
   prosrc => 'pg_start_backup' },
-{ oid => '2173', descr => 'finish taking an online backup',
-  proname => 'pg_stop_backup', provolatile => 'v', proparallel => 'r',
-  prorettype => 'pg_lsn', proargtypes => '', prosrc => 'pg_stop_backup' },
 { oid => '2739', descr => 'finish taking an online backup',
   proname => 'pg_stop_backup', prorows => '1', proretset => 't',
   provolatile => 'v', proparallel => 'r', prorettype => 'record',
-  proargtypes => 'bool bool', proallargtypes => '{bool,bool,pg_lsn,text,text}',
-  proargmodes => '{i,i,o,o,o}',
-  proargnames => '{exclusive,wait_for_archive,lsn,labelfile,spcmapfile}',
-  prosrc => 'pg_stop_backup_v2' },
-{ oid => '3813', descr => 'true if server is in online backup',
-  proname => 'pg_is_in_backup', provolatile => 'v', prorettype => 'bool',
-  proargtypes => '', prosrc => 'pg_is_in_backup' },
-{ oid => '3814', descr => 'start time of an online backup',
-  proname => 'pg_backup_start_time', provolatile => 's',
-  prorettype => 'timestamptz', proargtypes => '',
-  prosrc => 'pg_backup_start_time' },
+  proargtypes => 'bool', proallargtypes => '{bool,pg_lsn,text,text}',
+  proargmodes => '{i,o,o,o}',
+  proargnames => '{wait_for_archive,lsn,labelfile,spcmapfile}',
+  prosrc => 'pg_stop_backup' },
 { oid => '3436', descr => 'promote standby server',
   proname => 'pg_promote', provolatile => 'v', prorettype => 'bool',
   proargtypes => 'bool int4', proargnames => '{wait,wait_seconds}',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0abc3ad540..9321d7f264 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -481,10 +481,6 @@ extern void process_session_preload_libraries(void);
 extern void pg_bindtextdomain(const char *domain);
 extern bool has_rolreplication(Oid roleid);
 
-/* in access/transam/xlog.c */
-extern bool BackupInProgress(void);
-extern void CancelBackup(void);
-
 /* in executor/nodeHash.c */
 extern size_t get_hash_memory_limit(void);
 
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index ed70eff374..49573f9d7d 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -634,25 +634,6 @@ sub backup
 	return;
 }
 
-=item $node->backup_fs_hot(backup_name)
-
-Create a backup with a filesystem level copy in subdirectory B<backup_name> of
-B<< $node->backup_dir >>, including WAL.
-
-Archiving must be enabled, as B<pg_start_backup()> and B<pg_stop_backup()> are
-used. This is not checked or enforced.
-
-The backup name is passed as the backup label to B<pg_start_backup()>.
-
-=cut
-
-sub backup_fs_hot
-{
-	my ($self, $backup_name) = @_;
-	$self->_backup_fs($backup_name, 1);
-	return;
-}
-
 =item $node->backup_fs_cold(backup_name)
 
 Create a backup with a filesystem level copy in subdirectory B<backup_name> of
@@ -666,53 +647,18 @@ Use B<backup> or B<backup_fs_hot> if you want to back up a running server.
 sub backup_fs_cold
 {
 	my ($self, $backup_name) = @_;
-	$self->_backup_fs($backup_name, 0);
-	return;
-}
-
-
-# Common sub of backup_fs_hot and backup_fs_cold
-sub _backup_fs
-{
-	my ($self, $backup_name, $hot) = @_;
-	my $backup_path = $self->backup_dir . '/' . $backup_name;
-	my $port        = $self->port;
-	my $name        = $self->name;
-
-	print "# Taking filesystem backup $backup_name from node \"$name\"\n";
-
-	if ($hot)
-	{
-		my $stdout = $self->safe_psql('postgres',
-			"SELECT * FROM pg_start_backup('$backup_name');");
-		print "# pg_start_backup: $stdout\n";
-	}
 
 	PostgreSQL::Test::RecursiveCopy::copypath(
 		$self->data_dir,
-		$backup_path,
+		$self->backup_dir . '/' . $backup_name,
 		filterfn => sub {
 			my $src = shift;
 			return ($src ne 'log' and $src ne 'postmaster.pid');
 		});
 
-	if ($hot)
-	{
-
-		# We ignore pg_stop_backup's return value. We also assume archiving
-		# is enabled; otherwise the caller will have to copy the remaining
-		# segments.
-		my $stdout =
-		  $self->safe_psql('postgres', 'SELECT * FROM pg_stop_backup();');
-		print "# pg_stop_backup: $stdout\n";
-	}
-
-	print "# Backup finished\n";
 	return;
 }
 
-
-
 =pod
 
 =item $node->init_from_backup(root_node, backup_name)
diff --git a/src/test/recovery/t/010_logical_decoding_timelines.pl b/src/test/recovery/t/010_logical_decoding_timelines.pl
index 6e8b0b1b96..6351633b50 100644
--- a/src/test/recovery/t/010_logical_decoding_timelines.pl
+++ b/src/test/recovery/t/010_logical_decoding_timelines.pl
@@ -69,7 +69,9 @@ $node_primary->safe_psql('dropme',
 $node_primary->safe_psql('postgres', 'CHECKPOINT;');
 
 my $backup_name = 'b1';
-$node_primary->backup_fs_hot($backup_name);
+$node_primary->stop();
+$node_primary->backup_fs_cold($backup_name);
+$node_primary->start();
 
 $node_primary->safe_psql('postgres',
 	q[SELECT pg_create_physical_replication_slot('phys_slot');]);
-- 
2.25.1

---

From: Ashutosh Sharma <[email-masked]>
Date: Fri, 18 Feb 2022 22:48:10 +0530

Some review comments on the latest version:

What do you mean by "either of these is non-zero ''. Earlier we used
to set forcePageWrites in case of both exclusive and non-exclusive
backups, but we have just one type of backup now.

==

We still update the status of session-level lock so I don't think we
should update the above comment. See below code:

    if (XLogCtl->Insert.runningBackups == 0)
    {
        XLogCtl->Insert.forcePageWrites = false;
    }

    /*
     * Clean up session-level lock.
     *
     * You might think that WALInsertLockRelease() can be called before
     * cleaning up session-level lock because session-level lock doesn't need
     * to be protected with WAL insertion lock. But since
     * CHECK_FOR_INTERRUPTS() can occur in it, session-level lock must be
     * cleaned up before it.
     */
    sessionBackupState = SESSION_BACKUP_NONE;

    WALInsertLockRelease();

==

    /*

I think we have a lot of common code in do_pg_abort_backup() and
pg_do_stop_backup(). So why not have a common function that can be
called from both these functions.

==

Why do we need this additional change? Earlier this was not required.

--
With Regards,
Ashutosh Sharma.

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 18 Feb 2022 12:54:29 -0800

> Some review comments on the latest version:

Thanks for the feedback!  Before I start spending more time on this one, I
should probably ask if this has any chance of making it into v15.

---

From: Ashutosh Sharma <[email-masked]>
Date: Sat, 19 Feb 2022 09:37:40 +0530

> should probably ask if this has any chance of making it into v15.

I don't see any reason why it can't make it to v15. However, it is not
super urgent as the users facing this problem have a choice. They can
use non-exclusive mode.

--
With Regards,
Ashutosh Sharma.

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 21 Feb 2022 09:23:06 -0800

I've attached an updated patch.

> backups, but we have just one type of backup now.

Fixed this.

> should update the above comment. See below code:

Fixed this.

> called from both these functions.

I didn't follow through with this change.  I only saw a handful of lines
that looked similar, and AFAICT we'd need an extra branch for cleaning up
the session-level lock since do_pg_abort_backup() doesn't.

> Why do we need this additional change? Earlier this was not required.

IIUC this test relied on the following code to handle the bogus file:

			/*
			 * Terminate exclusive backup mode to avoid recovery after a clean
			 * fast shutdown.  Since an exclusive backup can only be taken
			 * during normal running (and not, for example, while running
			 * under Hot Standby) it only makes sense to do this if we reached
			 * normal running. If we're still in recovery, the backup file is
			 * one we're recovering *from*, and we must keep it around so that
			 * recovery restarts from the right place.
			 */
			if (ReachedNormalRunning)
				CancelBackup();

The attached patch removes this code.

>From 7fd092c90be90018eeddf3ea5088197627cbe593 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 1 Dec 2021 23:50:49 +0000
Subject: [PATCH v3 1/1] remove exclusive backup mode

---

-    <para>
index 0d2bd7a357..4f62f77d02 100644
 
-/*
 
 	/*
-	 * exclusiveBackupState indicates the state of an exclusive backup (see
-	 * comments of ExclusiveBackupState for more details). nonExclusiveBackups
-	 * is a counter indicating the number of streaming base backups currently
-	 * in progress. forcePageWrites is set to true when either of these is
-	 * non-zero. lastBackupStart is the latest checkpoint redo location used
-	 * as a starting point for an online backup.
+	 * runningBackups is a counter indicating the number of backups currently in
+	 * progress. forcePageWrites is set to true when runningBackups is non-zero.
+	 * lastBackupStart is the latest checkpoint redo location used as a starting
+	 * point for an online backup.
 	 */
-	ExclusiveBackupState exclusiveBackupState;
-	int			nonExclusiveBackups;
+	int			runningBackups;
 	XLogRecPtr	lastBackupStart;
 
 	/*

 
@@ -7008,7 +6981,7 @@ CreateRestartPoint(int flags)
@@ -8011,32 +7984,14 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 }
 
 /*
- * do_pg_start_backup
- *
@@ -8045,7 +8000,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
@@ -8056,7 +8011,6 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
@@ -8065,20 +8019,9 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,

-	/*
@@ -8117,30 +8060,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 
@@ -8375,122 +8300,19 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 
 	/*
 
 	/*
@@ -8504,43 +8326,15 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,

@@ -8560,9 +8354,6 @@ get_backup_status(void)
@@ -8572,7 +8363,6 @@ get_backup_status(void)
@@ -8586,7 +8376,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
@@ -8599,15 +8388,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)

-	/*
@@ -8618,106 +8398,23 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 
-	if (exclusive)
-	{
-		/*
+	 * OK to update backup counters, forcePageWrites, and session-level lock.
 
@@ -8982,10 +8679,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
@@ -8995,18 +8688,16 @@ do_pg_abort_backup(int code, Datum arg)
 
 	/*

@@ -9074,87 +8765,6 @@ GetOldestRestartPoint(XLogRecPtr *oldrecptr, TimeLineID *oldtli)
 
-/*
 
 /*

 
-	if (exclusive)
-	{

 
 
-/*
 
 /*

 
-	if (exclusive)
-	{
 
-		/*

 
 
-/*

 

 
 
index 80bb269599..b6767ed72e 100644

 	/*
 
 	/*
 
 			/*
 
 	/*
index 75d6810d3e..bd6ff92f9a 100644

 	/*

 
index be05845248..065670b5c1 100644

 
 
-	if ($hot)
-	{
-
 
-
-
 =pod

 my $backup_name = 'b1';

---

From: Chapman Flack <[email-masked]>
Date: Sat, 26 Feb 2022 16:48:52 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

This patch applies cleanly for me and passes installcheck-world.
I have not yet studied all of the changes in detail.

Some proofreading nits in the documentation: the pg_stop_backup
with arguments has lost the 'exclusive' argument, but still shows a comma
before the 'wait_for_archive' argument. And the niladic pg_stop_backup
is still documented, though it no longer exists.

My biggest concerns are the changes to the SQL-visible pg_start_backup
and pg_stop_backup functions. When the non-exclusive API was implemented
(in 7117685), that was done with care (with a new optional argument to
pg_start_backup, and a new overload of pg_stop_backup) to avoid immediate
breakage of working backup scripts.

With this patch, even scripts that were dutifully migrated to that new API and
now invoke pg_start_backup(label, false) or (label, exclusive => false) will
immediately and unnecessarily break. What I would suggest for this patch
would be to change the exclusive default from true to false, and have the
function report an ERROR if true is passed.

Otherwise, for sites using a third-party backup solution, there will be an
unnecessary requirement to synchronize a PostgreSQL upgrade with an
upgrade of the backup solution that won't be broken by the change. For
a site with their backup procedures scripted in-house, there will be an
unnecessarily urgent need for the current admin team to study and patch
the currently-working scripts.

That can be avoided by just changing the default to false and rejecting calls
where true is passed. That will break only scripts that never got the memo
about moving to non-exclusive backup, available for six years now.

Assuming the value is false, so no error is thrown, is it practical to determine
from flinfo->fn_expr whether the value was defaulted or supplied? If so, I would
further suggest reporting a deprecation WARNING if it was explicitly supplied,
with a HINT that the argument can simply be removed at the call site, and will
become unrecognized in some future release.

pg_stop_backup needs thought, because 7117685 added a new overload for that
function, rather than just an optional argument. This patch removes the old
niladic version that returned pg_lsn, leaving just one version, with an optional
argument, that returns a record.

Here again, the old niladic one was only suitable for exclusive backups, so there
can't be any script existing in 2022 that still calls that unless it has never been
updated in six years to nonexclusive backups, and that breakage can't be
helped.

Any scripts that did get dutifully updated over the last six years will be calling the
record-returning version, passing false, or exclusive => false. This patch as it
stands will unnecessarily break those, but here again I think that can be avoided
just by making the exclusive parameter optional with default false, and reporting
an error if true is passed.

Here again, I would consider also issuing a deprecation warning if the argument
is explicitly supplied, if it is practical to determine that from fn_expr. (I haven't
looked yet to see how practical that is.)

Regards,
-Chap

---

From: Chapman Flack <[email-masked]>
Date: Sat, 26 Feb 2022 17:03:04 -0500

> I have not yet studied all of the changes in detail.

I've now looked through the rest, and the only further thing I noticed
was that xlog.c's do_pg_start_backup still has a tablespaces parameter
to receive a List* of tablespaces if the caller wants, but this patch
removes the comment describing it:

which seems like collateral damage.

Regards,
-Chap

---

From: Nathan Bossart <[email-masked]>
Date: Sat, 26 Feb 2022 14:06:14 -0800

> become unrecognized in some future release.

This is a good point.  I think I agree with your proposed changes.  I
believe it is possible to add a deprecation warning only when 'exclusive'
is specified.  If anything, we can create a separate function that accepts
the 'exclusive' parameter and that always emits a NOTICE or WARNING.

> looked yet to see how practical that is.)

Agreed.  I will look into updating this one, too.  I think the 'exclusive'
parameter should remain documented for now for both pg_start_backup() and
pg_stop_backup(), but this documentation will just note that it is there
for backward compatibility and must be set to false.

---

From: Nathan Bossart <[email-masked]>
Date: Sat, 26 Feb 2022 14:08:32 -0800

> which seems like collateral damage.

Thanks.  I will fix this and the proofreading nits you noted upthread in
the next revision.