8000 Fix an assertion failure related to an exclusive backup. · wenxiaozxw/postgres@974ece5 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 974ece5

Browse files
MasaoFujiiMasaoFujii
committed
Fix an assertion failure related to an exclusive backup.
Previously multiple sessions could execute pg_start_backup() and pg_stop_backup() to start and stop an exclusive backup at the same time. This could trigger the assertion failure of "FailedAssertion("!(XLogCtl->Insert.exclusiveBackup)". This happend because, even while pg_start_backup() was starting an exclusive backup, other session could run pg_stop_backup() concurrently and mark the backup as not-in-progress unconditionally. This patch introduces ExclusiveBackupState indicating the state of an exclusive backup. This state is used to ensure that there is only one session running pg_start_backup() or pg_stop_backup() at the same time, to avoid the assertion failure. Back-patch to all supported versions. Author: Michael Paquier Reviewed-By: Kyotaro Horiguchi and me Reported-By: Andreas Seltenreich Discussion: <87mvktojme.fsf@credativ.de>
1 parent d43a619 commit 974ece5

File tree

1 file changed

+150
-73
lines changed
  • src/backend/access/transam

1 file changed

+150
-73
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 150 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,29 @@ typedef union WALInsertLockPadded
472472
char pad[PG_CACHE_LINE_SIZE];
473473
} WALInsertLockPadded;
474474

475+
/*
476+
* State of an exclusive backup, necessary to control concurrent activities
477+
* across sessions when working on exclusive backups.
478+
*
479+
* EXCLUSIVE_BACKUP_NONE means that there is no exclusive backup actually
480+
* running, to be more precise pg_start_backup() is not being executed for
481+
* an exclusive backup and there is no exclusive backup in progress.
482+
* EXCLUSIVE_BACKUP_STARTING means that pg_start_backup() is starting an
483+
* exclusive backup.
484+
* EXCLUSIVE_BACKUP_IN_PROGRESS means that pg_start_backup() has finished
485+
* running and an exclusive backup is in progress. pg_stop_backup() is
486+
* needed to finish it.
487+
* EXCLUSIVE_BACKUP_STOPPING means that pg_stop_backup() is stopping an
488+
* exclusive backup.
489+
*/
490+
typedef enum ExclusiveBackupState
491+
{
492+
EXCLUSIVE_BACKUP_NONE = 0,
493+
EXCLUSIVE_BACKUP_STARTING,
494+
EXCLUSIVE_BACKUP_IN_PROGRESS,
495+
EXCLUSIVE_BACKUP_STOPPING
496+
} ExclusiveBackupState;
497+
475498
/*
476499
* Shared state data for WAL insertion.
477500
*/
@@ -513,13 +536,15 @@ typedef struct XLogCtlInsert
513536
bool fullPageWrites;
514537

515538
/*
516-
* exclusiveBackup is true if a backup started with pg_start_backup() is
517-
* in progress, and nonExclusiveBackups is a counter indicating the number
518-
* of streaming base backups currently in progress. forcePageWrites is set
519-
* to true when either of these is non-zero. lastBackupStart is the latest
520-
* checkpoint redo location used as a starting point for an online backup.
539+
* exclusiveBackupState indicates the state of an exclusive backup
540+
* (see comments of ExclusiveBackupState for more details).
541+
* nonExclusiveBackups is a counter indicating the number of streaming
542+
* base backups currently in progress. forcePageWrites is set to true
543+
* when either of these is non-zero. lastBackupStart is the latest
544+
* checkpoint redo location used as a starting point for an online
545+
* backup.
521546
*/
522-
bool exclusiveBackup;
547+
ExclusiveBackupState exclusiveBackupState;
523548
int nonExclusiveBackups;
524549
XLogRecPtr lastBackupStart;
525550

@@ -858,6 +883,7 @@ static void xlog_outrec(StringInfo buf, XLogReaderState *record);
858883
#endif
859884
static void xlog_outdesc(StringInfo buf, XLogReaderState *record);
860885
static void pg_start_backup_callback(int code, Datum arg);
886+
static void pg_stop_backup_callback(int code, Datum arg);
861887
static bool read_backup_label(XLogRecPtr *checkPointLoc,
862888
bool *backupEndRequired, bool *backupFromStandby);
863889
static bool read_tablespace_map(List **tablespaces);
@@ -10016,15 +10042,20 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1001610042
WALInsertLockAcquireExclusive();
1001710043
if (exclusive)
1001810044
{
10019-
if (XLogCtl->Insert.exclusiveBackup)
10045+
/*
10046+
* At first, mark that we're now starting an exclusive backup,
10047+
* to ensure that there are no other sessions currently running
10048+
* pg_start_backup() or pg_stop_backup().
10049+
*/
10050+
if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_NONE)
1002010051
{
1002110052
WALInsertLockRelease();
1002210053
ereport(ERROR,
1002310054
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
1002410055
errmsg("a backup is already in progress"),
1002510056
errhint("Run pg_stop_backup() and try again.")));
1002610057
}
10027-
XLogCtl->Insert.exclusiveBackup = true;
10058+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STARTING;
1002810059
}
1002910060
else
1003010061
XLogCtl->Insert.nonExclusiveBackups++;
@@ -10279,7 +10310,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1027910310
{
1028010311
/*
1028110312
* Check for existing backup label --- implies a backup is already
10282-
* running. (XXX given that we checked exclusiveBackup above,
10313+
* running. (XXX given that we checked exclusiveBackupState above,
1028310314
* maybe it would be OK to just unlink any such label file?)
1028410315
*/
1028510316
if (stat(BACKUP_LABEL_FILE, &stat_buf) != 0)
@@ -10360,6 +10391,16 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1036010391
}
1036110392
PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) BoolGetDatum(exclusive));
1036210393

10394+
/*
10395+
* Mark that start phase has correctly finished for an exclusive backup.
10396+
*/
10397+
if (exclusive)
10398+
{
10399+
WALInsertLockAcquireExclusive();
10400+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
10401+
WALInsertLockRelease();
10402+
}
10403+
1036310404
/*
1036410405
* We're done. As a convenience, return the starting WAL location.
1036510406
*/
@@ -10378,23 +10419,41 @@ pg_start_backup_callback(int code, Datum arg)
1037810419
WALInsertLockAcquireExclusive();
1037910420
if (exclusive)
1038010421
{
10381-
Assert(XLogCtl->Insert.exclusiveBackup);
10382-
XLogCtl->Insert.exclusiveBackup = false;
10422+
Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STARTING);
10423+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
1038310424
}
1038410425
else
1038510426
{
1038610427
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
1038710428
XLogCtl->Insert.nonExclusiveBackups--;
1038810429
}
1038910430

10390-
if (!XLogCtl->Insert.exclusiveBackup &&
10431+
if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
1039110432
XLogCtl->Insert.nonExclusiveBackups == 0)
1039210433
{
1039310434
XLogCtl->Insert.forcePageWrites = false;
1039410435
}
1039510436
WALInsertLockRelease();
1039610437
}
1039710438

10439+
/*
10440+
* Error cleanup callback for pg_stop_backup
10441+
*/
10442+
static void
10443+
pg_stop_backup_callback(int code, Datum arg)
10444+
{
10445+
bool exclusive = DatumGetBool(arg);
10446+
10447+
/* Update backup status on failure */
10448+
WALInsertLockAcquireExclusive();
10449+
if (exclusive)
10450+
{
10451+
Assert(XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_STOPPING);
10452+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_IN_PROGRESS;
10453+
}
10454+
WALInsertLockRelease();
10455+
}
10456+
1039810457
/*
1039910458
* do_pg_stop_backup is the workhorse of the user-visible pg_stop_backup()
1040010459
* function.
@@ -10457,20 +10516,91 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1045710516
errmsg("WAL level not sufficient for making an online backup"),
1045810517
errhint("wal_level must be set to \"replica\" or \"logical\" at server start.")));
1045910518

10460-
/*
10461-
* OK to update backup counters and forcePageWrites
10462-
*/
10463-
WALInsertLockAcquireExclusive();
1046410519
if (exclusive)
1046510520
{
10466-
if (!XLogCtl->Insert.exclusiveBackup)
10521+
/*
10522+
* At first, mark that we're now stopping an exclusive backup,
10523+
* to ensure that there are no other sessions currently running
10524+
* pg_start_backup() or pg_stop_backup().
10525+
*/
10526+
WALInsertLockAcquireExclusive();
10527+
if (XLogCtl->Insert.exclusiveBackupState != EXCLUSIVE_BACKUP_IN_PROGRESS)
1046710528
{
1046810529
WALInsertLockRelease();
1046910530
ereport(ERROR,
1047010531
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
1047110532
errmsg("exclusive backup not in progress")));
1047210533
}
10473-
XLogCtl->Insert.exclusiveBackup = false;
10534+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_STOPPING;
10535+
WALInsertLockRelease();
10536+
10537+
/*
10538+
* Remove backup_label. In case of failure, the state for an exclusive
10539+
* backup is switched back to in-progress.
10540+
*/
10541+
PG_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
10542+
{
10543+
/*
10544+
* Read the existing label file into memory.
10545+
*/
10546+
struct stat statbuf;
10547+
int r;
10548+
10549+
if (stat(BACKUP_LABEL_FILE, &statbuf))
10550+
{
10551+
/* should not happen per the upper checks */
10552+
if (errno != ENOENT)
10553+
ereport(ERROR,
10554+
(errcode_for_file_access(),
10555+
errmsg("could not stat file \"%s\": %m",
10556+
BACKUP_LABEL_FILE)));
10557+
ereport(ERROR,
10558+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
10559+
errmsg("a backup is not in progress")));
10560+
}
10561+
10562+
lfp = AllocateFile(BACKUP_LABEL_FILE, "r");
10563+
if (!lfp)
10564+
{
10565+
ereport(ERROR,
10566+
(errcode_for_file_access(),
10567+
errmsg("could not read file \"%s\": %m",
10568+
BACKUP_LABEL_FILE)));
10569+
}
10570+
labelfile = palloc(statbuf.st_size + 1);
10571+
r = fread(labelfile, statbuf.st_size, 1, lfp);
10572+
labelfile[statbuf.st_size] = '\0';
10573+
10574+
/*
10575+
* Close and remove the backup label file
10576+
*/
10577+
if (r != 1 || ferror(lfp) || FreeFile(lfp))
10578+
ereport(ERROR,
10579+
(errcode_for_file_access(),
10580+
errmsg("could not read file \"%s\": %m",
10581+
BACKUP_LABEL_FILE)));
10582+
if (unlink(BACKUP_LABEL_FILE) != 0)
10583+
ereport(ERROR,
10584+
(errcode_for_file_access(),
10585+
errmsg("could not remove file \"%s\": %m",
10586+
BACKUP_LABEL_FILE)));
10587+
10588+
/*
10589+
* Remove tablespace_map file if present, it is created only if there
10590+
* are tablespaces.
10591+
*/
10592+
unlink(TABLESPACE_MAP);
10593+
}
10594+
PG_END_ENSURE_ERROR_CLEANUP(pg_stop_backup_callback, (Datum) BoolGetDatum(exclusive));
10595+
}
10596+
10597+
/*
10598+
* OK to update backup counters and forcePageWrites
10599+
*/
10600+
WALInsertLockAcquireExclusive();
10601+
if (exclusive)
10602+
{
10603+
XLogCtl->Insert.exclusiveBackupState = EXCLUSIVE_BACKUP_NONE;
1047410604
}
1047510605
else
1047610606
{
@@ -10484,66 +10614,13 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1048410614
XLogCtl->Insert.nonExclusiveBackups--;
1048510615
}
1048610616

10487-
if (!XLogCtl->Insert.exclusiveBackup &&
10617+
if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
1048810618
XLogCtl->Insert.nonExclusiveBackups == 0)
1048910619
{
1049010620
XLogCtl->Insert.forcePageWrites = false;
1049110621
}
1049210622
WALInsertLockRelease();
1049310623

10494-
if (exclusive)
10495-
{
10496-
/*
10497-
* Read the existing label file into memory.
10498-
*/
10499-
struct stat statbuf;
10500-
int r;
10501-
10502-
if (stat(BACKUP_LABEL_FILE, &statbuf))
10503-
{
10504-
if (errno != ENOENT)
10505-
ereport(ERROR,
10506-
(errcode_for_file_access(),
10507-
errmsg("could not stat file \"%s\": %m",
10508-
BACKUP_LABEL_FILE)));
10509-
ereport(ERROR,
10510-
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
10511-
errmsg("a backup is not in progress")));
10512-
}
10513-
10514-
lfp = AllocateFile(BACKUP_LABEL_FILE, "r");
10515-
if (!lfp)
10516-
{
10517-
ereport(ERROR,
10518-
(errcode_for_file_access(),
10519-
errmsg("could not read file \"%s\": %m",
10520-
BACKUP_LABEL_FILE)));
10521-
}
10522-
labelfile = palloc(statbuf.st_size + 1);
10523-
r = fread(labelfile, statbuf.st_size, 1, lfp);
10524-
labelfile[statbuf.st_size] = '\0';
10525-
10526-
/*
10527-
* Close and remove the backup label file
10528-
*/
10529-
if (r != 1 || ferror(lfp) || FreeFile(lfp))
10530-
ereport(ERROR,
10531-
(errcode_for_file_access(),
10532-
errmsg("could not read file \"%s\": %m",
10533-
BACKUP_LABEL_FILE)));
10534-
if (unlink(BACKUP_LABEL_FILE) != 0)
10535-
ereport(ERROR,
10536-
(errcode_for_file_access(),
10537-
errmsg("could not remove file \"%s\": %m",
10538-
BACKUP_LABEL_FILE)));
10539-
10540-
/*
10541-
* Remove tablespace_map file if present, it is created only if there
10542-
* are tablespaces.
10543-
*/
10544-
unlink(TABLESPACE_MAP);
10545-
}
10546-
1054710624
/*
1054810625
* Read and parse the START WAL LOCATION line (this code is pretty crude,
1054910626
* but we are not expecting any variability in the file format).
@@ -10780,7 +10857,7 @@ do_pg_abort_backup(void)
1078010857
Assert(XLogCtl->Insert.nonExclusiveBackups > 0);
1078110858
XLogCtl->Insert.nonExclusiveBackups--;
1078210859

10783-
if (!XLogCtl->Insert.exclusiveBackup &&
10860+
if (XLogCtl->Insert.exclusiveBackupState == EXCLUSIVE_BACKUP_NONE &&
1078410861
XLogCtl->Insert.nonExclusiveBackups == 0)
1078510862
{
1078610863
XLogCtl->Insert.forcePageWrites = false;

0 commit comments

Comments
 (0)
0