Skip to content

Navigation Menu

Sign in

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
Appearance settings

Commit 342bb38

Browse filesBrowse files
alvherrealvherre
committed
Get rid of XLogCtlInsert->forcePageWrites
After commit 39969e2, ->forcePageWrites is no longer very interesting: we can just test whether runningBackups is different from 0. This simplifies some code, so do away with it. Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/39969e2a1e4d7f5a37f3ef37d53bbfe171e7d77a
1 parent c2ae01f commit 342bb38
Copy full SHA for 342bb38

File tree

2 files changed

+27
-40
lines changed
Filter options

2 files changed

+27
-40
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/access/transam/xlog.c
+24-37Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ XLogRecPtr XactLastCommitEnd = InvalidXLogRecPtr;
276276
static XLogRecPtr RedoRecPtr;
277277

278278
/*
279-
* doPageWrites is this backend's local copy of (forcePageWrites ||
280-
* fullPageWrites). It is used together with RedoRecPtr to decide whether
279+
* doPageWrites is this backend's local copy of (fullPageWrites ||
280+
* runningBackups > 0). It is used together with RedoRecPtr to decide whether
281281
* a full-page image of a page need to be taken.
282282
*
283283
* NB: Initially this is false, and there's no guarantee that it will be
@@ -437,14 +437,12 @@ typedef struct XLogCtlInsert
437437
* you must hold ALL the locks.
438438
*/
439439
XLogRecPtr RedoRecPtr; /* current redo point for insertions */
440-
bool forcePageWrites; /* forcing full-page writes for PITR? */
441440
bool fullPageWrites;
442441

443442
/*
444443
* runningBackups is a counter indicating the number of backups currently
445-
* in progress. forcePageWrites is set to true when runningBackups is
446-
* non-zero. lastBackupStart is the latest checkpoint redo location used
447-
* as a starting point for an online backup.
444+
* in progress. lastBackupStart is the latest checkpoint redo location
445+
* used as a starting point for an online backup.
448446
*/
449447
int runningBackups;
450448
XLogRecPtr lastBackupStart;
@@ -805,9 +803,9 @@ XLogInsertRecord(XLogRecData *rdata,
805803
* happen just after a checkpoint, so it's better to be slow in this case
806804
* and fast otherwise.
807805
*
808-
* Also check to see if fullPageWrites or forcePageWrites was just turned
809-
* on; if we weren't already doing full-page writes then go back and
810-
* recompute.
806+
* Also check to see if fullPageWrites was just turned on or there's a
807+
* running backup (which forces full-page writes); if we weren't already
808+
* doing full-page writes then go back and recompute.
811809
*
812810
* If we aren't doing full-page writes then RedoRecPtr doesn't actually
813811
* affect the contents of the XLOG record, so we'll update our local copy
@@ -820,7 +818,7 @@ XLogInsertRecord(XLogRecData *rdata,
820818
Assert(RedoRecPtr < Insert->RedoRecPtr);
821819
RedoRecPtr = Insert->RedoRecPtr;
822820
}
823-
doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites);
821+
doPageWrites = (Insert->fullPageWrites || Insert->runningBackups > 0);
824822

