agora inbox for [email protected]  
help / color / mirror / Atom feed
From: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 2/2] Fix WAL skipping feature
Date: Mon, 20 May 2019 15:38:59 +0900

This patch shows a PoC of how change WAL-skipping feature to avoid
table corruption caused by mixing wal-logged and wal-skipped
operations.
---
 src/backend/access/heap/heapam.c         |  4 ++--
 src/backend/access/heap/heapam_handler.c |  7 +------
 src/backend/access/heap/rewriteheap.c    |  3 ---
 src/backend/access/transam/xact.c        |  6 ++++++
 src/backend/commands/copy.c              |  4 ----
 src/backend/commands/createas.c          |  3 +--
 src/backend/commands/tablecmds.c         |  2 --
 src/backend/utils/cache/relcache.c       | 22 ++++++++++++++++++++++
 src/include/access/heapam.h              |  1 -
 src/include/utils/rel.h                  |  3 ++-
 src/include/utils/relcache.h             |  1 +
 11 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 19d2c529d8..dda76c8736 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1950,7 +1950,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	MarkBufferDirty(buffer);
 
 	/* XLOG stuff */
-	if (!(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation))
+	if (RelationNeedsWAL(relation))
 	{
 		xl_heap_insert xlrec;
 		xl_heap_header xlhdr;
@@ -2133,7 +2133,7 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	/* currently not needed (thus unsupported) for heap_multi_insert() */
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
-	needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
+	needwal = RelationNeedsWAL(relation);
 	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
 												   HEAP_DEFAULT_FILLFACTOR);
 
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 8d8161fd97..f4af981a35 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -560,12 +560,7 @@ tuple_lock_retry:
 static void
 heapam_finish_bulk_insert(Relation relation, int options)
 {
-	/*
-	 * If we skipped writing WAL, then we need to sync the heap (but not
-	 * indexes since those use WAL anyway / don't go through tableam)
-	 */
-	if (options & HEAP_INSERT_SKIP_WAL)
-		heap_sync(relation);
+	/* heapam doesn't need do this */
 }
 
 
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index bce4274362..1ac77f7c14 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -654,9 +654,6 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 	{
 		int options = HEAP_INSERT_SKIP_FSM;
 
-		if (!state->rs_use_wal)
-			options |= HEAP_INSERT_SKIP_WAL;
-
 		/*
 		 * While rewriting the heap for VACUUM FULL / CLUSTER, make sure data
 		 * for the TOAST table are not logically decoded.  The main heap is
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 20feeec327..fb35992a13 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2133,6 +2133,9 @@ CommitTransaction(void)
 	/* Commit updates to the relation map --- do this as late as possible */
 	AtEOXact_RelationMap(true, is_parallel_worker);
 
+	/* Perform pending flush */
+	AtEOXact_DoPendingFlush();
+
 	/*
 	 * set the current transaction state information appropriately during
 	 * commit processing
@@ -2349,6 +2352,9 @@ PrepareTransaction(void)
 	 */
 	PreCommit_CheckForSerializationFailure();
 
+	/* Perform pending flush */
+	AtEOXact_DoPendingFlush();
+
 	/* NOTIFY will be handled below */
 
 	/*
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 6ffc3a62f6..9bae04b8a7 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2761,11 +2761,7 @@ CopyFrom(CopyState cstate)
 	if (RELKIND_HAS_STORAGE(cstate->rel->rd_rel->relkind) &&
 		(cstate->rel->rd_createSubid != InvalidSubTransactionId ||
 		 cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId))
-	{
 		ti_options |= TABLE_INSERT_SKIP_FSM;
-		if (!XLogIsNeeded())
-			ti_options |= TABLE_INSERT_SKIP_WAL;
-	}
 
 	/*
 	 * Optimize if new relfilenode was created in this subxact or one of its
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 43c2fa9124..83e5f9220f 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -558,8 +558,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	 * We can skip WAL-logging the insertions, unless PITR or streaming
 	 * replication is in use. We can skip the FSM in any case.
 	 */
-	myState->ti_options = TABLE_INSERT_SKIP_FSM |
-		(XLogIsNeeded() ? 0 : TABLE_INSERT_SKIP_WAL);
+	myState->ti_options = TABLE_INSERT_SKIP_FSM;
 	myState->bistate = GetBulkInsertState();
 
 	/* Not using WAL requires smgr_targblock be initially invalid */
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bfcf9472d7..b686497443 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4741,8 +4741,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 		bistate = GetBulkInsertState();
 
 		ti_options = TABLE_INSERT_SKIP_FSM;
-		if (!XLogIsNeeded())
-			ti_options |= TABLE_INSERT_SKIP_WAL;
 	}
 	else
 	{
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index d0f6f715e6..10fd405171 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2913,6 +2913,28 @@ RememberToFreeTupleDescAtEOX(TupleDesc td)
 	EOXactTupleDescArray[NextEOXactTupleDescNum++] = td;
 }
 
+void
+AtEOXact_DoPendingFlush()
+{
+	HASH_SEQ_STATUS status;
+	RelIdCacheEnt *idhentry;
+
+	if (!RelationIdCache)
+		return;
+
+	hash_seq_init(&status, RelationIdCache);
+	while ((idhentry = (RelIdCacheEnt *) hash_seq_search(&status)) != NULL)
+	{
+		Relation rel = idhentry->reldesc;
+		if (RELATION_IS_LOCAL(rel) && !XLogIsNeeded() && rel->rd_smgr)
+		{
+			FlushRelationBuffers(rel);
+			smgrimmedsync(rel->rd_smgr, MAIN_FORKNUM);
+		}
+	}
+}
+
+
 /*
  * AtEOXact_RelationCache
  *
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 62aaa08eff..0fb7d86bf2 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -29,7 +29,6 @@
 
 
 /* "options" flag bits for heap_insert */
-#define HEAP_INSERT_SKIP_WAL	TABLE_INSERT_SKIP_WAL
 #define HEAP_INSERT_SKIP_FSM	TABLE_INSERT_SKIP_FSM
 #define HEAP_INSERT_FROZEN		TABLE_INSERT_FROZEN
 #define HEAP_INSERT_NO_LOGICAL	TABLE_INSERT_NO_LOGICAL
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index d7f33abce3..41ab634ff5 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -514,7 +514,8 @@ typedef struct ViewOptions
  *		True if relation needs WAL.
  */
 #define RelationNeedsWAL(relation) \
-	((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT)
+	((relation)->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT && \
+	 !(RELATION_IS_LOCAL(relation) && !XLogIsNeeded()))
 
 /*
  * RelationUsesLocalBuffers
diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 364495a5f0..cd9b1a6f68 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -123,6 +123,7 @@ extern void RelationCloseSmgrByOid(Oid relationId);
 extern void AtEOXact_RelationCache(bool isCommit);
 extern void AtEOSubXact_RelationCache(bool isCommit, SubTransactionId mySubid,
 						  SubTransactionId parentSubid);
+extern void AtEOXact_DoPendingFlush(void);
 
 /*
  * Routines to help manage rebuilding of relcache init files
-- 
2.16.3


----Next_Part(Mon_May_20_15_54_31_2019_643)----






view thread (19+ 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/2] Fix WAL skipping feature
  In-Reply-To: <no-message-id-215579@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