Commit 78874531 authored by Teodor Sigaev's avatar Teodor Sigaev

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/
parent 457a4448
...@@ -503,6 +503,12 @@ typedef enum ExclusiveBackupState ...@@ -503,6 +503,12 @@ typedef enum ExclusiveBackupState
EXCLUSIVE_BACKUP_STOPPING EXCLUSIVE_BACKUP_STOPPING
} ExclusiveBackupState; } 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. * Shared state data for WAL insertion.
*/ */
...@@ -10566,13 +10572,17 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ...@@ -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. * Mark that start phase has correctly finished for an exclusive backup.
* Session-level locks are updated as well to reflect that state.
*/ */
if (exclusive) if (exclusive)
{ {
WALInsertLockAcquireExclusive(); WALInsertLockAcquireExclusive();
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS; XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
WALInsertLockRelease(); WALInsertLockRelease();
sessionBackupState = SESSION_BACKUP_EXCLUSIVE;
} }
else
sessionBackupState = SESSION_BACKUP_NON_EXCLUSIVE;
/* /*
* We're done. As a convenience, return the starting WAL location. * We're done. As a convenience, return the starting WAL location.
...@@ -10627,6 +10637,15 @@ pg_stop_backup_callback(int code, Datum arg) ...@@ -10627,6 +10637,15 @@ pg_stop_backup_callback(int code, Datum arg)
WALInsertLockRelease(); 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() * do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
* function. * function.
...@@ -10794,6 +10813,9 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p) ...@@ -10794,6 +10813,9 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
} }
WALInsertLockRelease(); WALInsertLockRelease();
/* Clean up session-level lock */
sessionBackupState = SESSION_BACKUP_NONE;
/* /*
* Read and parse the START WAL LOCATION line (this code is pretty crude, * Read and parse the START WAL LOCATION line (this code is pretty crude,
* but we are not expecting any variability in the file format). * but we are not expecting any variability in the file format).
......
...@@ -42,8 +42,6 @@ ...@@ -42,8 +42,6 @@
*/ */
static StringInfo label_file; static StringInfo label_file;
static StringInfo tblspc_map_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, * Called when the backend exits with a running non-exclusive base backup,
...@@ -78,10 +76,11 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -78,10 +76,11 @@ pg_start_backup(PG_FUNCTION_ARGS)
char *backupidstr; char *backupidstr;
XLogRecPtr startpoint; XLogRecPtr startpoint;
DIR *dir; DIR *dir;
SessionBackupState status = get_backup_status();
backupidstr = text_to_cstring(backupid); backupidstr = text_to_cstring(backupid);
if (exclusive_backup_running || nonexclusive_backup_running) if (status == SESSION_BACKUP_NON_EXCLUSIVE)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("a backup is already in progress in this session"))); errmsg("a backup is already in progress in this session")));
...@@ -96,7 +95,6 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -96,7 +95,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
{ {
startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL, startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL,
dir, NULL, NULL, false, true); dir, NULL, NULL, false, true);
exclusive_backup_running = true;
} }
else else
{ {
...@@ -113,7 +111,6 @@ pg_start_backup(PG_FUNCTION_ARGS) ...@@ -113,7 +111,6 @@ pg_start_backup(PG_FUNCTION_ARGS)
startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file, startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file,
dir, NULL, tblspc_map_file, false, true); dir, NULL, tblspc_map_file, false, true);
nonexclusive_backup_running = true;
before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0); before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
} }
...@@ -147,8 +144,9 @@ Datum ...@@ -147,8 +144,9 @@ Datum
pg_stop_backup(PG_FUNCTION_ARGS) pg_stop_backup(PG_FUNCTION_ARGS)
{ {
XLogRecPtr stoppoint; XLogRecPtr stoppoint;
SessionBackupState status = get_backup_status();
if (nonexclusive_backup_running) if (status == SESSION_BACKUP_NON_EXCLUSIVE)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("non-exclusive backup in progress"), errmsg("non-exclusive backup in progress"),
...@@ -156,14 +154,12 @@ pg_stop_backup(PG_FUNCTION_ARGS) ...@@ -156,14 +154,12 @@ pg_stop_backup(PG_FUNCTION_ARGS)
/* /*
* Exclusive backups were typically started in a different connection, so * Exclusive backups were typically started in a different connection, so
* don't try to verify that exclusive_backup_running is set in this one. * don't try to verify that status of backup is set to
* Actual verification that an exclusive backup is in fact running is * SESSION_BACKUP_EXCLUSIVE in this function. Actual verification that an
* handled inside do_pg_stop_backup. * exclusive backup is in fact running is handled inside do_pg_stop_backup.
*/ */
stoppoint = do_pg_stop_backup(NULL, true, NULL); stoppoint = do_pg_stop_backup(NULL, true, NULL);
exclusive_backup_running = false;
PG_RETURN_LSN(stoppoint); PG_RETURN_LSN(stoppoint);
} }
...@@ -199,6 +195,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS) ...@@ -199,6 +195,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
bool exclusive = PG_GETARG_BOOL(0); bool exclusive = PG_GETARG_BOOL(0);
bool waitforarchive = PG_GETARG_BOOL(1); bool waitforarchive = PG_GETARG_BOOL(1);
XLogRecPtr stoppoint; XLogRecPtr stoppoint;
SessionBackupState status = get_backup_status();
/* check to see if caller supports us returning a tuplestore */ /* check to see if caller supports us returning a tuplestore */
if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo)) if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
...@@ -230,7 +227,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS) ...@@ -230,7 +227,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
if (exclusive) if (exclusive)
{ {
if (nonexclusive_backup_running) if (status == SESSION_BACKUP_NON_EXCLUSIVE)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("non-exclusive backup in progress"), errmsg("non-exclusive backup in progress"),
...@@ -241,14 +238,13 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS) ...@@ -241,14 +238,13 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
* return NULL for both backup_label and tablespace_map. * return NULL for both backup_label and tablespace_map.
*/ */
stoppoint = do_pg_stop_backup(NULL, waitforarchive, NULL); stoppoint = do_pg_stop_backup(NULL, waitforarchive, NULL);
exclusive_backup_running = false;
nulls[1] = true; nulls[1] = true;
nulls[2] = true; nulls[2] = true;
} }
else else
{ {
if (!nonexclusive_backup_running) if (status != SESSION_BACKUP_NON_EXCLUSIVE)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("non-exclusive backup is not in progress"), errmsg("non-exclusive backup is not in progress"),
...@@ -259,7 +255,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS) ...@@ -259,7 +255,6 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
* and tablespace map so they can be written to disk by the caller. * and tablespace map so they can be written to disk by the caller.
*/ */
stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL); stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
nonexclusive_backup_running = false;
cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0); cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
values[1] = CStringGetTextDatum(label_file->data); values[1] = CStringGetTextDatum(label_file->data);
......
...@@ -288,8 +288,26 @@ extern void assign_max_wal_size(int newval, void *extra); ...@@ -288,8 +288,26 @@ extern void assign_max_wal_size(int newval, void *extra);
extern void assign_checkpoint_completion_target(double 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, extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast,
TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir, TimeLineID *starttli_p, StringInfo labelfile, DIR *tblspcdir,
List **tablespaces, StringInfo tblspcmapfile, bool infotbssize, List **tablespaces, StringInfo tblspcmapfile, bool infotbssize,
...@@ -297,6 +315,7 @@ extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast, ...@@ -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, extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive,
TimeLineID *stoptli_p); TimeLineID *stoptli_p);
extern void do_pg_abort_backup(void); extern void do_pg_abort_backup(void);
extern SessionBackupState get_backup_status(void);
/* File path names (all relative to $PGDATA) */ /* File path names (all relative to $PGDATA) */
#define BACKUP_LABEL_FILE "backup_label" #define BACKUP_LABEL_FILE "backup_label"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment