public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 4/4] Fix WAL skipping feature.
Date: Thu, 11 Oct 2018 16:18:04 +0900
This patch repalces WAL-skipping means from HEAP_INSERT_SKIP_WAL to
pending-sync tracking infrastructure.
---
src/backend/access/heap/heapam.c | 70 ++++++++++++++++++++++-----------
src/backend/access/heap/pruneheap.c | 3 +-
src/backend/access/heap/rewriteheap.c | 3 --
src/backend/access/heap/visibilitymap.c | 3 +-
src/backend/commands/copy.c | 13 +++---
src/backend/commands/createas.c | 9 ++---
src/backend/commands/matview.c | 6 +--
src/backend/commands/tablecmds.c | 5 +--
src/backend/commands/vacuumlazy.c | 6 +--
src/include/access/heapam.h | 9 ++---
10 files changed, 72 insertions(+), 55 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 8f1ea73541..c9c254a032 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -34,6 +34,28 @@
* the POSTGRES heap access method used for all POSTGRES
* relations.
*
+ * WAL CONSIDERATIONS
+ * All heap operations are normally WAL-logged. but there are a few
+ * exceptions. Temporary and unlogged relations never need to be
+ * WAL-logged, but we can also skip WAL-logging for a table that was
+ * created in the same transaction, if we don't need WAL for PITR or
+ * WAL archival purposes (i.e. if wal_level=minimal), and we fsync()
+ * the file to disk at COMMIT instead.
+ *
+ * The same-relation optimization is not employed automatically on all
+ * updates to a table that was created in the same transacton, because
+ * for a small number of changes, it's cheaper to just create the WAL
+ * records than fsyncing() the whole relation at COMMIT. It is only
+ * worthwhile for (presumably) large operations like COPY, CLUSTER,
+ * or VACUUM FULL. Use heap_register_sync() to initiate such an
+ * operation; it will cause any subsequent updates to the table to skip
+ * WAL-logging, if possible, and cause the heap to be synced to disk at
+ * COMMIT.
+ *
+ * To make that work, all modifications to heap must use
+ * BufferNeedsWAL() to check if WAL-logging is needed in this transaction
+ * for the given block.
+ *
*-------------------------------------------------------------------------
*/
#include "postgres.h"
@@ -2414,12 +2436,6 @@ ReleaseBulkInsertStatePin(BulkInsertState bistate)
* The new tuple is stamped with current transaction ID and the specified
* command ID.
*
- * If the HEAP_INSERT_SKIP_WAL option is specified, the new tuple is not
- * logged in WAL, even for a non-temp relation. Safe usage of this behavior
- * requires that we arrange that all new tuples go into new pages not
- * containing any tuples from other transactions, and that the relation gets
- * fsync'd before commit. (See also heap_sync() comments)
- *
* The HEAP_INSERT_SKIP_FSM option is passed directly to
* RelationGetBufferForTuple, which see for more info.
*
@@ -2528,7 +2544,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
MarkBufferDirty(buffer);
/* XLOG stuff */
- if (!(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_insert xlrec;
xl_heap_header xlhdr;
@@ -2704,7 +2720,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
int ndone;
PGAlignedBlock scratch;
Page page;
- bool needwal;
Size saveFreeSpace;
bool need_tuple_data = RelationIsLogicallyLogged(relation);
bool need_cids = RelationIsAccessibleInLogicalDecoding(relation);
@@ -2712,7 +2727,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, 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);
saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
HEAP_DEFAULT_FILLFACTOR);
@@ -2754,6 +2768,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
Buffer vmbuffer = InvalidBuffer;
bool all_visible_cleared = false;
int nthispage;
+ bool needwal;
CHECK_FOR_INTERRUPTS();
@@ -2765,6 +2780,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
InvalidBuffer, options, bistate,
&vmbuffer, NULL);
page = BufferGetPage(buffer);
+ needwal = BufferNeedsWAL(relation, buffer);
/* NO EREPORT(ERROR) from here till changes are logged */
START_CRIT_SECTION();
@@ -3327,7 +3343,7 @@ l1:
* NB: heap_abort_speculative() uses the same xlog record and replay
* routines.
*/
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_delete xlrec;
XLogRecPtr recptr;
@@ -4069,7 +4085,7 @@ l2:
MarkBufferDirty(buffer);
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_lock xlrec;
XLogRecPtr recptr;
@@ -4291,7 +4307,8 @@ l2:
MarkBufferDirty(buffer);
/* XLOG stuff */
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer) ||
+ BufferNeedsWAL(relation, newbuf))
{
XLogRecPtr recptr;
@@ -5263,7 +5280,7 @@ failed:
* (Also, in a PITR log-shipping or 2PC environment, we have to have XLOG
* entries for everything anyway.)
*/
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, *buffer))
{
xl_heap_lock xlrec;
XLogRecPtr recptr;
@@ -6007,7 +6024,7 @@ l4:
MarkBufferDirty(buf);
/* XLOG stuff */
- if (RelationNeedsWAL(rel))
+ if (BufferNeedsWAL(rel, buf))
{
xl_heap_lock_updated xlrec;
XLogRecPtr recptr;
@@ -6167,7 +6184,7 @@ heap_finish_speculative(Relation relation, HeapTuple tuple)
htup->t_ctid = tuple->t_self;
/* XLOG stuff */
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_confirm xlrec;
XLogRecPtr recptr;
@@ -6300,7 +6317,7 @@ heap_abort_speculative(Relation relation, HeapTuple tuple)
* The WAL records generated here match heap_delete(). The same recovery
* routines are used.
*/
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_delete xlrec;
XLogRecPtr recptr;
@@ -6409,7 +6426,7 @@ heap_inplace_update(Relation relation, HeapTuple tuple)
MarkBufferDirty(buffer);
/* XLOG stuff */
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_inplace xlrec;
XLogRecPtr recptr;
@@ -7605,7 +7622,7 @@ log_heap_clean(Relation reln, Buffer buffer,
XLogRecPtr recptr;
/* Caller should not call me on a non-WAL-logged relation */
- Assert(RelationNeedsWAL(reln));
+ Assert(BufferNeedsWAL(reln, buffer));
xlrec.latestRemovedXid = latestRemovedXid;
xlrec.nredirected = nredirected;
@@ -7653,7 +7670,7 @@ log_heap_freeze(Relation reln, Buffer buffer, TransactionId cutoff_xid,
XLogRecPtr recptr;
/* Caller should not call me on a non-WAL-logged relation */
- Assert(RelationNeedsWAL(reln));
+ Assert(BufferNeedsWAL(reln, buffer));
/* nor when there are no tuples to freeze */
Assert(ntuples > 0);
@@ -7738,7 +7755,7 @@ log_heap_update(Relation reln, Buffer oldbuf,
int bufflags;
/* Caller should not call me on a non-WAL-logged relation */
- Assert(RelationNeedsWAL(reln));
+ Assert(BufferNeedsWAL(reln, newbuf) || BufferNeedsWAL(reln, oldbuf));
XLogBeginInsert();
@@ -9342,9 +9359,16 @@ heap2_redo(XLogReaderState *record)
* heap_sync - sync a heap, for use when no WAL has been written
*
* This forces the heap contents (including TOAST heap if any) down to disk.
- * If we skipped using WAL, and WAL is otherwise needed, we must force the
- * relation down to disk before it's safe to commit the transaction. This
- * requires writing out any dirty buffers and then doing a forced fsync.
+ * If we did any changes to the heap bypassing the buffer manager, we must
+ * force the relation down to disk before it's safe to commit the
+ * transaction, because the direct modifications will not be flushed by
+ * the next checkpoint.
+ *
+ * We used to also use this after batch operations like COPY and CLUSTER,
+ * if we skipped using WAL and WAL is otherwise needed, but there were
+ * corner-cases involving other WAL-logged operations to the same
+ * relation, where that was not enough. heap_register_sync() should be
+ * used for that purpose instead.
*
* Indexes are not touched. (Currently, index operations associated with
* the commands that use this are WAL-logged and so do not need fsync.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index c2f5343dac..d0b68902d9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -20,6 +20,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
+#include "catalog/storage.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/bufmgr.h"
@@ -259,7 +260,7 @@ heap_page_prune(Relation relation, Buffer buffer, TransactionId OldestXmin,
/*
* Emit a WAL HEAP_CLEAN record showing what we did
*/
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
XLogRecPtr recptr;
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 44caeca336..ecddc40329 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -655,9 +655,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/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 695567b4b0..fce14ce35f 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -88,6 +88,7 @@
#include "access/heapam_xlog.h"
#include "access/visibilitymap.h"
#include "access/xlog.h"
+#include "catalog/storage.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
@@ -307,7 +308,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
map[mapByte] |= (flags << mapOffset);
MarkBufferDirty(vmBuf);
- if (RelationNeedsWAL(rel))
+ if (BufferNeedsWAL(rel, heapBuf))
{
if (XLogRecPtrIsInvalid(recptr))
{
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 4311e16007..d583b5a8a3 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2364,8 +2364,7 @@ CopyFrom(CopyState cstate)
* - data is being written to relfilenode created in this transaction
* then we can skip writing WAL. It's safe because if the transaction
* doesn't commit, we'll discard the table (or the new relfilenode file).
- * If it does commit, we'll have done the heap_sync at the bottom of this
- * routine first.
+ * If it does commit, commit will do heap_sync().
*
* As mentioned in comments in utils/rel.h, the in-same-transaction test
* is not always set correctly, since in rare cases rd_newRelfilenodeSubid
@@ -2406,7 +2405,7 @@ CopyFrom(CopyState cstate)
{
hi_options |= HEAP_INSERT_SKIP_FSM;
if (!XLogIsNeeded())
- hi_options |= HEAP_INSERT_SKIP_WAL;
+ heap_register_sync(cstate->rel);
}
/*
@@ -3036,11 +3035,11 @@ CopyFrom(CopyState cstate)
FreeExecutorState(estate);
/*
- * If we skipped writing WAL, then we need to sync the heap (but not
- * indexes since those use WAL anyway)
+ * If we skipped writing WAL, then we will sync the heap at the end of
+ * the transaction. (We used to do it here, but it was later found out
+ * that to be safe, we must also avoid WAL-logging any subsequent
+ * actions on the pages we skipped WAL for). Indexes always use WAL.
*/
- if (hi_options & HEAP_INSERT_SKIP_WAL)
- heap_sync(cstate->rel);
return processed;
}
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258b65..3d32d07d69 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -555,8 +555,9 @@ 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->hi_options = HEAP_INSERT_SKIP_FSM |
- (XLogIsNeeded() ? 0 : HEAP_INSERT_SKIP_WAL);
+ if (!XLogIsNeeded())
+ heap_register_sync(intoRelationDesc);
+ myState->hi_options = HEAP_INSERT_SKIP_FSM;
myState->bistate = GetBulkInsertState();
/* Not using WAL requires smgr_targblock be initially invalid */
@@ -599,9 +600,7 @@ intorel_shutdown(DestReceiver *self)
FreeBulkInsertState(myState->bistate);
- /* If we skipped using WAL, must heap_sync before commit */
- if (myState->hi_options & HEAP_INSERT_SKIP_WAL)
- heap_sync(myState->rel);
+ /* If we skipped using WAL, we will sync the relation at commit */
/* close rel, but keep lock until commit */
heap_close(myState->rel, NoLock);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index a171ebabf8..174aa3376a 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -461,7 +461,7 @@ transientrel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
*/
myState->hi_options = HEAP_INSERT_SKIP_FSM | HEAP_INSERT_FROZEN;
if (!XLogIsNeeded())
- myState->hi_options |= HEAP_INSERT_SKIP_WAL;
+ heap_register_sync(transientrel);
myState->bistate = GetBulkInsertState();
/* Not using WAL requires smgr_targblock be initially invalid */
@@ -507,9 +507,7 @@ transientrel_shutdown(DestReceiver *self)
FreeBulkInsertState(myState->bistate);
- /* If we skipped using WAL, must heap_sync before commit */
- if (myState->hi_options & HEAP_INSERT_SKIP_WAL)
- heap_sync(myState->transientrel);
+ /* If we skipped using WAL, we will sync the relation at commit */
/* close transientrel, but keep lock until commit */
heap_close(myState->transientrel, NoLock);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 879c3d981e..ce8f7cd881 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4591,8 +4591,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
bistate = GetBulkInsertState();
hi_options = HEAP_INSERT_SKIP_FSM;
+
if (!XLogIsNeeded())
- hi_options |= HEAP_INSERT_SKIP_WAL;
+ heap_register_sync(newrel);
}
else
{
@@ -4857,8 +4858,6 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
FreeBulkInsertState(bistate);
/* If we skipped writing WAL, then we need to sync the heap. */
- if (hi_options & HEAP_INSERT_SKIP_WAL)
- heap_sync(newrel);
heap_close(newrel, NoLock);
}
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 8134c52253..28caf92073 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -924,7 +924,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
* page has been previously WAL-logged, and if not, do that
* now.
*/
- if (RelationNeedsWAL(onerel) &&
+ if (BufferNeedsWAL(onerel, buf) &&
PageGetLSN(page) == InvalidXLogRecPtr)
log_newpage_buffer(buf, true);
@@ -1188,7 +1188,7 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
}
/* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(onerel))
+ if (BufferNeedsWAL(onerel, buf))
{
XLogRecPtr recptr;
@@ -1569,7 +1569,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
MarkBufferDirty(buffer);
/* XLOG stuff */
- if (RelationNeedsWAL(onerel))
+ if (BufferNeedsWAL(onerel, buffer))
{
XLogRecPtr recptr;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 4baa287c8c..d2fbc1ad47 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -25,11 +25,10 @@
/* "options" flag bits for heap_insert */
-#define HEAP_INSERT_SKIP_WAL 0x0001
-#define HEAP_INSERT_SKIP_FSM 0x0002
-#define HEAP_INSERT_FROZEN 0x0004
-#define HEAP_INSERT_SPECULATIVE 0x0008
-#define HEAP_INSERT_NO_LOGICAL 0x0010
+#define HEAP_INSERT_SKIP_FSM 0x0001
+#define HEAP_INSERT_FROZEN 0x0002
+#define HEAP_INSERT_SPECULATIVE 0x0004
+#define HEAP_INSERT_NO_LOGICAL 0x0008
typedef struct BulkInsertStateData *BulkInsertState;
--
2.16.3
----Next_Part(Thu_Dec_20_17_32_25_2018_311)----
view thread (9+ 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 4/4] Fix WAL skipping feature.
In-Reply-To: <no-message-id-3681@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