On Tue, Mar 13, 2018 at 05:04:04PM +0900, Michael Paquier wrote: > Instead of doing what you are suggesting, why not moving > InitXLogInsert() out of InitXLOGAccess() and change InitPostgres() so as > the allocations for WAL inserts is done unconditionally? This has > the cost of also making this allocation even for backends which are > started during recovery, still we are talking about allocating a couple > of bytes in exchange of addressing completely all race conditions in > this area. InitXLogInsert() does not depend on any post-recovery data > like ThisTimeLineId, so a split is possible.
I have been hacking things this way, and it seems to me that it takes care of all this class of problems. CreateCheckPoint() actually mentions that InitXLogInsert() cannot be called in a critical section, so the comments don't match the code. I also think that we still want to be able to use RecoveryInProgress() in critical sections to do decision-making for the generation of WAL records. -- Michael
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 47a6c4d895..cff238ee2a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8049,6 +8049,9 @@ LocalSetXLogInsertAllowed(void)
/* Initialize as RecoveryInProgress() would do when switching state */
InitXLOGAccess();
+
+ /* Also initialize the working areas for constructing WAL records */
+ InitXLogInsert();
}
/*
@@ -8178,9 +8181,6 @@ InitXLOGAccess(void)
(void) GetRedoRecPtr();
/* Also update our copy of doPageWrites. */
doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites);
-
- /* Also initialize the working areas for constructing WAL records */
- InitXLogInsert();
}
/*
@@ -8582,11 +8582,11 @@ CreateCheckPoint(int flags)
/*
* Initialize InitXLogInsert working areas before entering the critical
- * section. Normally, this is done by the first call to
- * RecoveryInProgress() or LocalSetXLogInsertAllowed(), but when creating
- * an end-of-recovery checkpoint, the LocalSetXLogInsertAllowed call is
- * done below in a critical section, and InitXLogInsert cannot be called
- * in a critical section.
+ * section. Normally, this is done at backend startup or when calling
+ * LocalSetXLogInsertAllowed(), but when creating an end-of-recovery
+ * checkpoint, the LocalSetXLogInsertAllowed call is done below in a
+ * critical section, and InitXLogInsert cannot be called in a critical
+ * section.
*/
InitXLogInsert();
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 28ff2f0979..4d20b9712f 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -452,6 +452,7 @@ AuxiliaryProcessMain(int argc, char *argv[])
case WalWriterProcess:
/* don't set signals, walwriter has its own agenda */
InitXLOGAccess();
+ InitXLogInsert();
WalWriterMain();
proc_exit(1); /* should never return */
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index d8f45b3c43..8db377c1cf 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -618,6 +618,15 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
*/
if (IsUnderPostmaster)
{
+ /*
+ * Initialize the working areas for constructing WAL records.
+ * This is done even for a standby instance to avoid initialization
+ * of this machinery after a promotion, which could happen in a
+ * critical section and InitXLogInsert() cannot be called in such
+ * a code path.
+ */
+ InitXLogInsert();
+
/*
* The postmaster already started the XLOG machinery, but we need to
* call InitXLOGAccess(), if the system isn't in hot-standby mode.
signature.asc
Description: PGP signature
