agora inbox for [email protected]  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 2/3] Set correct value of full_page_write reloaded during recovery
Date: Tue, 28 Aug 2018 18:53:20 +0900

If full_page_write was edited in config file then reloaded during
recovery, the change will be postponed until the next reload. This
patch fixes that.
---
 src/backend/access/transam/xlog.c     | 19 +++++++++------
 src/backend/postmaster/checkpointer.c | 44 ++++++++++++++++++++++++-----------
 src/include/postmaster/bgwriter.h     |  1 +
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 493f1db7b9..f315d11745 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7692,14 +7692,15 @@ StartupXLOG(void)
 	XLogCtl->LogwrtRqst.Flush = EndOfLog;
 
 	/*
-	 * 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.
+	 * Update full_page_writes in shared memory with the lastest value before
+	 * resource manager writes cleanup WAL records or checkpoint record is
+	 * written. We don't need to write XLOG_FPW_CHANGE since this just
+	 * reflects the status at the last redo'ed record. No lock is required
+	 * since startup is the only updator of the flag at this
+	 * point. Checkpointer will take over after SharedRecoveryInProgress is
+	 * turned to false.
 	 */
 	Insert->fullPageWrites = lastFullPageWrites;
-	LocalSetXLogInsertAllowed();
-	UpdateFullPageWrites();
-	LocalXLogInsertAllowed = -1;
 
 	if (InRecovery)
 	{
@@ -7941,10 +7942,14 @@ StartupXLOG(void)
 	 * If this was a fast promotion, request an (online) checkpoint now. This
 	 * isn't required for consistency, but the last restartpoint might be far
 	 * back, and in case of a crash, recovering from it might take a longer
-	 * than is appropriate now that we're not in standby mode anymore.
+	 * than is appropriate now that we're not in standby mode anymore.  If
+	 * checkpoint is not needed, tell checkpointer to update shared
+	 * configuration instead.
 	 */
 	if (fast_promoted)
 		RequestCheckpoint(CHECKPOINT_FORCE);
+	else
+		WakeupCheckpointer();
 }
 
 /*
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 011ff1d442..63bb4904ef 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -358,20 +358,25 @@ CheckpointerMain(void)
 		{
 			got_SIGHUP = false;
 			ProcessConfigFile(PGC_SIGHUP);
-
-			/*
-			 * Checkpointer is the last process to shut down, so we ask it to
-			 * hold the keys for a range of other tasks required most of which
-			 * have nothing to do with checkpointing at all.
-			 *
-			 * For various reasons, some config values can change dynamically
-			 * so the primary copy of them is held in shared memory to make
-			 * sure all backends see the same value.  We make Checkpointer
-			 * responsible for updating the shared memory copy if the
-			 * parameter setting changes because of SIGHUP.
-			 */
-			UpdateSharedMemoryConfig();
 		}
+
+		/*
+		 * Checkpointer is the last process to shut down, so we ask it to hold
+		 * the keys for a range of other tasks required most of which have
+		 * nothing to do with checkpointing at all.
+		 *
+		 * For various reasons, some config values can change dynamically so
+		 * the primary copy of them is held in shared memory to make sure all
+		 * backends see the same value.  We make Checkpointer responsible for
+		 * updating the shared memory copy if the parameter setting changes
+		 * because of SIGHUP.
+		 *
+		 * Since some of the shared variables are omitted during
+		 * recovery. UpdateSharedMemoryConfig() is needed regardless of SIGHUP
+		 * in order to keep them in sync without waiting the next reload.
+		 */
+		UpdateSharedMemoryConfig();
+
 		if (checkpoint_requested)
 		{
 			checkpoint_requested = false;
@@ -1074,6 +1079,19 @@ RequestCheckpoint(int flags)
 	}
 }
 
+/*
+ * WakeupCheckpointer
+ *
+ * Requests checkpointer to wake up. This causes per-loop processing
+ * regardless of the necessity of a checkpoint.
+ */
+void
+WakeupCheckpointer(void)
+{
+	if (IsUnderPostmaster && CheckpointerShmem->checkpointer_pid != 0)
+		kill(CheckpointerShmem->checkpointer_pid, SIGINT);
+}
+
 /*
  * ForwardFsyncRequest
  *		Forward a file-fsync request from a backend to the checkpointer
diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h
index 941c6aba7d..3aac1606a0 100644
--- a/src/include/postmaster/bgwriter.h
+++ b/src/include/postmaster/bgwriter.h
@@ -29,6 +29,7 @@ extern void BackgroundWriterMain(void) pg_attribute_noreturn();
 extern void CheckpointerMain(void) pg_attribute_noreturn();
 
 extern void RequestCheckpoint(int flags);
+extern void WakeupCheckpointer(void);
 extern void CheckpointWriteDelay(int flags, double progress);
 
 extern bool ForwardFsyncRequest(RelFileNode rnode, ForkNumber forknum,
-- 
2.16.3


----Next_Part(Tue_Aug_28_19_34_36_2018_153)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v3-0003-Let-startup-process-ignore-SIGHUP.patch"



view thread (2+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [PATCH 2/3] Set correct value of full_page_write reloaded during recovery
  In-Reply-To: <no-message-id-234568@localhost>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox