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