Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit56a95ee

Browse files
committed
Fix bug in cancellation of non-exclusive backup to avoid assertion failure.
Previously an assertion failure occurred when pg_stop_backup() fornon-exclusive backup was aborted while it's waiting for WAL files tobe archived. This assertion failure happened in do_pg_abort_backup()which was called when a non-exclusive backup was canceled.do_pg_abort_backup() assumes that there is at least one non-exclusivebackup running when it's called. But pg_stop_backup() can be canceledeven after it marks the end of non-exclusive backup (e.g.,during waiting for WAL archiving). This broke the assumption thatdo_pg_abort_backup() relies on, and which caused an assertion failure.This commit changes do_pg_abort_backup() so that it does nothingwhen non-exclusive backup has been already marked as completed.That is, the asssumption is also changed, and do_pg_abort_backup()now can handle even the case where it's called when there isno running backup.Backpatch to 9.6 where SQL-callable non-exclusive backup was added.Author: Masahiko Sawada and Michael PaquierReviewed-By: Robert Haas and Fujii MasaoDiscussion:https://www.postgresql.org/message-id/CAD21AoD2L1Fu2c==gnVASMyFAAaq3y-AQ2uEVj-zTCGFFjvmDg@mail.gmail.com
1 parentfd7c0fa commit56a95ee

File tree

2 files changed

+35
-6
lines changed

2 files changed

+35
-6
lines changed

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10628,13 +10628,20 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
1062810628
/*
1062910629
* Mark that start phase has correctly finished for an exclusive backup.
1063010630
* Session-level locks are updated as well to reflect that state.
10631+
*
10632+
* Note that CHECK_FOR_INTERRUPTS() must not occur while updating
10633+
* backup counters and session-level lock. Otherwise they can be
10634+
* updated inconsistently, and which might cause do_pg_abort_backup()
10635+
* to fail.
1063110636
*/
1063210637
if (exclusive)
1063310638
{
1063410639
WALInsertLockAcquireExclusive();
1063510640
XLogCtl->Insert.exclusiveBackupState=EXCLUSIVE_BACKUP_IN_PROGRESS;
10636-
WALInsertLockRelease();
10641+
10642+
/* Set session-level lock */
1063710643
sessionBackupState=SESSION_BACKUP_EXCLUSIVE;
10644+
WALInsertLockRelease();
1063810645
}
1063910646
else
1064010647
sessionBackupState=SESSION_BACKUP_NON_EXCLUSIVE;
@@ -10838,7 +10845,11 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1083810845
}
1083910846

1084010847
/*
10841-
* OK to update backup counters and forcePageWrites
10848+
* OK to update backup counters, forcePageWrites and session-level lock.
10849+
*
10850+
* Note that CHECK_FOR_INTERRUPTS() must not occur while updating them.
10851+
* Otherwise they can be updated inconsistently, and which might cause
10852+
* do_pg_abort_backup() to fail.
1084210853
*/
1084310854
WALInsertLockAcquireExclusive();
1084410855
if (exclusive)
@@ -10862,11 +10873,20 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1086210873
{
1086310874
XLogCtl->Insert.forcePageWrites= false;
1086410875
}
10865-
WALInsertLockRelease();
1086610876

10867-
/* Clean up session-level lock */
10877+
/*
10878+
* Clean up session-level lock.
10879+
*
10880+
* You might think that WALInsertLockRelease() can be called
10881+
* before cleaning up session-level lock because session-level
10882+
* lock doesn't need to be protected with WAL insertion lock.
10883+
* But since CHECK_FOR_INTERRUPTS() can occur in it,
10884+
* session-level lock must be cleaned up before it.
10885+
*/
1086810886
sessionBackupState=SESSION_BACKUP_NONE;
1086910887

10888+
WALInsertLockRelease();
10889+
1087010890
/*
1087110891
* Read and parse the START WAL LOCATION line (this code is pretty crude,
1087210892
* but we are not expecting any variability in the file format).
@@ -11104,8 +11124,16 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
1110411124
void
1110511125
do_pg_abort_backup(void)
1110611126
{
11127+
/*
11128+
* Quick exit if session is not keeping around a non-exclusive backup
11129+
* already started.
11130+
*/
11131+
if (sessionBackupState==SESSION_BACKUP_NONE)
11132+
return;
11133+
1110711134
WALInsertLockAcquireExclusive();
1110811135
Assert(XLogCtl->Insert.nonExclusiveBackups>0);
11136+
Assert(sessionBackupState==SESSION_BACKUP_NON_EXCLUSIVE);
1110911137
XLogCtl->Insert.nonExclusiveBackups--;
1111011138

1111111139
if (XLogCtl->Insert.exclusiveBackupState==EXCLUSIVE_BACKUP_NONE&&

‎src/backend/replication/basebackup.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ perform_base_backup(basebackup_options *opt)
215215
* Once do_pg_start_backup has been called, ensure that any failure causes
216216
* us to abort the backup so we don't "leak" a backup counter. For this
217217
* reason, *all* functionality between do_pg_start_backup() and
218-
* do_pg_stop_backup() should be inside the error cleanup block!
218+
*the end ofdo_pg_stop_backup() should be inside the error cleanup block!
219219
*/
220220

221221
PG_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum)0);
@@ -324,10 +324,11 @@ perform_base_backup(basebackup_options *opt)
324324
else
325325
pq_putemptymessage('c');/* CopyDone */
326326
}
327+
328+
endptr=do_pg_stop_backup(labelfile->data, !opt->nowait,&endtli);
327329
}
328330
PG_END_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum)0);
329331

330-
endptr=do_pg_stop_backup(labelfile->data, !opt->nowait,&endtli);
331332

332333
if (opt->includewal)
333334
{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp