From a8eb5d5249458a453ceb09f30861fb3195ab070e Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 11 Nov 2021 10:55:16 -0500 Subject: [PATCH v1 2/2] Remove InitXLOGAccess(). It's not great that RecoveryInProgress() calls InitXLOGAccess(), because a status inquiry function typically shouldn't have the side effect of performing initializations. We could fix that by calling InitXLOGAccess() from some other place, but instead, let's remove it altogether. One thing InitXLogAccess() did is initialize wal_segment_size, but it doesn't need to do that. In the postmaster, PostmasterMain() calls LocalProcessControlFile(), and all child processes will inherit that value -- except in EXEC_BACKEND bulds, but then each backend runs SubPostmasterMain() which also calls LocalProcessControlFile(). The other thing InitXLOGAccess() did is update RedoRecPtr and doPageWrites. That doesn't necessarily need to be done at all, because those values can always be inaccurate and all the code that uses them will just retry if it turns out that they've changed. However, doing it may avoid one retry, so make GetFullPageWritesInfo() do the work instead. The added cost of a branch in GetFullPageWritesInfo() is hopefully small enough to be negligible, especially since we're also saving something in RecoveryInProgress(). If not, we could also consider skipping these initializations altogether and letting the first attempt to insert WAL sort it out. That might result in, for example, one extra call to XLogRecordAssemble() over the lifetime of the backend, but that's probably not too bad. --- src/backend/access/transam/xlog.c | 61 ++++++++++------------------- src/backend/postmaster/auxprocess.c | 1 - src/backend/utils/init/postinit.c | 21 +++------- src/include/access/xlog.h | 1 - 4 files changed, 25 insertions(+), 59 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 355d1737c3..c718506bd9 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -352,8 +352,7 @@ XLogRecPtr XactLastCommitEnd = InvalidXLogRecPtr; * XLogCtl->Insert.RedoRecPtr, whenever we can safely do so (ie, when we * hold an insertion lock). See XLogInsertRecord for details. We are also * allowed to update from XLogCtl->RedoRecPtr if we hold the info_lck; - * see GetRedoRecPtr. A freshly spawned backend obtains the value during - * InitXLOGAccess. + * see GetRedoRecPtr. */ static XLogRecPtr RedoRecPtr; @@ -8418,23 +8417,6 @@ RecoveryInProgress(void) LocalRecoveryInProgress = (xlogctl->SharedRecoveryState != RECOVERY_STATE_DONE); - /* - * Initialize TimeLineID and RedoRecPtr when we discover that recovery - * is finished. InitPostgres() relies upon this behaviour to ensure - * that InitXLOGAccess() is called at backend startup. (If you change - * this, see also LocalSetXLogInsertAllowed.) - */ - if (!LocalRecoveryInProgress) - { - /* - * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryState (for machines with weak - * memory ordering). - */ - pg_memory_barrier(); - InitXLOGAccess(); - } - /* * Note: We don't need a memory barrier when we're still in recovery. * We might exit recovery immediately after return, so the caller @@ -8551,9 +8533,6 @@ LocalSetXLogInsertAllowed(void) LocalXLogInsertAllowed = 1; - /* Initialize as RecoveryInProgress() would do when switching state */ - InitXLOGAccess(); - return oldXLogAllowed; } @@ -8660,25 +8639,6 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, return record; } -/* - * This must be called in a backend process before creating WAL records - * (except in a standalone backend, which does StartupXLOG instead). We need - * to initialize the local copy of RedoRecPtr. - */ -void -InitXLOGAccess(void) -{ - XLogCtlInsert *Insert = &XLogCtl->Insert; - - /* set wal_segment_size */ - wal_segment_size = ControlFile->xlog_seg_size; - - /* Use GetRedoRecPtr to copy the RedoRecPtr safely */ - (void) GetRedoRecPtr(); - /* Also update our copy of doPageWrites. */ - doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites); -} - /* * Return the current Redo pointer from shared memory. * @@ -8716,6 +8676,25 @@ GetRedoRecPtr(void) void GetFullPageWriteInfo(XLogRecPtr *RedoRecPtr_p, bool *doPageWrites_p) { + /* + * As a special case, if RedoRecPtr is InvalidXLogRecPtr, then it's never + * been used for anything and the correct value must be different. In that + * case, get the current value from shared memory. + * + * Nothing really bad would happen if we didn't do this, because the + * caller can't rely on this information being accurate anyway, but it + * might, for example, result in an extra call to XLogRecordAssemble. + */ + if (unlikely(RedoRecPtr == InvalidXLogRecPtr)) + { + XLogCtlInsert *Insert = &XLogCtl->Insert; + + /* Use GetRedoRecPtr to copy the RedoRecPtr safely */ + (void) GetRedoRecPtr(); + /* Also update our copy of doPageWrites. */ + doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites); + } + *RedoRecPtr_p = RedoRecPtr; *doPageWrites_p = doPageWrites; } diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c index 7452f908b2..43497676ab 100644 --- a/src/backend/postmaster/auxprocess.c +++ b/src/backend/postmaster/auxprocess.c @@ -154,7 +154,6 @@ AuxiliaryProcessMain(AuxProcType auxtype) proc_exit(1); case WalWriterProcess: - InitXLOGAccess(); WalWriterMain(); proc_exit(1); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 0c56c38a14..e5d0852080 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -624,25 +624,14 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, } /* - * Initialize local process's access to XLOG. + * If this is either a bootstrap process nor a standalone backend, start + * up the XLOG machinery, and register to have it closed down at exit. + * In other cases, the startup process is responsible for starting up + * the XLOG machinery, and the checkpointer for closing it down. */ - if (IsUnderPostmaster) - { - /* - * The postmaster already started the XLOG machinery, but we need to - * call InitXLOGAccess(), if the system isn't in hot-standby mode. - * This is handled by calling RecoveryInProgress and ignoring the - * result. - */ - (void) RecoveryInProgress(); - } - else + if (!IsUnderPostmaster) { /* - * We are either a bootstrap process or a standalone backend. Either - * way, start up the XLOG machinery, and register to have it closed - * down at exit. - * * We don't yet have an aux-process resource owner, but StartupXLOG * and ShutdownXLOG will need one. Hence, create said resource owner * (and register a callback to clean it up after ShutdownXLOG runs). diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 898df2ee03..34f6c89f06 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -299,7 +299,6 @@ extern void BootStrapXLOG(void); extern void LocalProcessControlFile(bool reset); extern void StartupXLOG(void); extern void ShutdownXLOG(int code, Datum arg); -extern void InitXLOGAccess(void); extern void CreateCheckPoint(int flags); extern bool CreateRestartPoint(int flags); extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN); -- 2.24.3 (Apple Git-128)