I sent this email one month ago but forgot to cc pgsql-hackers ;)
For the record, it is the set of patches attached that have been
pushed as 974ece5, and only Fujii-san has received them... Thanks for
committing the fix by the way!
On Thu, Dec 15, 2016 at 3:30 AM, Fujii Masao <masao.fu...@gmail.com> wrote:
> On Wed, Dec 14, 2016 at 11:10 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>> On second thought, do users really want to distinguish those three errornous
>> states? I'm inclined to merge the checks for those three conditions into one,
>> that is,
>>
>> if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_IN_NONE)
>> {
>> WALInsertLockRelease();
>> ereport(ERROR,
>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>> errmsg("a backup is already in progress"),
>>
>> Also it may be better to handle the similar checks in pg_stop_backup,
>> in the same way.
>
> Here is the updated version of the patch that I applied the above "merge" to.
Okay.
> Unfortunately this patch is not applied cleanly to old versions.
> So we need to create more patches for back-patch.
Attached are patches for all supported branches. I have tweaked the
following comment for master/9.6, that's why I am re-sending it:
+ * (see comments of ExclusiveBackupState for more details).
Getting master and 9.6 was straight-forward. 9.5 showed a small
conflict. 9.4 bigger conflicts but the code blocks are
straight-forward to place except that the tablespace map does not
exist there, and that we need to be careful that the same flow is used
for the removal of the backup_label file. At 9.3, conflicts are caused
because of changes of APIs for WAL insert locking. Finally 9.2 showed
no conflicts with 9.3, which was a bit surprising.
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 457a83452e..b33e3697e8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -349,6 +349,29 @@ typedef struct XLogwrtResult
} XLogwrtResult;
/*
+ * 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;
+
+/*
* Shared state data for XLogInsert.
*/
typedef struct XLogCtlInsert
@@ -370,13 +393,15 @@ typedef struct XLogCtlInsert
bool fullPageWrites;
/*
- * exclusiveBackup is true if a backup started with pg_start_backup() is
- * in progress, and 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.
+ * 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.
*/
- bool exclusiveBackup;
+ ExclusiveBackupState exclusiveBackupState;
int nonExclusiveBackups;
XLogRecPtr lastBackupStart;
} XLogCtlInsert;
@@ -693,6 +718,7 @@ static bool CheckForStandbyTrigger(void);
static void xlog_outrec(StringInfo buf, XLogRecord *record);
#endif
static void pg_start_backup_callback(int code, Datum arg);
+static void pg_stop_backup_callback(int code, Datum arg);
static bool read_backup_label(XLogRecPtr *checkPointLoc,
bool *backupEndRequired, bool *backupFromStandby);
static void rm_redo_error_callback(void *arg);
@@ -8699,7 +8725,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
if (exclusive)
{
- if (XLogCtl->Insert.exclusiveBackup)
+ /*
+ * 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)
{
LWLockRelease(WALInsertLock);
ereport(ERROR,
@@ -8707,7 +8738,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errmsg("a backup is already in progress"),
errhint("Run pg_stop_backup() and try again.")));
}
- XLogCtl->Insert.exclusiveBackup = true;
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
}
else
XLogCtl->Insert.nonExclusiveBackups++;
@@ -8867,7 +8898,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
{
/*
* Check for existing backup label --- implies a backup is already
- * running. (XXX given that we checked exclusiveBackup above,
+ * 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)
@@ -8909,6 +8940,16 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
/*
+ * Mark that start phase has correctly finished for an exclusive backup.
+ */
+ if (exclusive)
+ {
+ LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ LWLockRelease(WALInsertLock);
+ }
+
+ /*
* We're done. As a convenience, return the starting WAL location.
*/
if (starttli_p)
@@ -8926,8 +8967,8 @@ pg_start_backup_callback(int code, Datum arg)
LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
if (exclusive)
{
- Assert(XLogCtl->Insert.exclusiveBackup);
- XLogCtl->Insert.exclusiveBackup = false;
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
}
else
{
@@ -8935,7 +8976,7 @@ pg_start_backup_callback(int code, Datum arg)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
@@ -8944,6 +8985,24 @@ pg_start_backup_callback(int code, Datum arg)
}
/*
+ * Error cleanup callback for pg_stop_backup
+ */
+static void
+pg_stop_backup_callback(int code, Datum arg)
+{
+ bool exclusive = DatumGetBool(arg);
+
+ /* Update backup status on failure */
+ LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
+ if (exclusive)
+ {
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ }
+ LWLockRelease(WALInsertLock);
+}
+
+/*
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
* function.
@@ -9006,12 +9065,85 @@ 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 \"archive\" or \"hot_standby\" 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().
+ */
+ LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
+ if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_IN_PROGRESS)
+ {
+ LWLockRelease(WALInsertLock);
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("exclusive backup not in progress")));
+ }
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STOPPING;
+ LWLockRelease(WALInsertLock);
+
+ /*
+ * 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))
+ {
+ 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)));
+ if (unlink(BACKUP_LABEL_FILE) != 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove file \"%s\": %m",
+ BACKUP_LABEL_FILE)));
+ }
+ PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
+ }
+
/*
* OK to update backup counters and forcePageWrites
*/
LWLockAcquire(WALInsertLock, LW_EXCLUSIVE);
if (exclusive)
- XLogCtl->Insert.exclusiveBackup = false;
+ {
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
+ }
else
{
/*
@@ -9024,60 +9156,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
LWLockRelease(WALInsertLock);
- if (exclusive)
- {
- /*
- * Read the existing label file into memory.
- */
- struct stat statbuf;
- int r;
-
- if (stat(BACKUP_LABEL_FILE, &statbuf))
- {
- 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)));
- if (unlink(BACKUP_LABEL_FILE) != 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove file \"%s\": %m",
- BACKUP_LABEL_FILE)));
- }
-
/*
* Read and parse the START WAL LOCATION line (this code is pretty crude,
* but we are not expecting any variability in the file format).
@@ -9318,7 +9403,7 @@ do_pg_abort_backup(void)
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
XLogCtl->Insert.nonExclusiveBackups--;
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0f000d52a9..91edd54e8a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -418,6 +418,29 @@ typedef union WALInsertLockPadded
} 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;
+
+/*
* Shared state data for XLogInsert.
*/
typedef struct XLogCtlInsert
@@ -458,13 +481,15 @@ typedef struct XLogCtlInsert
bool fullPageWrites;
/*
- * exclusiveBackup is true if a backup started with pg_start_backup() is
- * in progress, and 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.
+ * 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.
*/
- bool exclusiveBackup;
+ ExclusiveBackupState exclusiveBackupState;
int nonExclusiveBackups;
XLogRecPtr lastBackupStart;
@@ -806,6 +831,7 @@ static bool CheckForStandbyTrigger(void);
static void xlog_outrec(StringInfo buf, XLogRecord *record);
#endif
static void pg_start_backup_callback(int code, Datum arg);
+static void pg_stop_backup_callback(int code, Datum arg);
static bool read_backup_label(XLogRecPtr *checkPointLoc,
bool *backupEndRequired, bool *backupFromStandby);
static void rm_redo_error_callback(void *arg);
@@ -9880,7 +9906,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
WALInsertLockAcquireExclusive();
if (exclusive)
{
- if (XLogCtl->Insert.exclusiveBackup)
+ /*
+ * 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,
@@ -9888,7 +9919,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errmsg("a backup is already in progress"),
errhint("Run pg_stop_backup() and try again.")));
}
- XLogCtl->Insert.exclusiveBackup = true;
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
}
else
XLogCtl->Insert.nonExclusiveBackups++;
@@ -10048,7 +10079,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
{
/*
* Check for existing backup label --- implies a backup is already
- * running. (XXX given that we checked exclusiveBackup above,
+ * 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)
@@ -10090,6 +10121,16 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
/*
+ * Mark that start phase has correctly finished for an exclusive backup.
+ */
+ if (exclusive)
+ {
+ WALInsertLockAcquireExclusive();
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ WALInsertLockRelease();
+ }
+
+ /*
* We're done. As a convenience, return the starting WAL location.
*/
if (starttli_p)
@@ -10107,8 +10148,8 @@ pg_start_backup_callback(int code, Datum arg)
WALInsertLockAcquireExclusive();
if (exclusive)
{
- Assert(XLogCtl->Insert.exclusiveBackup);
- XLogCtl->Insert.exclusiveBackup = false;
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
}
else
{
@@ -10116,7 +10157,7 @@ pg_start_backup_callback(int code, Datum arg)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
@@ -10125,6 +10166,24 @@ pg_start_backup_callback(int code, Datum arg)
}
/*
+ * Error cleanup callback for pg_stop_backup
+ */
+static void
+pg_stop_backup_callback(int code, Datum arg)
+{
+ bool exclusive = DatumGetBool(arg);
+
+ /* Update backup status on failure */
+ WALInsertLockAcquireExclusive();
+ if (exclusive)
+ {
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ }
+ WALInsertLockRelease();
+}
+
+/*
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
* function.
@@ -10187,12 +10246,86 @@ 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 \"archive\", \"hot_standby\", 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)));
+ if (unlink(BACKUP_LABEL_FILE) != 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove file \"%s\": %m",
+ BACKUP_LABEL_FILE)));
+ }
+ PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
+ }
+
/*
* OK to update backup counters and forcePageWrites
*/
WALInsertLockAcquireExclusive();
if (exclusive)
- XLogCtl->Insert.exclusiveBackup = false;
+ {
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
+ }
else
{
/*
@@ -10205,60 +10338,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
WALInsertLockRelease();
- if (exclusive)
- {
- /*
- * Read the existing label file into memory.
- */
- struct stat statbuf;
- int r;
-
- if (stat(BACKUP_LABEL_FILE, &statbuf))
- {
- 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)));
- if (unlink(BACKUP_LABEL_FILE) != 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove file \"%s\": %m",
- BACKUP_LABEL_FILE)));
- }
-
/*
* Read and parse the START WAL LOCATION line (this code is pretty crude,
* but we are not expecting any variability in the file format).
@@ -10499,7 +10585,7 @@ do_pg_abort_backup(void)
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
XLogCtl->Insert.nonExclusiveBackups--;
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 084401d2f2..18b847aaa9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -463,6 +463,29 @@ typedef union WALInsertLockPadded
} 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;
+
+/*
* Shared state data for WAL insertion.
*/
typedef struct XLogCtlInsert
@@ -503,13 +526,15 @@ typedef struct XLogCtlInsert
bool fullPageWrites;
/*
- * exclusiveBackup is true if a backup started with pg_start_backup() is
- * in progress, and 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.
+ * 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.
*/
- bool exclusiveBackup;
+ ExclusiveBackupState exclusiveBackupState;
int nonExclusiveBackups;
XLogRecPtr lastBackupStart;
@@ -848,6 +873,7 @@ static void xlog_outrec(StringInfo buf, XLogReaderState *record);
#endif
static void xlog_outdesc(StringInfo buf, XLogReaderState *record);
static void pg_start_backup_callback(int code, Datum arg);
+static void pg_stop_backup_callback(int code, Datum arg);
static bool read_backup_label(XLogRecPtr *checkPointLoc,
bool *backupEndRequired, bool *backupFromStandby);
static bool read_tablespace_map(List **tablespaces);
@@ -9957,7 +9983,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
WALInsertLockAcquireExclusive();
if (exclusive)
{
- if (XLogCtl->Insert.exclusiveBackup)
+ /*
+ * 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,
@@ -9965,7 +9996,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errmsg("a backup is already in progress"),
errhint("Run pg_stop_backup() and try again.")));
}
- XLogCtl->Insert.exclusiveBackup = true;
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
}
else
XLogCtl->Insert.nonExclusiveBackups++;
@@ -10220,7 +10251,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
{
/*
* Check for existing backup label --- implies a backup is already
- * running. (XXX given that we checked exclusiveBackup above,
+ * 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)
@@ -10302,6 +10333,16 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
/*
+ * Mark that start phase has correctly finished for an exclusive backup.
+ */
+ if (exclusive)
+ {
+ WALInsertLockAcquireExclusive();
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ WALInsertLockRelease();
+ }
+
+ /*
* We're done. As a convenience, return the starting WAL location.
*/
if (starttli_p)
@@ -10319,8 +10360,8 @@ pg_start_backup_callback(int code, Datum arg)
WALInsertLockAcquireExclusive();
if (exclusive)
{
- Assert(XLogCtl->Insert.exclusiveBackup);
- XLogCtl->Insert.exclusiveBackup = false;
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
}
else
{
@@ -10328,7 +10369,7 @@ pg_start_backup_callback(int code, Datum arg)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
@@ -10337,6 +10378,24 @@ pg_start_backup_callback(int code, Datum arg)
}
/*
+ * Error cleanup callback for pg_stop_backup
+ */
+static void
+pg_stop_backup_callback(int code, Datum arg)
+{
+ bool exclusive = DatumGetBool(arg);
+
+ /* Update backup status on failure */
+ WALInsertLockAcquireExclusive();
+ if (exclusive)
+ {
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ }
+ WALInsertLockRelease();
+}
+
+/*
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
* function.
*
@@ -10398,20 +10457,91 @@ 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.")));
- /*
- * OK to update backup counters and forcePageWrites
- */
- WALInsertLockAcquireExclusive();
if (exclusive)
{
- if (!XLogCtl->Insert.exclusiveBackup)
+ /*
+ * 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.exclusiveBackup = false;
+ 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)));
+ if (unlink(BACKUP_LABEL_FILE) != 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove file \"%s\": %m",
+ BACKUP_LABEL_FILE)));
+
+ /*
+ * Remove tablespace_map file if present, it is created only if there
+ * are tablespaces.
+ */
+ unlink(TABLESPACE_MAP);
+ }
+ PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
+ }
+
+ /*
+ * OK to update backup counters and forcePageWrites
+ */
+ WALInsertLockAcquireExclusive();
+ if (exclusive)
+ {
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
}
else
{
@@ -10425,66 +10555,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
WALInsertLockRelease();
- if (exclusive)
- {
- /*
- * Read the existing label file into memory.
- */
- struct stat statbuf;
- int r;
-
- if (stat(BACKUP_LABEL_FILE, &statbuf))
- {
- 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)));
- if (unlink(BACKUP_LABEL_FILE) != 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove file \"%s\": %m",
- BACKUP_LABEL_FILE)));
-
- /*
- * Remove tablespace_map file if present, it is created only if there
- * are tablespaces.
- */
- unlink(TABLESPACE_MAP);
- }
-
/*
* Read and parse the START WAL LOCATION line (this code is pretty crude,
* but we are not expecting any variability in the file format).
@@ -10721,7 +10798,7 @@ do_pg_abort_backup(void)
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
XLogCtl->Insert.nonExclusiveBackups--;
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 720edf75d0..e141c96346 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -457,6 +457,29 @@ typedef union WALInsertLockPadded
} 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;
+
+/*
* Shared state data for WAL insertion.
*/
typedef struct XLogCtlInsert
@@ -497,13 +520,15 @@ typedef struct XLogCtlInsert
bool fullPageWrites;
/*
- * exclusiveBackup is true if a backup started with pg_start_backup() is
- * in progress, and 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.
+ * 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.
*/
- bool exclusiveBackup;
+ ExclusiveBackupState exclusiveBackupState;
int nonExclusiveBackups;
XLogRecPtr lastBackupStart;
@@ -845,6 +870,7 @@ static void xlog_outrec(StringInfo buf, XLogReaderState *record);
#endif
static void xlog_outdesc(StringInfo buf, XLogReaderState *record);
static void pg_start_backup_callback(int code, Datum arg);
+static void pg_stop_backup_callback(int code, Datum arg);
static bool read_backup_label(XLogRecPtr *checkPointLoc,
bool *backupEndRequired, bool *backupFromStandby);
static bool read_tablespace_map(List **tablespaces);
@@ -9829,7 +9855,12 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
WALInsertLockAcquireExclusive();
if (exclusive)
{
- if (XLogCtl->Insert.exclusiveBackup)
+ /*
+ * 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,
@@ -9837,7 +9868,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errmsg("a backup is already in progress"),
errhint("Run pg_stop_backup() and try again.")));
}
- XLogCtl->Insert.exclusiveBackup = true;
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
}
else
XLogCtl->Insert.nonExclusiveBackups++;
@@ -10090,7 +10121,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
{
/*
* Check for existing backup label --- implies a backup is already
- * running. (XXX given that we checked exclusiveBackup above,
+ * 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)
@@ -10174,6 +10205,16 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
/*
+ * Mark that start phase has correctly finished for an exclusive backup.
+ */
+ if (exclusive)
+ {
+ WALInsertLockAcquireExclusive();
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ WALInsertLockRelease();
+ }
+
+ /*
* We're done. As a convenience, return the starting WAL location.
*/
if (starttli_p)
@@ -10191,8 +10232,8 @@ pg_start_backup_callback(int code, Datum arg)
WALInsertLockAcquireExclusive();
if (exclusive)
{
- Assert(XLogCtl->Insert.exclusiveBackup);
- XLogCtl->Insert.exclusiveBackup = false;
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
}
else
{
@@ -10200,7 +10241,7 @@ pg_start_backup_callback(int code, Datum arg)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
@@ -10209,6 +10250,24 @@ pg_start_backup_callback(int code, Datum arg)
}
/*
+ * Error cleanup callback for pg_stop_backup
+ */
+static void
+pg_stop_backup_callback(int code, Datum arg)
+{
+ bool exclusive = DatumGetBool(arg);
+
+ /* Update backup status on failure */
+ WALInsertLockAcquireExclusive();
+ if (exclusive)
+ {
+ Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
+ }
+ WALInsertLockRelease();
+}
+
+/*
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
* function.
@@ -10270,12 +10329,93 @@ 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 \"archive\", \"hot_standby\", 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)));
+ if (unlink(BACKUP_LABEL_FILE) != 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not remove file \"%s\": %m",
+ BACKUP_LABEL_FILE)));
+
+ /*
+ * Remove tablespace_map file if present, it is created only if there
+ * are tablespaces.
+ */
+ unlink(TABLESPACE_MAP);
+ }
+ PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
+ }
+
/*
* OK to update backup counters and forcePageWrites
*/
WALInsertLockAcquireExclusive();
if (exclusive)
- XLogCtl->Insert.exclusiveBackup = false;
+ {
+ XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
+ }
else
{
/*
@@ -10288,66 +10428,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
XLogCtl->Insert.nonExclusiveBackups--;
}
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
WALInsertLockRelease();
- if (exclusive)
- {
- /*
- * Read the existing label file into memory.
- */
- struct stat statbuf;
- int r;
-
- if (stat(BACKUP_LABEL_FILE, &statbuf))
- {
- 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)));
- if (unlink(BACKUP_LABEL_FILE) != 0)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not remove file \"%s\": %m",
- BACKUP_LABEL_FILE)));
-
- /*
- * Remove tablespace_map file if present, it is created only if there
- * are tablespaces.
- */
- unlink(TABLESPACE_MAP);
- }
-
/*
* Read and parse the START WAL LOCATION line (this code is pretty crude,
* but we are not expecting any variability in the file format).
@@ -10584,7 +10671,7 @@ do_pg_abort_backup(void)
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
XLogCtl->Insert.nonExclusiveBackups--;
- if (!XLogCtl->Insert.exclusiveBackup &&
+ if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
XLogCtl->Insert.nonExclusiveBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
