Hi,
On Thu, Oct 7, 2021 at 6:57 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
>
> On 2021-Oct-07, Amul Sul wrote:
>
> > Make sense, thanks for the explanation.
>
> You're welcome. Also, I forgot: thank you for taking the time to review
> the code. Much appreciated.
:)
>
>
I have one more question, regarding the need for other global
variables i.e. abortedRecPtr. (Sorry for coming back after so long.)
Instead of abortedRecPtr point, isn't enough to write
overwrite-contrecord at XLogCtl->lastReplayedEndRecPtr? I think both
are pointing to the same location then can't we use
lastReplayedEndRecPtr instead of abortedRecPtr to write
overwrite-contrecord and remove need of extra global variable, like
attached?
You might wonder why I am so concerned about the global variable. The
reason is that I am working on another thread[1] where we are trying
to club all the WAL write operations that happen at the end of
StartupXLOG into a separate function. In the future, we might want to
allow executing this function from other processes (e.g.
Checkpointer). For that, we need to remove the dependency of those WAL
write operations having on the global variables which are mostly valid
in the startup process. The easiest way to do that is simply copy all
the global variables into shared memory but that will not be an
optimised solution, the goal is to try to see if we could leverage the existing
information available into shared memory. I would be grateful if you
could share your thoughts on the same, thanks.
Regards,
Amul
1]
https://postgr.es/m/caaj_b97kzzdjsffwrk7w0xu5hnxkcgkgtr69t8cozztsyxj...@mail.gmail.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 26dcc00ac01..52b93cd80cc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -200,12 +200,11 @@ static XLogRecPtr flushedUpto = 0;
static TimeLineID receiveTLI = 0;
/*
- * abortedRecPtr is the start pointer of a broken record at end of WAL when
- * recovery completes; missingContrecPtr is the location of the first
- * contrecord that went missing. See CreateOverwriteContrecordRecord for
- * details.
+ * missingContrecPtr is the location of the first contrecord that went missing.
+ * XLogCtl->lastReplayedEndRecPtr will be the start pointer of a broken record
+ * at end of WAL when recovery completes. See CreateOverwriteContrecordRecord
+ * for details.
*/
-static XLogRecPtr abortedRecPtr;
static XLogRecPtr missingContrecPtr;
/*
@@ -4425,11 +4424,8 @@ ReadRecord(XLogReaderState *xlogreader, int emode,
* that portion is to be ignored.
*/
if (!StandbyMode &&
- !XLogRecPtrIsInvalid(xlogreader->abortedRecPtr))
- {
- abortedRecPtr = xlogreader->abortedRecPtr;
+ !XLogRecPtrIsInvalid(xlogreader->missingContrecPtr))
missingContrecPtr = xlogreader->missingContrecPtr;
- }
if (readFile >= 0)
{
@@ -7109,7 +7105,6 @@ StartupXLOG(void)
/*
* Start recovery assuming that the final record isn't lost.
*/
- abortedRecPtr = InvalidXLogRecPtr;
missingContrecPtr = InvalidXLogRecPtr;
/* REDO */
@@ -7876,10 +7871,7 @@ StartupXLOG(void)
* we'll do as soon as we're open for writing new WAL.)
*/
if (!XLogRecPtrIsInvalid(missingContrecPtr))
- {
- Assert(!XLogRecPtrIsInvalid(abortedRecPtr));
EndOfLog = missingContrecPtr;
- }
/*
* Prepare to write WAL starting at EndOfLog location, and init xlog
@@ -7936,11 +7928,10 @@ StartupXLOG(void)
LocalSetXLogInsertAllowed();
/* If necessary, write overwrite-contrecord before doing anything else */
- if (!XLogRecPtrIsInvalid(abortedRecPtr))
+ if (!XLogRecPtrIsInvalid(missingContrecPtr))
{
- Assert(!XLogRecPtrIsInvalid(missingContrecPtr));
- CreateOverwriteContrecordRecord(abortedRecPtr);
- abortedRecPtr = InvalidXLogRecPtr;
+ Assert(!XLogRecPtrIsInvalid(XLogCtl->lastReplayedEndRecPtr));
+ CreateOverwriteContrecordRecord(XLogCtl->lastReplayedEndRecPtr);
missingContrecPtr = InvalidXLogRecPtr;
}
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 4b03577dccd..516124ecd5f 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -294,7 +294,6 @@ XLogReadRecord(XLogReaderState *state, char **errormsg)
state->errormsg_buf[0] = '\0';
ResetDecoder(state);
- state->abortedRecPtr = InvalidXLogRecPtr;
state->missingContrecPtr = InvalidXLogRecPtr;
RecPtr = state->EndRecPtr;
@@ -586,7 +585,6 @@ err:
* in turn signal downstream WAL consumers that the broken WAL record
* is to be ignored.
*/
- state->abortedRecPtr = RecPtr;
state->missingContrecPtr = targetPagePtr;
}
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index de6fd791fe6..d876e25de7f 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -176,11 +176,9 @@ struct XLogReaderState
XLogRecPtr EndRecPtr; /* end+1 of last record read */
/*
- * Set at the end of recovery: the start point of a partial record at the
- * end of WAL (InvalidXLogRecPtr if there wasn't one), and the start
- * location of its first contrecord that went missing.
+ * Set at the end of recovery: the start location of its first contrecord
+ * that went missing.
*/
- XLogRecPtr abortedRecPtr;
XLogRecPtr missingContrecPtr;
/* Set when XLP_FIRST_IS_OVERWRITE_CONTRECORD is found */
XLogRecPtr overwrittenRecPtr;
