On Thu, Aug 4, 2016 at 4:03 PM, Michael Paquier
<[email protected]> wrote:
> This window exists as well on back-branches btw, this is not new to
> 9.6. Magnus, what do you think?
Actually, a completely water-proof solution is to use an in-memory
flag proper to exclusive backups during the time pg_start_backup() is
called, at the cost of taking WALInsertLockAcquireExclusive once again
at the end of do_pg_start_backup(), but it seems to me that it is an
acceptable cost because pg_start_backup is not a hot code path. Using
a separate flag has the advantage to provide to the user a better
error message when pg_stop_backup is used while pg_start_backup is
running as well.
Andreas, with the patch attached is the assertion still triggered?
Thoughts from others are welcome as well.
--
Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f13f9c1..c4bc332 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -505,10 +505,13 @@ typedef struct XLogCtlInsert
* 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.
+ * exclusiveBackupStarted is true while pg_start_backup() is being called
+ * during an exclusive backup.
*/
bool exclusiveBackup;
int nonExclusiveBackups;
XLogRecPtr lastBackupStart;
+ bool exclusiveBackupStarted;
/*
* WAL insertion locks.
@@ -9833,7 +9836,8 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
WALInsertLockAcquireExclusive();
if (exclusive)
{
- if (XLogCtl->Insert.exclusiveBackup)
+ if (XLogCtl->Insert.exclusiveBackup ||
+ XLogCtl->Insert.exclusiveBackupStarted)
{
WALInsertLockRelease();
ereport(ERROR,
@@ -9842,6 +9846,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
errhint("Run pg_stop_backup() and try again.")));
}
XLogCtl->Insert.exclusiveBackup = true;
+ XLogCtl->Insert.exclusiveBackupStarted = true;
}
else
XLogCtl->Insert.nonExclusiveBackups++;
@@ -10178,6 +10183,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.exclusiveBackupStarted = false;
+ WALInsertLockRelease();
+ }
+
+ /*
* We're done. As a convenience, return the starting WAL location.
*/
if (starttli_p)
@@ -10195,8 +10210,10 @@ pg_start_backup_callback(int code, Datum arg)
WALInsertLockAcquireExclusive();
if (exclusive)
{
- Assert(XLogCtl->Insert.exclusiveBackup);
+ Assert(XLogCtl->Insert.exclusiveBackup &&
+ XLogCtl->Insert.exclusiveBackupStarted);
XLogCtl->Insert.exclusiveBackup = false;
+ XLogCtl->Insert.exclusiveBackupStarted = false;
}
else
{
@@ -10287,6 +10304,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("exclusive backup not in progress")));
}
+ if (XLogCtl->Insert.exclusiveBackupStarted)
+ {
+ WALInsertLockRelease();
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("exclusive backup currently starting")));
+ }
XLogCtl->Insert.exclusiveBackup = false;
}
else
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
