At Wed, 28 Mar 2018 15:59:48 +0900, Michael Paquier <mich...@paquier.xyz> wrote
in <20180328065948.gm1...@paquier.xyz>
> On Wed, Mar 28, 2018 at 03:40:59PM +0900, Kyotaro HORIGUCHI wrote:
> > The attached does that. I don't like that it uses ControlFileLock
> > to exlucde concurrent UpdateFullPageWrites and StartupXLOG but
> > WALInsertLock cannot be used since UpdateFullPageWrites may take
> > the same lock.
>
> You visibly forgot your patch.
Mmm, someone must have eaten that. I'm sure it is attached this
time.
I don't like UpdateFullPageWrites is using ControlFileLock to
exclusion..
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index cb9c2a29cb..d2bb5e16ac 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7558,9 +7558,11 @@ StartupXLOG(void)
/*
* 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.
+ * record is written, ignoreing the change of full_page_write GUC so far.
*/
+ WALInsertLockAcquireExclusive();
Insert->fullPageWrites = lastFullPageWrites;
+ WALInsertLockRelease();
LocalSetXLogInsertAllowed();
UpdateFullPageWrites();
LocalXLogInsertAllowed = -1;
@@ -7783,6 +7785,9 @@ StartupXLOG(void)
* itself, we use the info_lck here to ensure that there are no race
* conditions concerning visibility of other recent updates to shared
* memory.
+ *
+ * ControlFileLock excludes concurrent update of shared fullPageWrites in
+ * addition to its ordinary use.
*/
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
ControlFile->state = DB_IN_PRODUCTION;
@@ -7790,11 +7795,25 @@ StartupXLOG(void)
SpinLockAcquire(&XLogCtl->info_lck);
XLogCtl->SharedRecoveryInProgress = false;
+ lastFullPageWrites = Insert->fullPageWrites;
SpinLockRelease(&XLogCtl->info_lck);
UpdateControlFile();
LWLockRelease(ControlFileLock);
+ /*
+ * Shared fullPageWrites at the end of recovery differs to our last known
+ * value. The change has been made by checkpointer but we haven't made
+ * corresponding XLOG_FPW_CHANGE record. We reread GUC change if any and
+ * try update shared fullPageWrites by myself. It ends with doing nothing
+ * if checkpointer already did the same thing.
+ */
+ if (lastFullPageWrites != fullPageWrites)
+ {
+ HandleStartupProcInterrupts();
+ UpdateFullPageWrites();
+ }
+
/*
* If there were cascading standby servers connected to us, nudge any wal
* sender processes to notice that we've been promoted.
@@ -9525,8 +9544,10 @@ XLogReportParameters(void)
* Update full_page_writes in shared memory, and write an
* XLOG_FPW_CHANGE record if necessary.
*
- * Note: this function assumes there is no other process running
- * concurrently that could update it.
+ * This function assumes called concurrently from multiple processes and
+ * called concurrently with changing of shared fullPageWrites. See
+ * StartupXLOG().
+ *
*/
void
UpdateFullPageWrites(void)
@@ -9536,13 +9557,25 @@ UpdateFullPageWrites(void)
/*
* Do nothing if full_page_writes has not been changed.
*
- * It's safe to check the shared full_page_writes without the lock,
- * because we assume that there is no concurrently running process which
- * can update it.
+ * We acquire ControlFileLock to exclude other concurrent call and change
+ * of shared fullPageWrites.
*/
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+ WALInsertLockAcquireExclusive();
if (fullPageWrites == Insert->fullPageWrites)
+ {
+ WALInsertLockRelease();
+ LWLockRelease(ControlFileLock);
return;
+ }
+ WALInsertLockRelease();
+ /*
+ * Need to set up XLogInsert working area before entering critical section
+ * if we are not in recovery mode.
+ */
+ (void) RecoveryInProgress();
+
START_CRIT_SECTION();
/*
@@ -9578,6 +9611,8 @@ UpdateFullPageWrites(void)
WALInsertLockRelease();
}
END_CRIT_SECTION();
+
+ LWLockRelease(ControlFileLock);
}
/*
