sharpetronics/PostgreSQL
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-07 00:02:00 -04:00
Code Issues Projects Releases Wiki Activity

Get rid of XLogCtlInsert->forcePageWrites

Browse Source 
After commit 39969e2a1e4d, ->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
This commit is contained in:
Alvaro Herrera 2022-10-19 12:35:00 +02:00
parent c2ae01f695
commit 342bb38bfe
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
2 changed files with 27 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/backend/access/transam/xlog.c
View File

@ -276,8 +276,8 @@ XLogRecPtr XactLastCommitEnd = InvalidXLogRecPtr;
static XLogRecPtr RedoRecPtr; static XLogRecPtr RedoRecPtr;
/* /*
* doPageWrites is this backend's local copy of (forcePageWrites || * doPageWrites is this backend's local copy of (fullPageWrites ||
* fullPageWrites). It is used together with RedoRecPtr to decide whether * runningBackups > 0). It is used together with RedoRecPtr to decide whether
* a full-page image of a page need to be taken. * a full-page image of a page need to be taken.
* *
* NB: Initially this is false, and there's no guarantee that it will be * NB: Initially this is false, and there's no guarantee that it will be
@ -437,14 +437,12 @@ typedef struct XLogCtlInsert
* you must hold ALL the locks. * you must hold ALL the locks.
*/ */
XLogRecPtr RedoRecPtr; /* current redo point for insertions */ XLogRecPtr RedoRecPtr; /* current redo point for insertions */
bool forcePageWrites; /* forcing full-page writes for PITR? */
bool fullPageWrites; bool fullPageWrites;
/* /*
* runningBackups is a counter indicating the number of backups currently * runningBackups is a counter indicating the number of backups currently
* in progress. forcePageWrites is set to true when runningBackups is * in progress. lastBackupStart is the latest checkpoint redo location
* non-zero. lastBackupStart is the latest checkpoint redo location used * used as a starting point for an online backup.
* as a starting point for an online backup.
*/ */
int runningBackups; int runningBackups;
XLogRecPtr lastBackupStart; XLogRecPtr lastBackupStart;
@ -805,9 +803,9 @@ XLogInsertRecord(XLogRecData *rdata,
* happen just after a checkpoint, so it's better to be slow in this case * happen just after a checkpoint, so it's better to be slow in this case
* and fast otherwise. * and fast otherwise.
* *
* Also check to see if fullPageWrites or forcePageWrites was just turned * Also check to see if fullPageWrites was just turned on or there's a
* on; if we weren't already doing full-page writes then go back and * running backup (which forces full-page writes); if we weren't already
* recompute. * doing full-page writes then go back and recompute.
* *
* If we aren't doing full-page writes then RedoRecPtr doesn't actually * If we aren't doing full-page writes then RedoRecPtr doesn't actually
* affect the contents of the XLOG record, so we'll update our local copy * affect the contents of the XLOG record, so we'll update our local copy
@ -820,7 +818,7 @@ XLogInsertRecord(XLogRecData *rdata,
Assert(RedoRecPtr < Insert->RedoRecPtr); Assert(RedoRecPtr < Insert->RedoRecPtr);
RedoRecPtr = Insert->RedoRecPtr; RedoRecPtr = Insert->RedoRecPtr;
} }
doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites); doPageWrites = (Insert->fullPageWrites || Insert->runningBackups > 0);
if (doPageWrites && if (doPageWrites &&
(!prevDoPageWrites || (!prevDoPageWrites ||
@ -1899,7 +1897,7 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, TimeLineID tli, bool opportunistic)
* is unsafe, but at worst the archiver would miss the opportunity to * is unsafe, but at worst the archiver would miss the opportunity to
* compress a few records. * compress a few records.
*/ */
if (!Insert->forcePageWrites) if (Insert->runningBackups == 0)
NewPage->xlp_info |= XLP_BKP_REMOVABLE; NewPage->xlp_info |= XLP_BKP_REMOVABLE;
/* /*
@ -8299,29 +8297,28 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
* written) copy of a database page if it reads the page concurrently with * written) copy of a database page if it reads the page concurrently with
* our write to the same page. This can be fixed as long as the first * our write to the same page. This can be fixed as long as the first
* write to the page in the WAL sequence is a full-page write. Hence, we * write to the page in the WAL sequence is a full-page write. Hence, we
* turn on forcePageWrites and then force a CHECKPOINT, to ensure there * increment runningBackups then force a CHECKPOINT, to ensure there are
* are no dirty pages in shared memory that might get dumped while the * no dirty pages in shared memory that might get dumped while the backup
* backup is in progress without having a corresponding WAL record. (Once * is in progress without having a corresponding WAL record. (Once the
* the backup is complete, we need not force full-page writes anymore, * backup is complete, we need not force full-page writes anymore, since
* since we expect that any pages not modified during the backup interval * we expect that any pages not modified during the backup interval must
* must have been correctly captured by the backup.) * have been correctly captured by the backup.)
* *
* Note that forcePageWrites has no effect during an online backup from * Note that forcing full-page writes has no effect during an online
* the standby. * backup from the standby.
* *
* We must hold all the insertion locks to change the value of * We must hold all the insertion locks to change the value of
* forcePageWrites, to ensure adequate interlocking against * runningBackups, to ensure adequate interlocking against
* XLogInsertRecord(). * XLogInsertRecord().
*/ */
WALInsertLockAcquireExclusive(); WALInsertLockAcquireExclusive();
XLogCtl->Insert.runningBackups++; XLogCtl->Insert.runningBackups++;
XLogCtl->Insert.forcePageWrites = true;
WALInsertLockRelease(); WALInsertLockRelease();
/* /*
* Ensure we release forcePageWrites if fail below. NB -- for this to work * Ensure we decrement runningBackups if we fail below. NB -- for this to
* correctly, it is critical that sessionBackupState is only updated after * work correctly, it is critical that sessionBackupState is only updated
* this block is over. * after this block is over.
*/ */
PG_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, DatumGetBool(true)); PG_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, DatumGetBool(true));
{ {
@ -8593,10 +8590,10 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
errhint("wal_level must be set to \"replica\" or \"logical\" at server start."))); errhint("wal_level must be set to \"replica\" or \"logical\" at server start.")));
/* /*
* OK to update backup counters, forcePageWrites, and session-level lock. * OK to update backup counter and session-level lock.
* *
* Note that CHECK_FOR_INTERRUPTS() must not occur while updating them. * Note that CHECK_FOR_INTERRUPTS() must not occur while updating them,
* Otherwise they can be updated inconsistently, and which might cause * otherwise they can be updated inconsistently, which might cause
* do_pg_abort_backup() to fail. * do_pg_abort_backup() to fail.
*/ */
WALInsertLockAcquireExclusive(); WALInsertLockAcquireExclusive();
@ -8608,11 +8605,6 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
Assert(XLogCtl->Insert.runningBackups > 0); Assert(XLogCtl->Insert.runningBackups > 0);
XLogCtl->Insert.runningBackups--; XLogCtl->Insert.runningBackups--;
if (XLogCtl->Insert.runningBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
/* /*
* Clean up session-level lock. * Clean up session-level lock.
* *
@ -8859,11 +8851,6 @@ do_pg_abort_backup(int code, Datum arg)
Assert(XLogCtl->Insert.runningBackups > 0); Assert(XLogCtl->Insert.runningBackups > 0);
XLogCtl->Insert.runningBackups--; XLogCtl->Insert.runningBackups--;
if (XLogCtl->Insert.runningBackups == 0)
{
XLogCtl->Insert.forcePageWrites = false;
}
sessionBackupState = SESSION_BACKUP_NONE; sessionBackupState = SESSION_BACKUP_NONE;
WALInsertLockRelease(); WALInsertLockRelease();

6
src/backend/access/transam/xloginsert.c
View File

@ -973,9 +973,9 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
/* /*
* Determine whether the buffer referenced has to be backed up. * Determine whether the buffer referenced has to be backed up.
* *
* Since we don't yet have the insert lock, fullPageWrites and forcePageWrites * Since we don't yet have the insert lock, fullPageWrites and runningBackups
* could change later, so the result should be used for optimization purposes * (which forces full-page writes) could change later, so the result should
* only. * be used for optimization purposes only.
*/ */
bool bool
XLogCheckBufferNeedsBackup(Buffer buffer) XLogCheckBufferNeedsBackup(Buffer buffer)