agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 5/7] Fix WAL skipping feature.
Date: Thu, 11 Oct 2018 16:18:04 +0900
This patch replaces WAL-skipping means from HEAP_INSERT_SKIP_WAL to
pending-sync tracking infrastructure.
---
src/backend/access/heap/heapam.c | 104 ++++++++++++++++++++++++--------
src/backend/access/heap/pruneheap.c | 3 +-
src/backend/access/heap/rewriteheap.c | 3 -
src/backend/access/heap/vacuumlazy.c | 6 +-
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/include/access/heapam.h | 3 +-
src/include/access/tableam.h | 11 +---
11 files changed, 104 insertions(+), 62 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 024620ddc1..96f2cde3ce 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -28,6 +28,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"
@@ -1963,7 +1985,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))
{
XLogRecPtr recptr;
@@ -2073,7 +2095,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);
@@ -2081,7 +2102,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);
@@ -2123,6 +2143,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();
@@ -2134,6 +2155,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();
@@ -2686,7 +2708,7 @@ l1:
* NB: heap_abort_speculative() uses the same xlog record and replay
* routines.
*/
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
XLogRecPtr recptr;
@@ -2820,6 +2842,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
vmbuffer = InvalidBuffer,
vmbuffer_new = InvalidBuffer;
bool need_toast;
+ bool oldbuf_needs_wal,
+ newbuf_needs_wal;
Size newtupsize,
pagefree;
bool have_tuple_lock = false;
@@ -3371,7 +3395,7 @@ l2:
MarkBufferDirty(buffer);
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_lock xlrec;
XLogRecPtr recptr;
@@ -3585,8 +3609,20 @@ l2:
MarkBufferDirty(newbuf);
MarkBufferDirty(buffer);
- /* XLOG stuff */
- if (RelationNeedsWAL(relation))
+ /*
+ * XLOG stuff
+ *
+ * Emit heap-update log. When wal_level = minimal, we may emit insert or
+ * delete record according to wal-optimization.
+ */
+ oldbuf_needs_wal = BufferNeedsWAL(relation, buffer);
+
+ if (newbuf == buffer)
+ newbuf_needs_wal = oldbuf_needs_wal;
+ else
+ newbuf_needs_wal = BufferNeedsWAL(relation, newbuf);
+
+ if (oldbuf_needs_wal || newbuf_needs_wal)
{
XLogRecPtr recptr;
@@ -3596,15 +3632,26 @@ l2:
*/
if (RelationIsAccessibleInLogicalDecoding(relation))
{
- log_heap_new_cid(relation, &oldtup);
- log_heap_new_cid(relation, heaptup);
+ if (oldbuf_needs_wal)
+ log_heap_new_cid(relation, &oldtup);
+ if (newbuf_needs_wal)
+ log_heap_new_cid(relation, heaptup);
}
- recptr = log_heap_update(relation, buffer,
- newbuf, &oldtup, heaptup,
- old_key_tuple,
- all_visible_cleared,
- all_visible_cleared_new);
+ if (oldbuf_needs_wal && newbuf_needs_wal)
+ recptr = log_heap_update(relation, buffer, newbuf,
+ &oldtup, heaptup,
+ old_key_tuple,
+ all_visible_cleared,
+ all_visible_cleared_new);
+ else if (oldbuf_needs_wal)
+ recptr = log_heap_delete(relation, buffer, &oldtup, old_key_tuple,
+ xmax_old_tuple, false,
+ all_visible_cleared);
+ else
+ recptr = log_heap_insert(relation, buffer, newtup,
+ 0, all_visible_cleared_new);
+
if (newbuf != buffer)
{
PageSetLSN(BufferGetPage(newbuf), recptr);
@@ -4482,7 +4529,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;
@@ -5234,7 +5281,7 @@ l4:
MarkBufferDirty(buf);
/* XLOG stuff */
- if (RelationNeedsWAL(rel))
+ if (BufferNeedsWAL(rel, buf))
{
xl_heap_lock_updated xlrec;
XLogRecPtr recptr;
@@ -5394,7 +5441,7 @@ heap_finish_speculative(Relation relation, ItemPointer tid)
htup->t_ctid = *tid;
/* XLOG stuff */
- if (RelationNeedsWAL(relation))
+ if (BufferNeedsWAL(relation, buffer))
{
xl_heap_confirm xlrec;
XLogRecPtr recptr;
@@ -5526,7 +5573,7 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
* 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;
@@ -5635,7 +5682,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;
@@ -6832,7 +6879,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;
@@ -6880,7 +6927,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);
@@ -7107,7 +7154,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();
@@ -8711,9 +8758,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 a3e51922d8..a05659b168 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"
@@ -258,7 +259,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 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/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5c554f9465..3f5df63df8 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -929,7 +929,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);
@@ -1193,7 +1193,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;
@@ -1575,7 +1575,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/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 64dfe06b26..1f5f7d92dd 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 "port/pg_bitutils.h"
#include "storage/bufmgr.h"
@@ -276,7 +277,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 705df8900b..1074320a5a 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2391,8 +2391,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
@@ -2438,7 +2437,7 @@ CopyFrom(CopyState cstate)
{
hi_options |= HEAP_INSERT_SKIP_FSM;
if (!XLogIsNeeded())
- hi_options |= HEAP_INSERT_SKIP_WAL;
+ heap_register_sync(cstate->rel);
}
/*
@@ -3091,11 +3090,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 36e3d44aad..8cba15fd3c 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -557,8 +557,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 */
@@ -604,9 +605,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 */
table_close(myState->rel, NoLock);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 5a47be4b33..5f447c6d94 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -463,7 +463,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 */
@@ -509,9 +509,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 */
table_close(myState->transientrel, NoLock);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 45bb0b5614..242311b0d7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4666,8 +4666,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
{
@@ -4958,8 +4959,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);
table_close(newrel, NoLock);
}
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 509394bb35..a9aec90e86 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -29,11 +29,10 @@
/* "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
-#define HEAP_INSERT_SPECULATIVE 0x0010
+#define HEAP_INSERT_SPECULATIVE 0x0008
typedef struct BulkInsertStateData *BulkInsertState;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index c2baa9d7a8..268e672470 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -94,10 +94,9 @@ typedef struct TM_FailureData
} TM_FailureData;
/* "options" flag bits for table_insert */
-#define TABLE_INSERT_SKIP_WAL 0x0001
-#define TABLE_INSERT_SKIP_FSM 0x0002
-#define TABLE_INSERT_FROZEN 0x0004
-#define TABLE_INSERT_NO_LOGICAL 0x0008
+#define TABLE_INSERT_SKIP_FSM 0x0001
+#define TABLE_INSERT_FROZEN 0x0002
+#define TABLE_INSERT_NO_LOGICAL 0x0004
/* flag bits fortable_lock_tuple */
/* Follow tuples whose update is in progress if lock modes don't conflict */
@@ -634,10 +633,6 @@ table_tuple_satisfies_snapshot(Relation rel, TupleTableSlot *slot, Snapshot snap
* behaviour of the AM. Several options might be ignored by AMs not supporting
* them.
*
- * If the TABLE_INSERT_SKIP_WAL option is specified, the new tuple will not
- * necessarily logged to WAL, even for a non-temp relation. It is the AMs
- * choice whether this optimization is supported.
- *
* If the TABLE_INSERT_SKIP_FSM option is specified, AMs are free to not reuse
* free space in the relation. This can save some cycles when we know the
* relation is new and doesn't contain useful amounts of free space. It's
--
2.16.3
----Next_Part(Mon_Mar_25_21_32_04_2019_838)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v8-0006-Change-cluster-to-use-the-new-pending-sync-infrastru.patch"
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 5/7] Fix WAL skipping feature.
In-Reply-To: <no-message-id-226527@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