public inbox for [email protected]  

help / color / mirror / Atom feed

From: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 7/8] Change ALTER TABLESPACE to use the pending-sync infrastructure
Date: Mon, 25 Mar 2019 20:39:21 +0900

Apply heap_register_sync() to ATLER TABLESPACE stuff.
---
 src/backend/commands/tablecmds.c | 54 +++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ce52eaae..aabb3806f6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -84,7 +84,6 @@
 #include "storage/lmgr.h"
 #include "storage/lock.h"
 #include "storage/predicate.h"
-#include "storage/smgr.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -11891,7 +11890,7 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst,
 {
 	PGAlignedBlock buf;
 	Page		page;
-	bool		use_wal;
+	bool		use_wal = false;
 	bool		copying_initfork;
 	BlockNumber nblocks;
 	BlockNumber blkno;
@@ -11906,12 +11905,33 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst,
 	copying_initfork = relpersistence == RELPERSISTENCE_UNLOGGED &&
 		forkNum == INIT_FORKNUM;
 
-	/*
-	 * We need to log the copied data in WAL iff WAL archiving/streaming is
-	 * enabled AND it's a permanent relation.
-	 */
-	use_wal = XLogIsNeeded() &&
-		(relpersistence == RELPERSISTENCE_PERMANENT || copying_initfork);
+	if (relpersistence == RELPERSISTENCE_PERMANENT || copying_initfork)
+	{
+		/*
+		 * We need to log the copied data in WAL iff WAL archiving/streaming
+		 * is enabled AND it's a permanent relation.
+		 */
+		if (XLogIsNeeded())
+			use_wal = true;
+
+		/*
+		 * If the rel is WAL-logged, must fsync at commit.  We do the same to
+		 * ensure that the toast table gets fsync'd too.  (For a temp or
+		 * unlogged rel we don't care since the data will be gone after a
+		 * crash anyway.)
+		 *
+		 * It's obvious that we must do this when not WAL-logging the
+		 * copy. It's less obvious that we have to do it even if we did
+		 * WAL-log the copied pages. The reason is that since we're copying
+		 * outside shared buffers, a CHECKPOINT occurring during the copy has
+		 * no way to flush the previously written data to disk (indeed it
+		 * won't know the new rel even exists).  A crash later on would replay
+		 * WAL from the checkpoint, therefore it wouldn't replay our earlier
+		 * WAL entries. If we do not fsync those pages here, they might still
+		 * not be on disk when the crash occurs.
+		 */
+		RecordPendingSync(dst, forkNum);
+	}
 
 	nblocks = smgrnblocks(src, forkNum);
 
@@ -11948,24 +11968,6 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst,
 		 */
 		smgrextend(dst, forkNum, blkno, buf.data, true);
 	}
-
-	/*
-	 * If the rel is WAL-logged, must fsync before commit.  We use heap_sync
-	 * to ensure that the toast table gets fsync'd too.  (For a temp or
-	 * unlogged rel we don't care since the data will be gone after a crash
-	 * anyway.)
-	 *
-	 * It's obvious that we must do this when not WAL-logging the copy. It's
-	 * less obvious that we have to do it even if we did WAL-log the copied
-	 * pages. The reason is that since we're copying outside shared buffers, a
-	 * CHECKPOINT occurring during the copy has no way to flush the previously
-	 * written data to disk (indeed it won't know the new rel even exists).  A
-	 * crash later on would replay WAL from the checkpoint, therefore it
-	 * wouldn't replay our earlier WAL entries. If we do not fsync those pages
-	 * here, they might still not be on disk when the crash occurs.
-	 */
-	if (relpersistence == RELPERSISTENCE_PERMANENT || copying_initfork)
-		smgrimmedsync(dst, forkNum);
 }
 
 /*
-- 
2.16.3


----Next_Part(Tue_Mar_26_16_35_07_2019_128)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v9-0008-Optimize-WAL-logging-on-btree-bulk-insertion.patch"

view thread (6+ 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 7/8] Change ALTER TABLESPACE to use the pending-sync infrastructure
  In-Reply-To: <no-message-id-1883705@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