On Fri, Nov 26, 2021 at 12:16 PM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Thu, Nov 25, 2021 at 04:04:23PM +0900, Michael Paquier wrote:
> > I have not check the performance implication of that with a micro
> > benchmark or the like, but I can get behind 0001 on consistency
> > grounds between the backend and the frontend.
>
> /* Now create pg_control */
> InitControlFile(sysidentifier);
> - ControlFile->time = checkPoint.time;
> ControlFile->checkPoint = checkPoint.redo;
> ControlFile->checkPointCopy = checkPoint;
> 0001 has a mistake here, no? The initial control file creation goes
> through WriteControlFile(), and not update_controlfile(), so this
> change means that we would miss setting up this timestamp for the
> first time.
>
> @@ -714,7 +714,6 @@ GuessControlValues(void)
> ControlFile.checkPointCopy.oldestActiveXid = InvalidTransactionId;
>
> ControlFile.state = DB_SHUTDOWNED;
> - ControlFile.time = (pg_time_t) time(NULL);
> This one had better not be removed, either, as we require pg_resetwal
> to guess a set of control file values. Removing the one in
> RewriteControlFile() is fine, on the contrary.
My bad, sorry for the sloppy change, corrected it in the attached version.
Regards,
Amul
From a2c385f6a6152dbba1e33149f1d7f102243ed0cd Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Thu, 23 Sep 2021 00:47:52 -0400
Subject: [PATCH v6] Do the ControlFile timestamp setting in
update_controlfile.
---
src/backend/access/transam/xlog.c | 8 +-------
src/bin/pg_resetwal/pg_resetwal.c | 1 -
src/common/controldata_utils.c | 4 ++++
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b54ec549705..b980c6ac21c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7339,7 +7339,7 @@ StartupXLOG(void)
ControlFile->backupEndPoint = ControlFile->minRecoveryPoint;
}
}
- ControlFile->time = (pg_time_t) time(NULL);
+
/* No need to hold ControlFileLock yet, we aren't up far enough */
UpdateControlFile();
@@ -8199,7 +8199,6 @@ StartupXLOG(void)
*/
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
ControlFile->state = DB_IN_PRODUCTION;
- ControlFile->time = (pg_time_t) time(NULL);
SpinLockAcquire(&XLogCtl->info_lck);
XLogCtl->SharedRecoveryState = RECOVERY_STATE_DONE;
@@ -9142,7 +9141,6 @@ CreateCheckPoint(int flags)
{
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
ControlFile->state = DB_SHUTDOWNING;
- ControlFile->time = (pg_time_t) time(NULL);
UpdateControlFile();
LWLockRelease(ControlFileLock);
}
@@ -9412,7 +9410,6 @@ CreateCheckPoint(int flags)
ControlFile->state = DB_SHUTDOWNED;
ControlFile->checkPoint = ProcLastRecPtr;
ControlFile->checkPointCopy = checkPoint;
- ControlFile->time = (pg_time_t) time(NULL);
/* crash recovery should always recover to the end of WAL */
ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
ControlFile->minRecoveryPointTLI = 0;
@@ -9539,7 +9536,6 @@ CreateEndOfRecoveryRecord(void)
* changes to this point.
*/
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- ControlFile->time = (pg_time_t) time(NULL);
ControlFile->minRecoveryPoint = recptr;
ControlFile->minRecoveryPointTLI = xlrec.ThisTimeLineID;
UpdateControlFile();
@@ -9740,7 +9736,6 @@ CreateRestartPoint(int flags)
{
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- ControlFile->time = (pg_time_t) time(NULL);
UpdateControlFile();
LWLockRelease(ControlFileLock);
}
@@ -9801,7 +9796,6 @@ CreateRestartPoint(int flags)
{
ControlFile->checkPoint = lastCheckPointRecPtr;
ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
/*
* Ensure minRecoveryPoint is past the checkpoint record. Normally,
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index 84b6f70ad6a..c0ab392c3a2 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -911,7 +911,6 @@ RewriteControlFile(void)
ControlFile.checkPointCopy.time = (pg_time_t) time(NULL);
ControlFile.state = DB_SHUTDOWNED;
- ControlFile.time = (pg_time_t) time(NULL);
ControlFile.checkPoint = ControlFile.checkPointCopy.redo;
ControlFile.minRecoveryPoint = 0;
ControlFile.minRecoveryPointTLI = 0;
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 7d4af7881ea..4ce097c05ce 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -23,6 +23,7 @@
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
+#include <time.h>
#include "access/xlog_internal.h"
#include "catalog/pg_control.h"
@@ -168,6 +169,9 @@ update_controlfile(const char *DataDir,
StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE,
"sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
+ /* Update timestamp */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/* Recalculate CRC of control file */
INIT_CRC32C(ControlFile->crc);
COMP_CRC32C(ControlFile->crc,
--
2.18.0
