Fix backup canceling

Assert-enabled build crashes but without asserts it works by wrong way:
it may not reset forcing full page write and preventing from starting
exclusive backup with the same name as cancelled.
Patch replaces pair of booleans
nonexclusive_backup_running/exclusive_backup_running to single enum to
correctly describe backup state.

Backpatch to 9.6 where bug was introduced

Reported-by: David Steele
Authors: Michael Paquier, David Steele
Reviewed-by: Anastasia Lubennikova
https://commitfest.postgresql.org/13/1068/

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b99ded5df67db13826195a77ae8c41e61b9f73f0..58790e0e96e29a77c72211c1f20118ce95608daa 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -503,6 +503,12 @@ typedef enum ExclusiveBackupState
        EXCLUSIVE_BACKUP_STOPPING
 } ExclusiveBackupState;
 
+/*
+ * Session status of running backup, used for sanity checks in SQL-callable
+ * functions to start and stop backups.
+ */
+static SessionBackupState sessionBackupState = SESSION_BACKUP_NONE;
+
 /*
  * Shared state data for WAL insertion.
  */
@@ -10566,13 +10572,17 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 
        /*
         * Mark that start phase has correctly finished for an exclusive backup.
+        * Session-level locks are updated as well to reflect that state.
         */
        if (exclusive)
        {
                WALInsertLockAcquireExclusive();
                XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
                WALInsertLockRelease();
+               sessionBackupState = SESSION_BACKUP_EXCLUSIVE;
        }
+       else
+               sessionBackupState = SESSION_BACKUP_NON_EXCLUSIVE;
 
        /*
         * We're done.  As a convenience, return the starting WAL location.
@@ -10627,6 +10637,15 @@ pg_stop_backup_callback(int code, Datum arg)
        WALInsertLockRelease();
 }
 
+/*
+ * Utility routine to fetch the session-level status of a backup running.
+ */
+SessionBackupState
+get_backup_status(void)
+{
+       return sessionBackupState;
+}
+
 /*
  * do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
  * function.
@@ -10794,6 +10813,9 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
        }
        WALInsertLockRelease();
 
+       /* Clean up session-level lock */
+       sessionBackupState = SESSION_BACKUP_NONE;
+
        /*
         * Read and parse the START WAL LOCATION line (this code is pretty crude,
         * but we are not expecting any variability in the file format).

diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 5073aaca84f60064b280d0dcd04a6227c180d99f..5041f0e2a9409a6fdcf8a0067fd71549b5e4be60 100644 (file)
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -42,8 +42,6 @@
  */
 static StringInfo label_file;
 static StringInfo tblspc_map_file;
-static bool exclusive_backup_running = false;
-static bool nonexclusive_backup_running = false;
 
 /*
  * Called when the backend exits with a running non-exclusive base backup,
@@ -78,10 +76,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
        char       *backupidstr;
        XLogRecPtr      startpoint;
        DIR                *dir;
+       SessionBackupState status = get_backup_status();
 
        backupidstr = text_to_cstring(backupid);
 
-       if (exclusive_backup_running || nonexclusive_backup_running)
+       if (status == SESSION_BACKUP_NON_EXCLUSIVE)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                 errmsg("a backup is already in progress in this session")));
@@ -96,7 +95,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
        {
                startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
                                                                                dir, NULL, NULL, false, true);
-               exclusive_backup_running = true;
        }
        else
        {
@@ -113,7 +111,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
 
                startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
                                                                        dir, NULL, tblspc_map_file, false, true);
-               nonexclusive_backup_running = true;
 
                before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
        }
@@ -147,8 +144,9 @@ Datum
 pg_stop_backup(PG_FUNCTION_ARGS)
 {
        XLogRecPtr      stoppoint;
+       SessionBackupState status = get_backup_status();
 
-       if (nonexclusive_backup_running)
+       if (status == SESSION_BACKUP_NON_EXCLUSIVE)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                 errmsg("non-exclusive backup in progress"),
@@ -156,14 +154,12 @@ pg_stop_backup(PG_FUNCTION_ARGS)
 
        /*
         * Exclusive backups were typically started in a different connection, so
-        * don't try to verify that exclusive_backup_running is set in this one.
-        * Actual verification that an exclusive backup is in fact running is
-        * handled inside do_pg_stop_backup.
+        * 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);
 
-       exclusive_backup_running = false;
-
        PG_RETURN_LSN(stoppoint);
 }
 
@@ -199,6 +195,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
        bool            exclusive = PG_GETARG_BOOL(0);
        bool            waitforarchive = PG_GETARG_BOOL(1);
        XLogRecPtr      stoppoint;
+       SessionBackupState status = get_backup_status();
 
        /* check to see if caller supports us returning a tuplestore */
        if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
@@ -230,7 +227,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
 
        if (exclusive)
        {
-               if (nonexclusive_backup_running)
+               if (status == SESSION_BACKUP_NON_EXCLUSIVE)
                        ereport(ERROR,
                                        (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                         errmsg("non-exclusive backup in progress"),
@@ -241,14 +238,13 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
                 * return NULL for both backup_label and tablespace_map.
                 */
                stoppoint = do_pg_stop_backup(NULL, waitforarchive, NULL);
-               exclusive_backup_running = false;
 
                nulls[1] = true;
                nulls[2] = true;
        }
        else
        {
-               if (!nonexclusive_backup_running)
+               if (status != SESSION_BACKUP_NON_EXCLUSIVE)
                        ereport(ERROR,
                                        (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                         errmsg("non-exclusive backup is not in progress"),
@@ -259,7 +255,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
                 * and tablespace map so they can be written to disk by the caller.
                 */
                stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
-               nonexclusive_backup_running = false;
                cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
 
                values[1] = CStringGetTextDatum(label_file->data);

diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 104ee7dd5ed904cd96286eb8bdaf24b076cb291b..d4abf948628a37427b674657247e1780826ed467 100644 (file)
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -288,8 +288,26 @@ extern void assign_max_wal_size(int newval, void *extra);
 extern void assign_checkpoint_completion_target(double newval, void *extra);
 
 /*
- * Starting/stopping a base backup
+ * Routines to start, stop, and get status of a base backup.
  */
+
+/*
+ * Session-level status of base backups
+ *
+ * This is used in parallel with the shared memory status to control parallel
+ * execution of base backup functions for a given session, be it a backend
+ * dedicated to replication or a normal backend connected to a database. The
+ * update of the session-level status happens at the same time as the shared
+ * memory counters to keep a consistent global and local state of the backups
+ * running.
+ */
+typedef enum SessionBackupState
+{
+       SESSION_BACKUP_NONE,
+       SESSION_BACKUP_EXCLUSIVE,
+       SESSION_BACKUP_NON_EXCLUSIVE
+} SessionBackupState;
+
 extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
                                TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir,
                          List **tablespaces, StringInfo tblspcmapfile, bool infotbssize,
@@ -297,6 +315,7 @@ extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
 extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,
                                  TimeLineID *stoptli_p);
 extern void do_pg_abort_backup(void);
+extern SessionBackupState get_backup_status(void);
 
 /* File path names (all relative to $PGDATA) */
 #define BACKUP_LABEL_FILE              "backup_label"