825823
if (doPageWrites &&
826824
(!prevDoPageWrites ||
@@ -1899,7 +1897,7 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, TimeLineID tli, bool opportunistic)
18991897
* is unsafe, but at worst the archiver would miss the opportunity to
19001898
* compress a few records.
19011899
*/
1902-
if (!Insert->forcePageWrites)
1900+
if (Insert->runningBackups == 0)
19031901
NewPage->xlp_info |= XLP_BKP_REMOVABLE;
19041902

19051903
/*
@@ -8299,29 +8297,28 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
82998297
* written) copy of a database page if it reads the page concurrently with
83008298
* our write to the same page. This can be fixed as long as the first
83018299
* write to the page in the WAL sequence is a full-page write. Hence, we
8302-
* turn on forcePageWrites and then force a CHECKPOINT, to ensure there
8303-
* are no dirty pages in shared memory that might get dumped while the
8304-
* backup is in progress without having a corresponding WAL record. (Once
8305-
* the backup is complete, we need not force full-page writes anymore,
8306-
* since we expect that any pages not modified during the backup interval
8307-
* must have been correctly captured by the backup.)
8300+
* increment runningBackups then force a CHECKPOINT, to ensure there are
8301+
* no dirty pages in shared memory that might get dumped while the backup
8302+
* is in progress without having a corresponding WAL record. (Once the
8303+
* backup is complete, we need not force full-page writes anymore, since
8304+
* we expect that any pages not modified during the backup interval must
8305+
* have been correctly captured by the backup.)
83088306
*
8309-
* Note that forcePageWrites has no effect during an online backup from
8310-
* the standby.
8307+
* Note that forcing full-page writes has no effect during an online
8308+
* backup from the standby.
83118309
*
83128310
* We must hold all the insertion locks to change the value of
8313-
* forcePageWrites, to ensure adequate interlocking against
8311+
* runningBackups, to ensure adequate interlocking against
83148312
* XLogInsertRecord().
83158313
*/
83168314
WALInsertLockAcquireExclusive();
83178315
XLogCtl->Insert.runningBackups++;
8318-
XLogCtl->Insert.forcePageWrites = true;
83198316
WALInsertLockRelease();
83208317

83218318
/*
8322-
* Ensure we release forcePageWrites if fail below. NB -- for this to work
8323-
* correctly, it is critical that sessionBackupState is only updated after
8324-
* this block is over.
8319+
* Ensure we decrement runningBackups if we fail below. NB -- for this to
8320+
* work correctly, it is critical that sessionBackupState is only updated
8321+
* after this block is over.
83258322
*/
83268323
PG_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, DatumGetBool(true));
83278324
{
@@ -8593,10 +8590,10 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
85938590
errhint("wal_level must be set to \"replica\" or \"logical\" at server start.")));
85948591

85958592
/*
8596-
* OK to update backup counters, forcePageWrites, and session-level lock.
8593+
* OK to update backup counter and session-level lock.
85978594
*
8598-
* Note that CHECK_FOR_INTERRUPTS() must not occur while updating them.
8599-
* Otherwise they can be updated inconsistently, and which might cause
8595+
* Note that CHECK_FOR_INTERRUPTS() must not occur while updating them,
8596+
* otherwise they can be updated inconsistently, which might cause
86008597
* do_pg_abort_backup() to fail.
86018598
*/
86028599
WALInsertLockAcquireExclusive();
@@ -8608,11 +8605,6 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
86088605
Assert(XLogCtl->Insert.runningBackups > 0);
86098606
XLogCtl->Insert.runningBackups--;
86108607

8611-
if (XLogCtl->Insert.runningBackups == 0)
8612-
{
8613-
XLogCtl->Insert.forcePageWrites = false;
8614-
}
8615-
86168608
/*
86178609
* Clean up session-level lock.
86188610
*
@@ -8859,11 +8851,6 @@ do_pg_abort_backup(int code, Datum arg)
88598851
Assert(XLogCtl->Insert.runningBackups > 0);
88608852
XLogCtl->Insert.runningBackups--;
88618853

8862-
if (XLogCtl->Insert.runningBackups == 0)
8863-
{
8864-
XLogCtl->Insert.forcePageWrites = false;
8865-
}
8866-
88678854
sessionBackupState = SESSION_BACKUP_NONE;
88688855
WALInsertLockRelease();
88698856

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

Copy file name to clipboardExpand all lines: src/backend/access/transam/xloginsert.c
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -973,9 +973,9 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
973973
/*
974974
* Determine whether the buffer referenced has to be backed up.
975975
*
976-
* Since we don't yet have the insert lock, fullPageWrites and forcePageWrites
977-
* could change later, so the result should be used for optimization purposes
978-
* only.
976+
* Since we don't yet have the insert lock, fullPageWrites and runningBackups
977+
* (which forces full-page writes) could change later, so the result should
978+
* be used for optimization purposes only.
979979
*/
980980
bool
981981
XLogCheckBufferNeedsBackup(Buffer buffer)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.