On Tue, Oct 19, 2021 at 3:50 AM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Mon, Oct 18, 2021 at 9:54 AM Amul Sul <sula...@gmail.com> wrote:
> > I tried this in the attached version, but I'm a bit skeptical with
> > changes that are needed for CreateCheckPoint(), those don't seem to be
> > clean.
>
> Yeah, that doesn't look great. I don't think it's entirely correct,
> actually, because surely you want LocalXLogInsertAllowed = 0 to be
> executed even if !IsPostmasterEnvironment. It's only
> LocalXLogInsertAllowed = -1 that we would want to have depend on
> IsPostmasterEnvironment. But that's pretty ugly too: I guess the
> reason it has to be like is that, if it does that unconditionally, it
> will overwrite the temporary value of 1 set by the caller, which will
> then cause problems when the caller tries to XLogReportParameters().
>
> I think that problem goes away if we drive the decision off of shared
> state rather than a local variable, but I agree that it's otherwise a
> bit tricky to untangle. One idea might be to have
> LocalSetXLogInsertAllowed return the old value. Then we could use the
> same kind of coding we do when switching memory contexts, where we
> say:
>
> oldcontext = MemoryContextSwitchTo(something);
> // do stuff
> MemoryContextSwitchTo(oldcontext);
>
> Here we could maybe do:
>
> oldxlallowed = LocalSetXLogInsertAllowed();
> // do stuff
> XLogInsertAllowed = oldxlallowed;
>

Ok, did the same in the attached 0001 patch.

There is no harm in calling LocalSetXLogInsertAllowed() calling
multiple times, but the problem I can see is that with this patch user
is allowed to call LocalSetXLogInsertAllowed() at the time it is
supposed not to be called e.g. when LocalXLogInsertAllowed = 0;
WAL writes are explicitly disabled.

> That way, instead of CreateCheckPoint() knowing under what
> circumstances the caller might have changed the value, it only knows
> that some callers might have already changed the value. That seems
> better.
>
> > I agree that calling InitXLOGAccess() from RecoveryInProgress() is not
> > good, but I am not sure about calling it from XLogInsertAllowed()
> > either, perhaps, both are status check function and general
> > expectations might be that status checking functions are not going
> > change and/or initialize the system state. InitXLOGAccess() should
> > get called from the very first WAL write operation if needed, but if
> > we don't want to do that, then I would prefer to call InitXLOGAccess()
> > from XLogInsertAllowed() instead of RecoveryInProgress().
>
> Well, that's a fair point, too, but it might not be safe to, say, move
> this to XLogBeginInsert(). Like, imagine that there's a hypothetical
> piece of code that looks like this:
>
> if (RecoveryInProgress())
>     ereport(ERROR, errmsg("can't do that in recovery")));
>
> // do something here that depends on ThisTimeLineID or
> wal_segment_size or RedoRecPtr
>
> XLogBeginInsert();
> ....
> lsn = XLogInsert(...);
>
> Such code would work correctly the way things are today, but if the
> InitXLOGAccess() call were deferred until XLogBeginInsert() time, then
> it would fail.
>
> I was curious whether this is just a theoretical problem. It turns out
> that it's not. I wrote a couple of just-for-testing patches, which I
> attach here. The first one just adjusts things so that we'll fail an
> assertion if we try to make use of ThisTimeLineID before we've set it
> to a legal value. I had to exempt two places from these checks just
> for 'make check-world' to pass; these are shown in the patch, and one
> or both of them might be existing bugs -- or maybe not, I haven't
> looked too deeply. The second one then adjusts the patch to pretend
> that ThisTimeLineID is not necessarily valid just because we've called
> InitXLOGAccess() but that it is valid after XLogBeginInsert(). With
> that change, I find about a dozen places where, apparently, the early
> call to InitXLOGAccess() is critical to getting ThisTimeLineID
> adjusted in time. So apparently a change of this type is not entirely
> trivial. And this is just a quick test, and just for one of the three
> things that get initialized here.
>
> On the other hand, just moving it to XLogInsertAllowed() isn't
> risk-free either and would likely require adjusting some of the same
> places I found with this test. So I guess if we want to do something
> like this we need more study.
>

Yeah, that requires a lot of energy and time -- not done anything
related to this in the attached version.

Please have a look at the attached version where the 0001 patch does
change LocalSetXLogInsertAllowed() as said before. 0002 patch moves
XLogReportParameters() closer to other wal write operations and
removes unnecessary LocalSetXLogInsertAllowed() calls. 0003 is code
movements adds XLogAcceptWrites() function same as the before, and
0004 patch tries to remove the dependency. 0004 patch could change
w.r.t. decision that is going to be made for the patch that I
posted[1] to remove abortedRecPtr global variable. For now, I have
copied abortedRecPtr into shared memory. Thanks !

1] 
https://postgr.es/m/caaj_b94y75zwmim+gxxexvwf_yzo-dchof90ky0db2gstsp...@mail.gmail.com

Regards,
Amul
From 1173a55fab286cd1a8eb9ae3ae35fe9c2ad5cd0f Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Thu, 30 Sep 2021 06:29:06 -0400
Subject: [PATCH v39 4/4] Remove dependencies on startup-process specifical
 variables.

To make XLogAcceptWrites(), need to dependency on few global and local
variable spcific to startup process.

Global variables are abortedRecPtr, missingContrecPtr,
ArchiveRecoveryRequested and LocalPromoteIsTriggered, whereas
LocalPromoteIsTriggered can be accessed in any other process using
existing PromoteIsTriggered().  abortedRecPtr & ArchiveRecoveryRequested
is made accessible by copying into shared memory.  missingContrecPtr
can get from the existing shared memory values where it get stored
through EndOfLog if valid.

XLogAcceptWrites() accepts two argument as EndOfLogTLI and EndOfLog
which are local to StartupXLOG(). Instead of passing as an argument
XLogCtl->replayEndTLI and XLogCtl->lastSegSwitchLSN from the shared
memory can be used as an replacement to EndOfLogTLI and EndOfLog
respectively.  XLogCtl->lastSegSwitchLSN is not going to change until
we use it. That changes only when the current WAL segment gets full
which never going to happen because of two reasons, first WAL writes
are disabled for other processes until XLogAcceptWrites() finishes and
other reasons before use of lastSegSwitchLSN, XLogAcceptWrites() is
writes fix size wal records as full-page write and record for either
recovery end or checkpoint which not going to fill up the 16MB wal
segment.

EndOfLogTLI in the StartupXLOG() is the timeline ID of the last record
that xlogreader reads, but this xlogreader was simply re-fetching the
last record which we have replied in redo loop if it was in recovery,
if not in recovery, we don't need to worry since this value is needed
only in case of ArchiveRecoveryRequested = true, which implicitly
forces redo and sets XLogCtl->replayEndTLI value.
---
 src/backend/access/transam/xlog.c | 83 ++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e5375cd1bb5..5b12addc228 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -668,6 +668,13 @@ typedef struct XLogCtlData
 	 */
 	bool		SharedPromoteIsTriggered;
 
+	/*
+	 * SharedArchiveRecoveryRequested exports the value of the
+	 * ArchiveRecoveryRequested flag to be share which is otherwise valid only
+	 * in the startup process.
+	 */
+	bool		SharedArchiveRecoveryRequested;
+
 	/*
 	 * WalWriterSleeping indicates whether the WAL writer is currently in
 	 * low-power mode (and hence should be nudged if an async commit occurs).
@@ -717,6 +724,13 @@ typedef struct XLogCtlData
 	/* timestamp of last COMMIT/ABORT record replayed (or being replayed) */
 	TimestampTz recoveryLastXTime;
 
+	/*
+	 * SharedAbortedRecPtr exports abortedRecPtr to be shared with another
+	 * process to write OVERWRITE_CONTRECORD message, if WAL writes are not
+	 * permitted in the current process which reads that.
+	 */
+	XLogRecPtr	SharedAbortedRecPtr;
+
 	/*
 	 * timestamp of when we started replaying the current chunk of WAL data,
 	 * only relevant for replication or archive recovery
@@ -889,8 +903,7 @@ static MemoryContext walDebugCxt = NULL;
 static void readRecoverySignalFile(void);
 static void validateRecoveryParameters(void);
 static void exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog);
-static void CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI,
-										XLogRecPtr EndOfLog);
+static void CleanupAfterArchiveRecovery(void);
 static bool recoveryStopsBefore(XLogReaderState *record);
 static bool recoveryStopsAfter(XLogReaderState *record);
 static char *getRecoveryStopReason(void);
@@ -939,7 +952,7 @@ static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force);
 static XLogRecord *ReadRecord(XLogReaderState *xlogreader,
 							  int emode, bool fetching_ckpt);
 static void CheckRecoveryConsistency(void);
-static bool XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog);
+static bool XLogAcceptWrites(void);
 static bool PerformRecoveryXLogAction(void);
 static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader,
 										XLogRecPtr RecPtr, int whichChkpt, bool report);
@@ -5267,7 +5280,9 @@ XLOGShmemInit(void)
 	XLogCtl->SharedHotStandbyActive = false;
 	XLogCtl->InstallXLogFileSegmentActive = false;
 	XLogCtl->SharedPromoteIsTriggered = false;
+	XLogCtl->SharedArchiveRecoveryRequested = false;
 	XLogCtl->WalWriterSleeping = false;
+	XLogCtl->SharedAbortedRecPtr = InvalidXLogRecPtr;
 
 	SpinLockInit(&XLogCtl->Insert.insertpos_lck);
 	SpinLockInit(&XLogCtl->info_lck);
@@ -5548,6 +5563,11 @@ readRecoverySignalFile(void)
 		ereport(FATAL,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("standby mode is not supported by single-user servers")));
+
+	/*
+	 * Remember archive recovery request in shared memory state.
+	 */
+	XLogCtl->SharedArchiveRecoveryRequested = ArchiveRecoveryRequested;
 }
 
 static void
@@ -5739,8 +5759,10 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
  * Perform cleanup actions at the conclusion of archive recovery.
  */
 static void
-CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
+CleanupAfterArchiveRecovery(void)
 {
+	XLogRecPtr	EndOfLog;
+
 	/*
 	 * Execute the recovery_end_command, if any.
 	 */
@@ -5757,6 +5779,7 @@ CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
 	 * files containing garbage. In any case, they are not part of the new
 	 * timeline's history so we don't need them.
 	 */
+	(void) GetLastSegSwitchData(&EndOfLog);
 	RemoveNonParentXlogFiles(EndOfLog, ThisTimeLineID);
 
 	/*
@@ -5791,6 +5814,7 @@ CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
 	{
 		char		origfname[MAXFNAMELEN];
 		XLogSegNo	endLogSegNo;
+		TimeLineID	EndOfLogTLI = XLogCtl->replayEndTLI;
 
 		XLByteToPrevSeg(EndOfLog, endLogSegNo, wal_segment_size);
 		XLogFileName(origfname, EndOfLogTLI, endLogSegNo, wal_segment_size);
@@ -7965,6 +7989,16 @@ StartupXLOG(void)
 	{
 		Assert(!XLogRecPtrIsInvalid(abortedRecPtr));
 		EndOfLog = missingContrecPtr;
+
+		/*
+		 * Remember broken record pointer in shared memory state. This process
+		 * might unable to write an OVERWRITE_CONTRECORD message because of WAL
+		 * write restriction.  Storing in shared memory helps that get written
+		 * later by another process as soon as WAL writing is enabled.
+		 */
+		XLogCtl->SharedAbortedRecPtr = abortedRecPtr;
+		abortedRecPtr = InvalidXLogRecPtr;
+		missingContrecPtr = InvalidXLogRecPtr;
 	}
 
 	/*
@@ -8063,8 +8097,15 @@ StartupXLOG(void)
 	}
 	XLogReaderFree(xlogreader);
 
+	/*
+	 * Update full_page_writes in shared memory, and later whenever wal write
+	 * permitted, write an XLOG_FPW_CHANGE record before resource manager
+	 * writes cleanup WAL records or checkpoint record is written.
+	 */
+	Insert->fullPageWrites = lastFullPageWrites;
+
 	/* Prepare to accept WAL writes. */
-	promoted = XLogAcceptWrites(EndOfLogTLI, EndOfLog);
+	promoted = XLogAcceptWrites();
 
 	/*
 	 * All done with end-of-recovery actions.
@@ -8124,7 +8165,7 @@ StartupXLOG(void)
  * Prepare to accept WAL writes.
  */
 static bool
-XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
+XLogAcceptWrites(void)
 {
 	bool		promoted = false;
 	int			oldXLogAllowed;
@@ -8132,20 +8173,24 @@ XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
 	oldXLogAllowed = LocalSetXLogInsertAllowed();
 
 	/* If necessary, write overwrite-contrecord before doing anything else */
-	if (!XLogRecPtrIsInvalid(abortedRecPtr))
+	if (!XLogRecPtrIsInvalid(XLogCtl->SharedAbortedRecPtr))
 	{
+		/*
+		 * Restore missingContrecPtr, needed to set
+		 * XLP_FIRST_IS_OVERWRITE_CONTRECORD flag on the page header where
+		 * overwrite-contrecord get written. See AdvanceXLInsertBuffer().
+		 *
+		 * NB: We can safely use lastSegSwitchLSN to restore missingContrecPtr,
+		 * which is never going to change until we reach here since there wasn't
+		 * any wal write before.
+		 */
+		GetLastSegSwitchData(&missingContrecPtr);
 		Assert(!XLogRecPtrIsInvalid(missingContrecPtr));
-		CreateOverwriteContrecordRecord(abortedRecPtr);
-		abortedRecPtr = InvalidXLogRecPtr;
-		missingContrecPtr = InvalidXLogRecPtr;
+		CreateOverwriteContrecordRecord(XLogCtl->SharedAbortedRecPtr);
+		XLogCtl->SharedAbortedRecPtr = InvalidXLogRecPtr;
 	}
 
-	/*
-	 * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE
-	 * record before resource manager writes cleanup WAL records or checkpoint
-	 * record is written.
-	 */
-	Insert->fullPageWrites = lastFullPageWrites;
+	/* Write an XLOG_FPW_CHANGE record */
 	UpdateFullPageWrites();
 
 	/*
@@ -8169,7 +8214,7 @@ XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
 
 	/* If this is archive recovery, perform post-recovery cleanup actions. */
 	if (ArchiveRecoveryRequested)
-		CleanupAfterArchiveRecovery(EndOfLogTLI, EndOfLog);
+		CleanupAfterArchiveRecovery();
 
 	/*
 	 * Local WAL inserts enabled, so it's time to finish initialization of
@@ -8304,8 +8349,8 @@ PerformRecoveryXLogAction(void)
 	 * a full checkpoint. A checkpoint is requested later, after we're fully out
 	 * of recovery mode and already accepting queries.
 	 */
-	if (ArchiveRecoveryRequested && IsUnderPostmaster &&
-		LocalPromoteIsTriggered)
+	if (XLogCtl->SharedArchiveRecoveryRequested && IsUnderPostmaster &&
+		PromoteIsTriggered())
 	{
 		promoted = true;
 
-- 
2.18.0

From fc9ff858b2a56c6a75f56d88562d8cab821687db Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Oct 2021 09:08:04 -0400
Subject: [PATCH v39 1/4] Changed LocalSetXLogInsertAllowed() to return
 previous value

---
 src/backend/access/transam/xlog.c | 32 +++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 62862255fca..23a3f35e1fe 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -905,7 +905,7 @@ static void checkTimeLineSwitch(XLogRecPtr lsn, TimeLineID newTLI,
 								TimeLineID prevTLI);
 static void VerifyOverwriteContrecord(xl_overwrite_contrecord *xlrec,
 									  XLogReaderState *state);
-static void LocalSetXLogInsertAllowed(void);
+static int LocalSetXLogInsertAllowed(void);
 static void CreateEndOfRecoveryRecord(void);
 static XLogRecPtr CreateOverwriteContrecordRecord(XLogRecPtr aborted_lsn);
 static void CheckPointGuts(XLogRecPtr checkPointRedo, int flags);
@@ -6635,6 +6635,7 @@ StartupXLOG(void)
 	XLogReaderState *xlogreader;
 	XLogPageReadPrivate private;
 	bool		promoted = false;
+	int			oldXLogAllowed;
 	struct stat st;
 
 	/*
@@ -8062,7 +8063,7 @@ StartupXLOG(void)
 	}
 	XLogReaderFree(xlogreader);
 
-	LocalSetXLogInsertAllowed();
+	oldXLogAllowed = LocalSetXLogInsertAllowed();
 
 	/* If necessary, write overwrite-contrecord before doing anything else */
 	if (!XLogRecPtrIsInvalid(abortedRecPtr))
@@ -8080,7 +8081,7 @@ StartupXLOG(void)
 	 */
 	Insert->fullPageWrites = lastFullPageWrites;
 	UpdateFullPageWrites();
-	LocalXLogInsertAllowed = -1;
+	LocalXLogInsertAllowed = oldXLogAllowed;
 
 	/*
 	 * Emit checkpoint or end-of-recovery record in XLOG, if required.
@@ -8102,7 +8103,7 @@ StartupXLOG(void)
 	 * If any of the critical GUCs have changed, log them before we allow
 	 * backends to write WAL.
 	 */
-	LocalSetXLogInsertAllowed();
+	(void) LocalSetXLogInsertAllowed();
 	XLogReportParameters();
 
 	/*
@@ -8463,19 +8464,20 @@ XLogInsertAllowed(void)
 }
 
 /*
- * Make XLogInsertAllowed() return true in the current process only.
- *
- * Note: it is allowed to switch LocalXLogInsertAllowed back to -1 later,
- * and even call LocalSetXLogInsertAllowed() again after that.
+ * Sets LocalXLogInsertAllowed to make XLogInsertAllowed() return true in the
+ * current process only and returns previous LocalXLogInsertAllowed value.
  */
-static void
+static int
 LocalSetXLogInsertAllowed(void)
 {
-	Assert(LocalXLogInsertAllowed == -1);
+	int		oldXLogAllowed = LocalXLogInsertAllowed;
+
 	LocalXLogInsertAllowed = 1;
 
 	/* Initialize as RecoveryInProgress() would do when switching state */
 	InitXLOGAccess();
+
+	return oldXLogAllowed;
 }
 
 /*
@@ -9020,6 +9022,7 @@ CreateCheckPoint(int flags)
 	XLogRecPtr	last_important_lsn;
 	VirtualTransactionId *vxids;
 	int			nvxids;
+	int			oldXLogAllowed;
 
 	/*
 	 * An end-of-recovery checkpoint is really a shutdown checkpoint, just
@@ -9127,7 +9130,7 @@ CreateCheckPoint(int flags)
 	 * initialized, which we need here and in AdvanceXLInsertBuffer.)
 	 */
 	if (flags & CHECKPOINT_END_OF_RECOVERY)
-		LocalSetXLogInsertAllowed();
+		oldXLogAllowed = LocalSetXLogInsertAllowed();
 
 	checkPoint.ThisTimeLineID = ThisTimeLineID;
 	if (flags & CHECKPOINT_END_OF_RECOVERY)
@@ -9307,7 +9310,7 @@ CreateCheckPoint(int flags)
 	if (shutdown)
 	{
 		if (flags & CHECKPOINT_END_OF_RECOVERY)
-			LocalXLogInsertAllowed = -1;	/* return to "check" state */
+			LocalXLogInsertAllowed = oldXLogAllowed;
 		else
 			LocalXLogInsertAllowed = 0; /* never again write WAL */
 	}
@@ -9435,6 +9438,7 @@ CreateEndOfRecoveryRecord(void)
 {
 	xl_end_of_recovery xlrec;
 	XLogRecPtr	recptr;
+	int			oldXLogAllowed;
 
 	/* sanity check */
 	if (!RecoveryInProgress())
@@ -9447,7 +9451,7 @@ CreateEndOfRecoveryRecord(void)
 	xlrec.PrevTimeLineID = XLogCtl->PrevTimeLineID;
 	WALInsertLockRelease();
 
-	LocalSetXLogInsertAllowed();
+	oldXLogAllowed = LocalSetXLogInsertAllowed();
 
 	START_CRIT_SECTION();
 
@@ -9470,7 +9474,7 @@ CreateEndOfRecoveryRecord(void)
 
 	END_CRIT_SECTION();
 
-	LocalXLogInsertAllowed = -1;	/* return to "check" state */
+	LocalXLogInsertAllowed = oldXLogAllowed;
 }
 
 /*
-- 
2.18.0

From 254763823f76ceba15610eb745372cae2a50878a Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Oct 2021 09:17:34 -0400
Subject: [PATCH v39 2/4] Minimize LocalSetXLogInsertAllowed() calls by moving
 XLogReportParameters().

Also, remove LocalSetXLogInsertAllowed() call from
CreateEndOfRecoveryRecord() which is not longer need since we not
resetting LocalXLogInsertAllowed it.
---
 src/backend/access/transam/xlog.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 23a3f35e1fe..d58b0ce0c71 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8081,7 +8081,6 @@ StartupXLOG(void)
 	 */
 	Insert->fullPageWrites = lastFullPageWrites;
 	UpdateFullPageWrites();
-	LocalXLogInsertAllowed = oldXLogAllowed;
 
 	/*
 	 * Emit checkpoint or end-of-recovery record in XLOG, if required.
@@ -8095,16 +8094,16 @@ StartupXLOG(void)
 	if (!XLogRecPtrIsInvalid(XLogCtl->lastReplayedEndRecPtr))
 		promoted = PerformRecoveryXLogAction();
 
-	/* If this is archive recovery, perform post-recovery cleanup actions. */
-	if (ArchiveRecoveryRequested)
-		CleanupAfterArchiveRecovery(EndOfLogTLI, EndOfLog);
-
 	/*
 	 * If any of the critical GUCs have changed, log them before we allow
 	 * backends to write WAL.
 	 */
-	(void) LocalSetXLogInsertAllowed();
 	XLogReportParameters();
+	LocalXLogInsertAllowed = oldXLogAllowed;
+
+	/* If this is archive recovery, perform post-recovery cleanup actions. */
+	if (ArchiveRecoveryRequested)
+		CleanupAfterArchiveRecovery(EndOfLogTLI, EndOfLog);
 
 	/*
 	 * Local WAL inserts enabled, so it's time to finish initialization of
@@ -9438,7 +9437,6 @@ CreateEndOfRecoveryRecord(void)
 {
 	xl_end_of_recovery xlrec;
 	XLogRecPtr	recptr;
-	int			oldXLogAllowed;
 
 	/* sanity check */
 	if (!RecoveryInProgress())
@@ -9451,8 +9449,6 @@ CreateEndOfRecoveryRecord(void)
 	xlrec.PrevTimeLineID = XLogCtl->PrevTimeLineID;
 	WALInsertLockRelease();
 
-	oldXLogAllowed = LocalSetXLogInsertAllowed();
-
 	START_CRIT_SECTION();
 
 	XLogBeginInsert();
@@ -9473,8 +9469,6 @@ CreateEndOfRecoveryRecord(void)
 	LWLockRelease(ControlFileLock);
 
 	END_CRIT_SECTION();
-
-	LocalXLogInsertAllowed = oldXLogAllowed;
 }
 
 /*
-- 
2.18.0

From 5cbffa95285a8e3ce272afb6eca8e34b71f56169 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Mon, 4 Oct 2021 00:44:31 -0400
Subject: [PATCH v39 3/4] Create XLogAcceptWrites() function with code from
 StartupXLOG().

This is just code movement. A future patch will want to defer the
call to XLogAcceptWrites() until a later time, rather than doing
it as soon as we finish applying WAL, but here we're just grouping
related code together into a new function.

Robert Haas, with modifications by Amul Sul.
---
 src/backend/access/transam/xlog.c | 111 +++++++++++++++++-------------
 1 file changed, 63 insertions(+), 48 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index d58b0ce0c71..e5375cd1bb5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -939,6 +939,7 @@ static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force);
 static XLogRecord *ReadRecord(XLogReaderState *xlogreader,
 							  int emode, bool fetching_ckpt);
 static void CheckRecoveryConsistency(void);
+static bool XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog);
 static bool PerformRecoveryXLogAction(void);
 static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader,
 										XLogRecPtr RecPtr, int whichChkpt, bool report);
@@ -6635,7 +6636,6 @@ StartupXLOG(void)
 	XLogReaderState *xlogreader;
 	XLogPageReadPrivate private;
 	bool		promoted = false;
-	int			oldXLogAllowed;
 	struct stat st;
 
 	/*
@@ -8063,53 +8063,8 @@ StartupXLOG(void)
 	}
 	XLogReaderFree(xlogreader);
 
-	oldXLogAllowed = LocalSetXLogInsertAllowed();
-
-	/* If necessary, write overwrite-contrecord before doing anything else */
-	if (!XLogRecPtrIsInvalid(abortedRecPtr))
-	{
-		Assert(!XLogRecPtrIsInvalid(missingContrecPtr));
-		CreateOverwriteContrecordRecord(abortedRecPtr);
-		abortedRecPtr = InvalidXLogRecPtr;
-		missingContrecPtr = InvalidXLogRecPtr;
-	}
-
-	/*
-	 * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE
-	 * record before resource manager writes cleanup WAL records or checkpoint
-	 * record is written.
-	 */
-	Insert->fullPageWrites = lastFullPageWrites;
-	UpdateFullPageWrites();
-
-	/*
-	 * Emit checkpoint or end-of-recovery record in XLOG, if required.
-	 *
-	 * XLogCtl->lastReplayedEndRecPtr will be a valid LSN if and only if we
-	 * entered recovery. Even if we ultimately replayed no WAL records, it will
-	 * have been initialized based on where replay was due to start.  We don't
-	 * need a lock to access this, since this can't change any more by the time
-	 * we reach this code.
-	 */
-	if (!XLogRecPtrIsInvalid(XLogCtl->lastReplayedEndRecPtr))
-		promoted = PerformRecoveryXLogAction();
-
-	/*
-	 * If any of the critical GUCs have changed, log them before we allow
-	 * backends to write WAL.
-	 */
-	XLogReportParameters();
-	LocalXLogInsertAllowed = oldXLogAllowed;
-
-	/* If this is archive recovery, perform post-recovery cleanup actions. */
-	if (ArchiveRecoveryRequested)
-		CleanupAfterArchiveRecovery(EndOfLogTLI, EndOfLog);
-
-	/*
-	 * Local WAL inserts enabled, so it's time to finish initialization of
-	 * commit timestamp.
-	 */
-	CompleteCommitTsInitialization();
+	/* Prepare to accept WAL writes. */
+	promoted = XLogAcceptWrites(EndOfLogTLI, EndOfLog);
 
 	/*
 	 * All done with end-of-recovery actions.
@@ -8165,6 +8120,66 @@ StartupXLOG(void)
 		RequestCheckpoint(CHECKPOINT_FORCE);
 }
 
+/*
+ * Prepare to accept WAL writes.
+ */
+static bool
+XLogAcceptWrites(TimeLineID EndOfLogTLI, XLogRecPtr EndOfLog)
+{
+	bool		promoted = false;
+	int			oldXLogAllowed;
+
+	oldXLogAllowed = LocalSetXLogInsertAllowed();
+
+	/* If necessary, write overwrite-contrecord before doing anything else */
+	if (!XLogRecPtrIsInvalid(abortedRecPtr))
+	{
+		Assert(!XLogRecPtrIsInvalid(missingContrecPtr));
+		CreateOverwriteContrecordRecord(abortedRecPtr);
+		abortedRecPtr = InvalidXLogRecPtr;
+		missingContrecPtr = InvalidXLogRecPtr;
+	}
+
+	/*
+	 * Update full_page_writes in shared memory and write an XLOG_FPW_CHANGE
+	 * record before resource manager writes cleanup WAL records or checkpoint
+	 * record is written.
+	 */
+	Insert->fullPageWrites = lastFullPageWrites;
+	UpdateFullPageWrites();
+
+	/*
+	 * Emit checkpoint or end-of-recovery record in XLOG, if required.
+	 *
+	 * XLogCtl->lastReplayedEndRecPtr will be a valid LSN if and only if we
+	 * entered recovery. Even if we ultimately replayed no WAL records, it will
+	 * have been initialized based on where replay was due to start.  We don't
+	 * need a lock to access this, since this can't change any more by the time
+	 * we reach this code.
+	 */
+	if (!XLogRecPtrIsInvalid(XLogCtl->lastReplayedEndRecPtr))
+		promoted = PerformRecoveryXLogAction();
+
+	/*
+	 * If any of the critical GUCs have changed, log them before we allow
+	 * backends to write WAL.
+	 */
+	XLogReportParameters();
+	LocalXLogInsertAllowed = oldXLogAllowed;
+
+	/* If this is archive recovery, perform post-recovery cleanup actions. */
+	if (ArchiveRecoveryRequested)
+		CleanupAfterArchiveRecovery(EndOfLogTLI, EndOfLog);
+
+	/*
+	 * Local WAL inserts enabled, so it's time to finish initialization of
+	 * commit timestamp.
+	 */
+	CompleteCommitTsInitialization();
+
+	return promoted;
+}
+
 /*
  * Checks if recovery has reached a consistent state. When consistency is
  * reached and we have a valid starting standby snapshot, tell postmaster
-- 
2.18.0

Reply via email to