public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v18 14/18] tableam: Sample Scan Support.
32+ messages / 6 participants
[nested] [flat]

* [PATCH v18 14/18] tableam: Sample Scan Support.
@ 2019-01-20 08:08 Andres Freund <[email protected]>
  0 siblings, 0 replies; 32+ messages in thread

From: Andres Freund @ 2019-01-20 08:08 UTC (permalink / raw)

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 contrib/tsm_system_rows/tsm_system_rows.c |  86 ++------
 contrib/tsm_system_time/tsm_system_time.c |  13 +-
 src/backend/access/heap/heapam_handler.c  | 218 +++++++++++++++++++
 src/backend/access/tablesample/system.c   |  11 +-
 src/backend/executor/nodeSamplescan.c     | 249 +++-------------------
 src/include/access/tableam.h              |  31 +++
 src/include/access/tsmapi.h               |   2 +-
 src/include/nodes/execnodes.h             |   4 +
 src/include/nodes/tidbitmap.h             |   2 +-
 9 files changed, 308 insertions(+), 308 deletions(-)

diff --git a/contrib/tsm_system_rows/tsm_system_rows.c b/contrib/tsm_system_rows/tsm_system_rows.c
index 1d35ea3c53a..3611d058331 100644
--- a/contrib/tsm_system_rows/tsm_system_rows.c
+++ b/contrib/tsm_system_rows/tsm_system_rows.c
@@ -28,7 +28,6 @@
 
 #include "postgres.h"
 
-#include "access/heapam.h"
 #include "access/relscan.h"
 #include "access/tsmapi.h"
 #include "catalog/pg_type.h"
@@ -46,7 +45,6 @@ typedef struct
 {
 	uint32		seed;			/* random seed */
 	int64		ntuples;		/* number of tuples to return */
-	int64		donetuples;		/* number of tuples already returned */
 	OffsetNumber lt;			/* last tuple returned from current block */
 	BlockNumber doneblocks;		/* number of already-scanned blocks */
 	BlockNumber lb;				/* last block visited */
@@ -67,11 +65,10 @@ static void system_rows_beginsamplescan(SampleScanState *node,
 							Datum *params,
 							int nparams,
 							uint32 seed);
-static BlockNumber system_rows_nextsampleblock(SampleScanState *node);
+static BlockNumber system_rows_nextsampleblock(SampleScanState *node, BlockNumber nblocks);
 static OffsetNumber system_rows_nextsampletuple(SampleScanState *node,
 							BlockNumber blockno,
 							OffsetNumber maxoffset);
-static bool SampleOffsetVisible(OffsetNumber tupoffset, HeapScanDesc scan);
 static uint32 random_relative_prime(uint32 n, SamplerRandomState randstate);
 
 
@@ -187,7 +184,6 @@ system_rows_beginsamplescan(SampleScanState *node,
 
 	sampler->seed = seed;
 	sampler->ntuples = ntuples;
-	sampler->donetuples = 0;
 	sampler->lt = InvalidOffsetNumber;
 	sampler->doneblocks = 0;
 	/* lb will be initialized during first NextSampleBlock call */
@@ -206,11 +202,9 @@ system_rows_beginsamplescan(SampleScanState *node,
  * Uses linear probing algorithm for picking next block.
  */
 static BlockNumber
-system_rows_nextsampleblock(SampleScanState *node)
+system_rows_nextsampleblock(SampleScanState *node, BlockNumber nblocks)
 {
 	SystemRowsSamplerData *sampler = (SystemRowsSamplerData *) node->tsm_state;
-	TableScanDesc scan = node->ss.ss_currentScanDesc;
-	HeapScanDesc hscan = (HeapScanDesc) scan;
 
 	/* First call within scan? */
 	if (sampler->doneblocks == 0)
@@ -222,14 +216,14 @@ system_rows_nextsampleblock(SampleScanState *node)
 			SamplerRandomState randstate;
 
 			/* If relation is empty, there's nothing to scan */
-			if (hscan->rs_nblocks == 0)
+			if (nblocks == 0)
 				return InvalidBlockNumber;
 
 			/* We only need an RNG during this setup step */
 			sampler_random_init_state(sampler->seed, randstate);
 
 			/* Compute nblocks/firstblock/step only once per query */
-			sampler->nblocks = hscan->rs_nblocks;
+			sampler->nblocks = nblocks;
 
 			/* Choose random starting block within the relation */
 			/* (Actually this is the predecessor of the first block visited) */
@@ -246,7 +240,7 @@ system_rows_nextsampleblock(SampleScanState *node)
 
 	/* If we've read all blocks or returned all needed tuples, we're done */
 	if (++sampler->doneblocks > sampler->nblocks ||
-		sampler->donetuples >= sampler->ntuples)
+		node->donetuples >= sampler->ntuples)
 		return InvalidBlockNumber;
 
 	/*
@@ -259,7 +253,7 @@ system_rows_nextsampleblock(SampleScanState *node)
 	{
 		/* Advance lb, using uint64 arithmetic to forestall overflow */
 		sampler->lb = ((uint64) sampler->lb + sampler->step) % sampler->nblocks;
-	} while (sampler->lb >= hscan->rs_nblocks);
+	} while (sampler->lb >= nblocks);
 
 	return sampler->lb;
 }
@@ -279,77 +273,27 @@ system_rows_nextsampletuple(SampleScanState *node,
 							OffsetNumber maxoffset)
 {
 	SystemRowsSamplerData *sampler = (SystemRowsSamplerData *) node->tsm_state;
-	TableScanDesc scan = node->ss.ss_currentScanDesc;
-	HeapScanDesc hscan = (HeapScanDesc) scan;
 	OffsetNumber tupoffset = sampler->lt;
 
 	/* Quit if we've returned all needed tuples */
-	if (sampler->donetuples >= sampler->ntuples)
+	if (node->donetuples >= sampler->ntuples)
 		return InvalidOffsetNumber;
 
-	/*
-	 * Because we should only count visible tuples as being returned, we need
-	 * to search for a visible tuple rather than just let the core code do it.
-	 */
+	/* Advance to next possible offset on page */
+	if (tupoffset == InvalidOffsetNumber)
+		tupoffset = FirstOffsetNumber;
+	else
+		tupoffset++;
 
-	/* We rely on the data accumulated in pagemode access */
-	Assert(scan->rs_pageatatime);
-	for (;;)
-	{
-		/* Advance to next possible offset on page */
-		if (tupoffset == InvalidOffsetNumber)
-			tupoffset = FirstOffsetNumber;
-		else
-			tupoffset++;
-
-		/* Done? */
-		if (tupoffset > maxoffset)
-		{
-			tupoffset = InvalidOffsetNumber;
-			break;
-		}
-
-		/* Found a candidate? */
-		if (SampleOffsetVisible(tupoffset, hscan))
-		{
-			sampler->donetuples++;
-			break;
-		}
-	}
+	/* Done? */
+	if (tupoffset > maxoffset)
+		tupoffset = InvalidOffsetNumber;
 
 	sampler->lt = tupoffset;
 
 	return tupoffset;
 }
 
-/*
- * Check if tuple offset is visible
- *
- * In pageatatime mode, heapgetpage() already did visibility checks,
- * so just look at the info it left in rs_vistuples[].
- */
-static bool
-SampleOffsetVisible(OffsetNumber tupoffset, HeapScanDesc scan)
-{
-	int			start = 0,
-				end = scan->rs_ntuples - 1;
-
-	while (start <= end)
-	{
-		int			mid = (start + end) / 2;
-		OffsetNumber curoffset = scan->rs_vistuples[mid];
-
-		if (tupoffset == curoffset)
-			return true;
-		else if (tupoffset < curoffset)
-			end = mid - 1;
-		else
-			start = mid + 1;
-	}
-
-	return false;
-}
-
 /*
  * Compute greatest common divisor of two uint32's.
  */
diff --git a/contrib/tsm_system_time/tsm_system_time.c b/contrib/tsm_system_time/tsm_system_time.c
index 1cc7264e084..ab9f685f0af 100644
--- a/contrib/tsm_system_time/tsm_system_time.c
+++ b/contrib/tsm_system_time/tsm_system_time.c
@@ -26,7 +26,6 @@
 
 #include <math.h>
 
-#include "access/heapam.h"
 #include "access/relscan.h"
 #include "access/tsmapi.h"
 #include "catalog/pg_type.h"
@@ -66,7 +65,7 @@ static void system_time_beginsamplescan(SampleScanState *node,
 							Datum *params,
 							int nparams,
 							uint32 seed);
-static BlockNumber system_time_nextsampleblock(SampleScanState *node);
+static BlockNumber system_time_nextsampleblock(SampleScanState *node, BlockNumber nblocks);
 static OffsetNumber system_time_nextsampletuple(SampleScanState *node,
 							BlockNumber blockno,
 							OffsetNumber maxoffset);
@@ -213,11 +212,9 @@ system_time_beginsamplescan(SampleScanState *node,
  * Uses linear probing algorithm for picking next block.
  */
 static BlockNumber
-system_time_nextsampleblock(SampleScanState *node)
+system_time_nextsampleblock(SampleScanState *node, BlockNumber nblocks)
 {
 	SystemTimeSamplerData *sampler = (SystemTimeSamplerData *) node->tsm_state;
-	TableScanDesc scan = node->ss.ss_currentScanDesc;
-	HeapScanDesc hscan = (HeapScanDesc) scan;
 	instr_time	cur_time;
 
 	/* First call within scan? */
@@ -230,14 +227,14 @@ system_time_nextsampleblock(SampleScanState *node)
 			SamplerRandomState randstate;
 
 			/* If relation is empty, there's nothing to scan */
-			if (hscan->rs_nblocks == 0)
+			if (nblocks == 0)
 				return InvalidBlockNumber;
 
 			/* We only need an RNG during this setup step */
 			sampler_random_init_state(sampler->seed, randstate);
 
 			/* Compute nblocks/firstblock/step only once per query */
-			sampler->nblocks = hscan->rs_nblocks;
+			sampler->nblocks = nblocks;
 
 			/* Choose random starting block within the relation */
 			/* (Actually this is the predecessor of the first block visited) */
@@ -273,7 +270,7 @@ system_time_nextsampleblock(SampleScanState *node)
 	{
 		/* Advance lb, using uint64 arithmetic to forestall overflow */
 		sampler->lb = ((uint64) sampler->lb + sampler->step) % sampler->nblocks;
-	} while (sampler->lb >= hscan->rs_nblocks);
+	} while (sampler->lb >= nblocks);
 
 	return sampler->lb;
 }
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index d13f934833a..f71b9b2a062 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -27,6 +27,7 @@
 #include "access/heapam.h"
 #include "access/multixact.h"
 #include "access/tableam.h"
+#include "access/tsmapi.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
 #include "catalog/index.h"
@@ -34,6 +35,7 @@
 #include "catalog/storage_xlog.h"
 #include "executor/executor.h"
 #include "optimizer/plancat.h"
+#include "pgstat.h"
 #include "storage/bufmgr.h"
 #include "storage/bufpage.h"
 #include "storage/bufmgr.h"
@@ -1804,6 +1806,219 @@ heapam_index_validate_scan(Relation heapRelation,
 	indexInfo->ii_PredicateState = NULL;
 }
 
+/*
+ * Check visibility of the tuple.
+ */
+static bool
+SampleHeapTupleVisible(HeapScanDesc scan, Buffer buffer,
+					   HeapTuple tuple,
+					   OffsetNumber tupoffset)
+{
+	if (scan->rs_base.rs_pageatatime)
+	{
+		/*
+		 * In pageatatime mode, heapgetpage() already did visibility checks,
+		 * so just look at the info it left in rs_vistuples[].
+		 *
+		 * We use a binary search over the known-sorted array.  Note: we could
+		 * save some effort if we insisted that NextSampleTuple select tuples
+		 * in increasing order, but it's not clear that there would be enough
+		 * gain to justify the restriction.
+		 */
+		int			start = 0,
+					end = scan->rs_ntuples - 1;
+
+		while (start <= end)
+		{
+			int			mid = (start + end) / 2;
+			OffsetNumber curoffset = scan->rs_vistuples[mid];
+
+			if (tupoffset == curoffset)
+				return true;
+			else if (tupoffset < curoffset)
+				end = mid - 1;
+			else
+				start = mid + 1;
+		}
+
+		return false;
+	}
+	else
+	{
+		/* Otherwise, we have to check the tuple individually. */
+		return HeapTupleSatisfiesVisibility(tuple, scan->rs_base.rs_snapshot,
+											buffer);
+	}
+}
+
+static bool
+heapam_scan_sample_next_block(TableScanDesc sscan, struct SampleScanState *scanstate)
+{
+	HeapScanDesc scan = (HeapScanDesc) sscan;
+	TsmRoutine *tsm = scanstate->tsmroutine;
+	BlockNumber blockno;
+
+	/* return false immediately if relation is empty */
+	if (scan->rs_nblocks == 0)
+		return false;
+
+	if (tsm->NextSampleBlock)
+	{
+		blockno = tsm->NextSampleBlock(scanstate, scan->rs_nblocks);
+		scan->rs_cblock = blockno;
+	}
+	else
+	{
+		/* scanning table sequentially */
+
+		if (scan->rs_cblock == InvalidBlockNumber)
+		{
+			Assert(!scan->rs_inited);
+			blockno = scan->rs_startblock;
+		}
+		else
+		{
+			Assert(scan->rs_inited);
+
+			blockno = scan->rs_cblock + 1;
+
+			if (blockno >= scan->rs_nblocks)
+			{
+				/* wrap to begining of rel, might not have started at 0 */
+				blockno = 0;
+			}
+
+			/*
+			 * Report our new scan position for synchronization purposes.
+			 *
+			 * Note: we do this before checking for end of scan so that the
+			 * final state of the position hint is back at the start of the
+			 * rel.  That's not strictly necessary, but otherwise when you run
+			 * the same query multiple times the starting position would shift
+			 * a little bit backwards on every invocation, which is confusing.
+			 * We don't guarantee any specific ordering in general, though.
+			 */
+			if (scan->rs_base.rs_syncscan)
+				ss_report_location(scan->rs_base.rs_rd, blockno);
+
+			if (blockno == scan->rs_startblock)
+			{
+				blockno = InvalidBlockNumber;
+			}
+		}
+	}
+
+	if (!BlockNumberIsValid(blockno))
+	{
+		if (BufferIsValid(scan->rs_cbuf))
+			ReleaseBuffer(scan->rs_cbuf);
+		scan->rs_cbuf = InvalidBuffer;
+		scan->rs_cblock = InvalidBlockNumber;
+		scan->rs_inited = false;
+
+		return false;
+	}
+
+	heapgetpage(sscan, blockno);
+	scan->rs_inited = true;
+
+	return true;
+}
+
+static bool
+heapam_scan_sample_next_tuple(TableScanDesc sscan, struct SampleScanState *scanstate, TupleTableSlot *slot)
+{
+	HeapScanDesc scan = (HeapScanDesc) sscan;
+	TsmRoutine *tsm = scanstate->tsmroutine;
+	BlockNumber blockno = scan->rs_cblock;
+	bool		pagemode = scan->rs_base.rs_pageatatime;
+
+	Page		page;
+	bool		all_visible;
+	OffsetNumber maxoffset;
+
+	ExecClearTuple(slot);
+
+	/*
+	 * When not using pagemode, we must lock the buffer during tuple
+	 * visibility checks.
+	 */
+	if (!pagemode)
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+
+	page = (Page) BufferGetPage(scan->rs_cbuf);
+	all_visible = PageIsAllVisible(page) &&
+		!scan->rs_base.rs_snapshot->takenDuringRecovery;
+	maxoffset = PageGetMaxOffsetNumber(page);
+
+	for (;;)
+	{
+		OffsetNumber tupoffset;
+
+		CHECK_FOR_INTERRUPTS();
+
+		/* Ask the tablesample method which tuples to check on this page. */
+		tupoffset = tsm->NextSampleTuple(scanstate,
+										 blockno,
+										 maxoffset);
+
+		if (OffsetNumberIsValid(tupoffset))
+		{
+			ItemId		itemid;
+			bool		visible;
+			HeapTuple	tuple = &(scan->rs_ctup);
+
+			/* Skip invalid tuple pointers. */
+			itemid = PageGetItemId(page, tupoffset);
+			if (!ItemIdIsNormal(itemid))
+				continue;
+
+			tuple->t_data = (HeapTupleHeader) PageGetItem(page, itemid);
+			tuple->t_len = ItemIdGetLength(itemid);
+			ItemPointerSet(&(tuple->t_self), blockno, tupoffset);
+
+
+			if (all_visible)
+				visible = true;
+			else
+				visible = SampleHeapTupleVisible(scan, scan->rs_cbuf, tuple, tupoffset);
+
+			/* in pagemode, heapgetpage did this for us */
+			if (!pagemode)
+				CheckForSerializableConflictOut(visible, scan->rs_base.rs_rd, tuple,
+												scan->rs_cbuf, scan->rs_base.rs_snapshot);
+
+			/* Try next tuple from same page. */
+			if (!visible)
+				continue;
+
+			/* Found visible tuple, return it. */
+			if (!pagemode)
+				LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+
+			ExecStoreBufferHeapTuple(tuple, slot, scan->rs_cbuf);
+
+			/* Count successfully-fetched tuples as heap fetches */
+			pgstat_count_heap_getnext(scan->rs_base.rs_rd);
+
+			return true;
+		}
+		else
+		{
+			/*
+			 * If we get here, it means we've exhausted the items on this page
+			 * and it's time to move to the next.
+			 */
+			if (!pagemode)
+				LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+
+			break;
+		}
+	}
+
+	return false;
+}
+
 /*
  * Reconstruct and rewrite the given tuple
  *
@@ -1992,6 +2207,9 @@ static const TableAmRoutine heapam_methods = {
 	.index_validate_scan = heapam_index_validate_scan,
 
 	.relation_estimate_size = heapam_estimate_rel_size,
+
+	.scan_sample_next_block = heapam_scan_sample_next_block,
+	.scan_sample_next_tuple = heapam_scan_sample_next_tuple
 };
 
 
diff --git a/src/backend/access/tablesample/system.c b/src/backend/access/tablesample/system.c
index fe62a73341e..476411caf1a 100644
--- a/src/backend/access/tablesample/system.c
+++ b/src/backend/access/tablesample/system.c
@@ -27,7 +27,6 @@
 #include <math.h>
 
 #include "access/hash.h"
-#include "access/heapam.h"
 #include "access/relscan.h"
 #include "access/tsmapi.h"
 #include "catalog/pg_type.h"
@@ -56,7 +55,7 @@ static void system_beginsamplescan(SampleScanState *node,
 					   Datum *params,
 					   int nparams,
 					   uint32 seed);
-static BlockNumber system_nextsampleblock(SampleScanState *node);
+static BlockNumber system_nextsampleblock(SampleScanState *node, BlockNumber nblocks);
 static OffsetNumber system_nextsampletuple(SampleScanState *node,
 					   BlockNumber blockno,
 					   OffsetNumber maxoffset);
@@ -177,11 +176,9 @@ system_beginsamplescan(SampleScanState *node,
  * Select next block to sample.
  */
 static BlockNumber
-system_nextsampleblock(SampleScanState *node)
+system_nextsampleblock(SampleScanState *node, BlockNumber nblocks)
 {
 	SystemSamplerData *sampler = (SystemSamplerData *) node->tsm_state;
-	TableScanDesc scan = node->ss.ss_currentScanDesc;
-	HeapScanDesc hscan = (HeapScanDesc) scan;
 	BlockNumber nextblock = sampler->nextblock;
 	uint32		hashinput[2];
 
@@ -200,7 +197,7 @@ system_nextsampleblock(SampleScanState *node)
 	 * Loop over block numbers until finding suitable block or reaching end of
 	 * relation.
 	 */
-	for (; nextblock < hscan->rs_nblocks; nextblock++)
+	for (; nextblock < nblocks; nextblock++)
 	{
 		uint32		hash;
 
@@ -212,7 +209,7 @@ system_nextsampleblock(SampleScanState *node)
 			break;
 	}
 
-	if (nextblock < hscan->rs_nblocks)
+	if (nextblock < nblocks)
 	{
 		/* Found a suitable block; remember where we should start next time */
 		sampler->nextblock = nextblock + 1;
diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c
index 3a00d648099..ce0d3bfa572 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -15,7 +15,6 @@
 #include "postgres.h"
 
 #include "access/hash.h"
-#include "access/heapam.h"
 #include "access/relscan.h"
 #include "access/tableam.h"
 #include "access/tsmapi.h"
@@ -29,9 +28,7 @@
 
 static TupleTableSlot *SampleNext(SampleScanState *node);
 static void tablesample_init(SampleScanState *scanstate);
-static HeapTuple tablesample_getnext(SampleScanState *scanstate);
-static bool SampleTupleVisible(HeapTuple tuple, OffsetNumber tupoffset,
-				   HeapScanDesc scan);
+static TupleTableSlot *tablesample_getnext(SampleScanState *scanstate);
 
 /* ----------------------------------------------------------------
  *						Scan Support
@@ -47,10 +44,6 @@ static bool SampleTupleVisible(HeapTuple tuple, OffsetNumber tupoffset,
 static TupleTableSlot *
 SampleNext(SampleScanState *node)
 {
-	HeapTuple	tuple;
-	TupleTableSlot *slot;
-	HeapScanDesc hscan;
-
 	/*
 	 * if this is first call within a scan, initialize
 	 */
@@ -60,19 +53,7 @@ SampleNext(SampleScanState *node)
 	/*
 	 * get the next tuple, and store it in our result slot
 	 */
-	tuple = tablesample_getnext(node);
-
-	slot = node->ss.ss_ScanTupleSlot;
-	hscan = (HeapScanDesc) node->ss.ss_currentScanDesc;
-
-	if (tuple)
-		ExecStoreBufferHeapTuple(tuple, /* tuple to store */
-								 slot,	/* slot to store in */
-								 hscan->rs_cbuf); /* tuple's buffer */
-	else
-		ExecClearTuple(slot);
-
-	return slot;
+	return tablesample_getnext(node);
 }
 
 /*
@@ -237,6 +218,9 @@ ExecReScanSampleScan(SampleScanState *node)
 {
 	/* Remember we need to do BeginSampleScan again (if we did it at all) */
 	node->begun = false;
+	node->done = false;
+	node->haveblock = false;
+	node->donetuples = 0;
 
 	ExecScanReScan(&node->ss);
 }
@@ -258,6 +242,7 @@ tablesample_init(SampleScanState *scanstate)
 	int			i;
 	ListCell   *arg;
 
+	scanstate->donetuples = 0;
 	params = (Datum *) palloc(list_length(scanstate->args) * sizeof(Datum));
 
 	i = 0;
@@ -345,225 +330,49 @@ tablesample_init(SampleScanState *scanstate)
 
 /*
  * Get next tuple from TABLESAMPLE method.
- *
- * Note: an awful lot of this is copied-and-pasted from heapam.c.  It would
- * perhaps be better to refactor to share more code.
  */
-static HeapTuple
+static TupleTableSlot *
 tablesample_getnext(SampleScanState *scanstate)
 {
-	TsmRoutine *tsm = scanstate->tsmroutine;
 	TableScanDesc scan = scanstate->ss.ss_currentScanDesc;
-	HeapScanDesc hscan = (HeapScanDesc) scan;
-	HeapTuple	tuple = &(hscan->rs_ctup);
-	Snapshot	snapshot = scan->rs_snapshot;
-	bool		pagemode = scan->rs_pageatatime;
-	BlockNumber blockno;
-	Page		page;
-	bool		all_visible;
-	OffsetNumber maxoffset;
+	TupleTableSlot *slot = scanstate->ss.ss_ScanTupleSlot;
 
-	if (!hscan->rs_inited)
-	{
-		/*
-		 * return null immediately if relation is empty
-		 */
-		if (hscan->rs_nblocks == 0)
-		{
-			Assert(!BufferIsValid(hscan->rs_cbuf));
-			tuple->t_data = NULL;
-			return NULL;
-		}
-		if (tsm->NextSampleBlock)
-		{
-			blockno = tsm->NextSampleBlock(scanstate);
-			if (!BlockNumberIsValid(blockno))
-			{
-				tuple->t_data = NULL;
-				return NULL;
-			}
-		}
-		else
-			blockno = hscan->rs_startblock;
-		Assert(blockno < hscan->rs_nblocks);
-		heapgetpage(scan, blockno);
-		hscan->rs_inited = true;
-	}
-	else
-	{
-		/* continue from previously returned page/tuple */
-		blockno = hscan->rs_cblock;	/* current page */
-	}
+	ExecClearTuple(slot);
 
-	/*
-	 * When not using pagemode, we must lock the buffer during tuple
-	 * visibility checks.
-	 */
-	if (!pagemode)
-		LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_SHARE);
-
-	page = (Page) BufferGetPage(hscan->rs_cbuf);
-	all_visible = PageIsAllVisible(page) && !snapshot->takenDuringRecovery;
-	maxoffset = PageGetMaxOffsetNumber(page);
+	if (scanstate->done)
+		return NULL;
 
 	for (;;)
 	{
-		OffsetNumber tupoffset;
-		bool		finished;
-
-		CHECK_FOR_INTERRUPTS();
-
-		/* Ask the tablesample method which tuples to check on this page. */
-		tupoffset = tsm->NextSampleTuple(scanstate,
-										 blockno,
-										 maxoffset);
-
-		if (OffsetNumberIsValid(tupoffset))
+		if (!scanstate->haveblock)
 		{
-			ItemId		itemid;
-			bool		visible;
-
-			/* Skip invalid tuple pointers. */
-			itemid = PageGetItemId(page, tupoffset);
-			if (!ItemIdIsNormal(itemid))
-				continue;
-
-			tuple->t_data = (HeapTupleHeader) PageGetItem(page, itemid);
-			tuple->t_len = ItemIdGetLength(itemid);
-			ItemPointerSet(&(tuple->t_self), blockno, tupoffset);
-
-			if (all_visible)
-				visible = true;
-			else
-				visible = SampleTupleVisible(tuple, tupoffset, hscan);
-
-			/* in pagemode, heapgetpage did this for us */
-			if (!pagemode)
-				CheckForSerializableConflictOut(visible, scan->rs_rd, tuple,
-												hscan->rs_cbuf, snapshot);
-
-			if (visible)
+			if (!table_scan_sample_next_block(scan, scanstate))
 			{
-				/* Found visible tuple, return it. */
-				if (!pagemode)
-					LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_UNLOCK);
-				break;
-			}
-			else
-			{
-				/* Try next tuple from same page. */
-				continue;
+				scanstate->haveblock = false;
+				scanstate->done = true;
+
+				/* exhausted relation */
+				return NULL;
 			}
+
+			scanstate->haveblock = true;
 		}
 
-		/*
-		 * if we get here, it means we've exhausted the items on this page and
-		 * it's time to move to the next.
-		 */
-		if (!pagemode)
-			LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_UNLOCK);
-
-		if (tsm->NextSampleBlock)
+		if (!table_scan_sample_next_tuple(scan, scanstate, slot))
 		{
-			blockno = tsm->NextSampleBlock(scanstate);
-			Assert(!scan->rs_syncscan);
-			finished = !BlockNumberIsValid(blockno);
-		}
-		else
-		{
-			/* Without NextSampleBlock, just do a plain forward seqscan. */
-			blockno++;
-			if (blockno >= hscan->rs_nblocks)
-				blockno = 0;
-
 			/*
-			 * Report our new scan position for synchronization purposes.
-			 *
-			 * Note: we do this before checking for end of scan so that the
-			 * final state of the position hint is back at the start of the
-			 * rel.  That's not strictly necessary, but otherwise when you run
-			 * the same query multiple times the starting position would shift
-			 * a little bit backwards on every invocation, which is confusing.
-			 * We don't guarantee any specific ordering in general, though.
+			 * If we get here, it means we've exhausted the items on this page
+			 * and it's time to move to the next.
 			 */
-			if (scan->rs_syncscan)
-				ss_report_location(scan->rs_rd, blockno);
-
-			finished = (blockno == hscan->rs_startblock);
+			scanstate->haveblock = false;
+			continue;
 		}
 
-		/*
-		 * Reached end of scan?
-		 */
-		if (finished)
-		{
-			if (BufferIsValid(hscan->rs_cbuf))
-				ReleaseBuffer(hscan->rs_cbuf);
-			hscan->rs_cbuf = InvalidBuffer;
-			hscan->rs_cblock = InvalidBlockNumber;
-			tuple->t_data = NULL;
-			hscan->rs_inited = false;
-			return NULL;
-		}
-
-		Assert(blockno < hscan->rs_nblocks);
-		heapgetpage(scan, blockno);
-
-		/* Re-establish state for new page */
-		if (!pagemode)
-			LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_SHARE);
-
-		page = (Page) BufferGetPage(hscan->rs_cbuf);
-		all_visible = PageIsAllVisible(page) && !snapshot->takenDuringRecovery;
-		maxoffset = PageGetMaxOffsetNumber(page);
+		/* Found visible tuple, return it. */
+		break;
 	}
 
-	/* Count successfully-fetched tuples as heap fetches */
-	pgstat_count_heap_getnext(scan->rs_rd);
+	scanstate->donetuples++;
 
-	return &(hscan->rs_ctup);
-}
-
-/*
- * Check visibility of the tuple.
- */
-static bool
-SampleTupleVisible(HeapTuple tuple, OffsetNumber tupoffset, HeapScanDesc scan)
-{
-	if (scan->rs_base.rs_pageatatime)
-	{
-		/*
-		 * In pageatatime mode, heapgetpage() already did visibility checks,
-		 * so just look at the info it left in rs_vistuples[].
-		 *
-		 * We use a binary search over the known-sorted array.  Note: we could
-		 * save some effort if we insisted that NextSampleTuple select tuples
-		 * in increasing order, but it's not clear that there would be enough
-		 * gain to justify the restriction.
-		 */
-		int			start = 0,
-					end = scan->rs_ntuples - 1;
-
-		while (start <= end)
-		{
-			int			mid = (start + end) / 2;
-			OffsetNumber curoffset = scan->rs_vistuples[mid];
-
-			if (tupoffset == curoffset)
-				return true;
-			else if (tupoffset < curoffset)
-				end = mid - 1;
-			else
-				start = mid + 1;
-		}
-
-		return false;
-	}
-	else
-	{
-		/* Otherwise, we have to check the tuple individually. */
-		return HeapTupleSatisfiesVisibility(tuple,
-											scan->rs_base.rs_snapshot,
-											scan->rs_cbuf);
-	}
+	return slot;
 }
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 2fc144c3665..0c9339c676e 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -30,6 +30,7 @@ extern bool synchronize_seqscans;
 
 struct VacuumParams;
 struct ValidateIndexState;
+struct SampleScanState;
 struct BulkInsertStateData;
 
 /*
@@ -344,6 +345,18 @@ typedef struct TableAmRoutine
 	void		(*relation_estimate_size) (Relation rel, int32 *attr_widths,
 										   BlockNumber *pages, double *tuples, double *allvisfrac);
 
+
+	/* ------------------------------------------------------------------------
+	 * Executor related functions.
+	 * ------------------------------------------------------------------------
+	 */
+
+	bool		(*scan_sample_next_block) (TableScanDesc scan,
+										   struct SampleScanState *scanstate);
+	bool		(*scan_sample_next_tuple) (TableScanDesc scan,
+										   struct SampleScanState *scanstate,
+										   TupleTableSlot *slot);
+
 } TableAmRoutine;
 
 
@@ -887,6 +900,24 @@ table_estimate_size(Relation rel, int32 *attr_widths,
 }
 
 
+/* ----------------------------------------------------------------------------
+ * Executor related functionality
+ * ----------------------------------------------------------------------------
+ */
+
+static inline bool
+table_scan_sample_next_block(TableScanDesc scan, struct SampleScanState *scanstate)
+{
+	return scan->rs_rd->rd_tableam->scan_sample_next_block(scan, scanstate);
+}
+
+static inline bool
+table_scan_sample_next_tuple(TableScanDesc scan, struct SampleScanState *scanstate, TupleTableSlot *slot)
+{
+	return scan->rs_rd->rd_tableam->scan_sample_next_tuple(scan, scanstate, slot);
+}
+
+
 /* ----------------------------------------------------------------------------
  * Functions to make modifications a bit simpler.
  * ----------------------------------------------------------------------------
diff --git a/src/include/access/tsmapi.h b/src/include/access/tsmapi.h
index a5c0b4cafec..ccef65aedb4 100644
--- a/src/include/access/tsmapi.h
+++ b/src/include/access/tsmapi.h
@@ -34,7 +34,7 @@ typedef void (*BeginSampleScan_function) (SampleScanState *node,
 										  int nparams,
 										  uint32 seed);
 
-typedef BlockNumber (*NextSampleBlock_function) (SampleScanState *node);
+typedef BlockNumber (*NextSampleBlock_function) (SampleScanState *node, BlockNumber nblocks);
 
 typedef OffsetNumber (*NextSampleTuple_function) (SampleScanState *node,
 												  BlockNumber blockno,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 62eb1a06eef..22ac3b17518 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1300,6 +1300,9 @@ typedef struct SampleScanState
 	bool		use_pagemode;	/* use page-at-a-time visibility checking? */
 	bool		begun;			/* false means need to call BeginSampleScan */
 	uint32		seed;			/* random seed */
+	int64		donetuples;		/* number of tuples already returned */
+	bool		haveblock;		/* has a block for sampling been determined */
+	bool		done;			/* exhausted all tuples? */
 } SampleScanState;
 
 /*
@@ -1528,6 +1531,7 @@ typedef struct BitmapHeapScanState
 	Buffer		pvmbuffer;
 	long		exact_pages;
 	long		lossy_pages;
+	int			return_empty_tuples;
 	TBMIterator *prefetch_iterator;
 	int			prefetch_pages;
 	int			prefetch_target;
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 2645085b344..6a7f3054a41 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -37,7 +37,7 @@ typedef struct TBMIterator TBMIterator;
 typedef struct TBMSharedIterator TBMSharedIterator;
 
 /* Result structure for tbm_iterate */
-typedef struct
+typedef struct TBMIterateResult
 {
 	BlockNumber blockno;		/* page number containing tuples */
 	int			ntuples;		/* -1 indicates lossy result */
-- 
2.21.0.dirty


--yvn3crbc4qf4vymf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v18-0015-tableam-bitmap-heap-scan.patch"



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* do only critical work during single-user vacuum?
@ 2021-12-09 19:28 John Naylor <[email protected]>
  2021-12-09 20:32 ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 22:08 ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  0 siblings, 3 replies; 32+ messages in thread

From: John Naylor @ 2021-12-09 19:28 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

When a user must shut down and restart in single-user mode to run
vacuum on an entire database, that does a lot of work that's
unnecessary for getting the system online again, even without
index_cleanup. We had a recent case where a single-user vacuum took
around 3 days to complete.

Now that we have a concept of a fail-safe vacuum, maybe it would be
beneficial to skip a vacuum in single-user mode if the fail-safe
criteria were not met at the beginning of vacuuming a relation. This
is not without risk, of course, but it should be much faster than
today and once up and running the admin would have a chance to get a
handle on things. Thoughts?

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-09 20:32 ` Bossart, Nathan <[email protected]>
  2 siblings, 0 replies; 32+ messages in thread

From: Bossart, Nathan @ 2021-12-09 20:32 UTC (permalink / raw)
  To: John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

On 12/9/21, 11:34 AM, "John Naylor" <[email protected]> wrote:
> When a user must shut down and restart in single-user mode to run
> vacuum on an entire database, that does a lot of work that's
> unnecessary for getting the system online again, even without
> index_cleanup. We had a recent case where a single-user vacuum took
> around 3 days to complete.
>
> Now that we have a concept of a fail-safe vacuum, maybe it would be
> beneficial to skip a vacuum in single-user mode if the fail-safe
> criteria were not met at the beginning of vacuuming a relation. This
> is not without risk, of course, but it should be much faster than
> today and once up and running the admin would have a chance to get a
> handle on things. Thoughts?

Would the --min-xid-age and --no-index-cleanup vacuumdb options help
with this?

Nathan



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-09 21:04 ` Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2 siblings, 1 reply; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-09 21:04 UTC (permalink / raw)
  To: John Naylor <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 11:28 AM John Naylor
<[email protected]> wrote:
> Now that we have a concept of a fail-safe vacuum, maybe it would be
> beneficial to skip a vacuum in single-user mode if the fail-safe
> criteria were not met at the beginning of vacuuming a relation.

Obviously the main goal of the failsafe is to not get into this
situation in the first place. But it's still very reasonable to ask
"what happens when the failsafe even fails at that?". This was
something that we considered directly when working on the feature.

There is a precheck that takes place before any other work, which
ensures that we won't even start off any of the nonessential tasks the
failsafe skips (e.g., index vacuuming). The precheck works like any
other check -- it checks if relfrozenxid is dangerously old. (We won't
even bother trying to launch parallel workers when this precheck
triggers, which is another reason to have it that Mashahiko pointed
out during development.)

Presumably there is no need to specifically check if we're running in
single user mode when considering if we need to trigger the failsafe
-- which, as you say, we won't do. It shouldn't matter, because
anybody running single-user mode just to VACUUM must already be unable
to allocate new XIDs outside of single user mode. That condition alone
will trigger the failsafe.

That said, it would be very easy to add a check for single user mode.
It didn't happen because we weren't aware of any specific need for it.
Perhaps there is an argument for it.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-09 21:12   ` Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-09 21:12 UTC (permalink / raw)
  To: John Naylor <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 1:04 PM Peter Geoghegan <[email protected]> wrote:
> On Thu, Dec 9, 2021 at 11:28 AM John Naylor
> <[email protected]> wrote:
> > Now that we have a concept of a fail-safe vacuum, maybe it would be
> > beneficial to skip a vacuum in single-user mode if the fail-safe
> > criteria were not met at the beginning of vacuuming a relation.
>
> Obviously the main goal of the failsafe is to not get into this
> situation in the first place. But it's still very reasonable to ask
> "what happens when the failsafe even fails at that?". This was
> something that we considered directly when working on the feature.

Oh, I think I misunderstood. Your concern is for the case where the
DBA runs a simple "VACUUM" in single-user mode; you want to skip over
tables that don't really need to advance relfrozenxid, automatically.

I can see an argument for something like that, but I think that it
should be a variant of VACUUM. Or maybe it could be addressed with a
better user interface; single-user mode should prompt the user about
what exact VACUUM command they ought to run to get things going.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-09 23:53     ` John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: John Naylor @ 2021-12-09 23:53 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; Andres Freund <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 5:13 PM Peter Geoghegan <[email protected]> wrote:
> Oh, I think I misunderstood. Your concern is for the case where the
> DBA runs a simple "VACUUM" in single-user mode; you want to skip over
> tables that don't really need to advance relfrozenxid, automatically.

Right.

> I can see an argument for something like that, but I think that it
> should be a variant of VACUUM. Or maybe it could be addressed with a
> better user interface;

On Thu, Dec 9, 2021 at 6:08 PM Andres Freund <[email protected]> wrote:
> What if the user tried to reclaim space by vacuuming (via truncation)? Or is
> working around some corruption or such?  I think this is too much magic.
>
> That said, having a VACUUM "selector" that selects the oldest tables could be
> quite useful. And address this usecase both for single-user and normal
> operation.

All good points.

[Peter again]
> single-user mode should prompt the user about
> what exact VACUUM command they ought to run to get things going.

The current message is particularly bad in its vagueness because some
users immediately reach for VACUUM FULL, which quite logically seems
like the most complete thing to do.

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-10 00:34       ` Peter Geoghegan <[email protected]>
  2021-12-10 01:05         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  0 siblings, 3 replies; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-10 00:34 UTC (permalink / raw)
  To: John Naylor <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 3:53 PM John Naylor <[email protected]> wrote:
> > single-user mode should prompt the user about
> > what exact VACUUM command they ought to run to get things going.
>
> The current message is particularly bad in its vagueness because some
> users immediately reach for VACUUM FULL, which quite logically seems
> like the most complete thing to do.

You mean the GetNewTransactionId() error, about single-user mode? Why
do we need to use single-user mode at all? I'm pretty sure that the
reason is "as an escape hatch", but I wonder what that really means.

***Thinks***

I suppose that it might be a good idea to make sure that autovacuum
cannot run, because in general autovacuum might need to allocate an
XID (for autoanalyze), and locking all that down in exactly the right
way might not be a very good use of our time.

But even still, why not have some variant of single-user mode just for
this task? Something that's easy to use when the DBA is rudely
awakened at 4am -- something a little bit like a big red button that
fixes the exact problem of XID exhaustion, in a reasonably targeted
way? I don't think that this needs to involve the VACUUM command
itself.

The current recommendation to do a whole-database VACUUM doesn't take
a position on how old the oldest datfrozenxid has to be in order to
become safe again, preferring to "make a conservative recommendation"
-- which is what a database-level VACUUM really is. But that doesn't
seem helpful at all. In fact, it's not even conservative. We could
easily come up with a reasonable definition of "datfrozenxid that's
sufficiently new to make it safe to come back online and allocate XIDs
again". Perhaps something based on the current
autovacuum_freeze_max_age (and autovacuum_multixact_freeze_max_age)
settings, with sanity checks.

We could then apply this criteria in new code that implements this
"big red button" (maybe this is a new option for the postgres
executable, a little like --single?). Something that's reasonably
targeted, and dead simple to use.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-10 01:05         ` Bossart, Nathan <[email protected]>
  2 siblings, 0 replies; 32+ messages in thread

From: Bossart, Nathan @ 2021-12-10 01:05 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; John Naylor <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On 12/9/21, 12:33 PM, "Bossart, Nathan" <[email protected]> wrote:
> On 12/9/21, 11:34 AM, "John Naylor" <[email protected]> wrote:
>> Now that we have a concept of a fail-safe vacuum, maybe it would be
>> beneficial to skip a vacuum in single-user mode if the fail-safe
>> criteria were not met at the beginning of vacuuming a relation. This
>> is not without risk, of course, but it should be much faster than
>> today and once up and running the admin would have a chance to get a
>> handle on things. Thoughts?
>
> Would the --min-xid-age and --no-index-cleanup vacuumdb options help
> with this?

Sorry, I'm not sure what I was thinking.  Of coure you cannot use
vacuumdb in single-user mode.  But I think something like
--min-xid-age in VACUUM is what you are looking for.

Nathan



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-10 01:06         ` Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2 siblings, 1 reply; 32+ messages in thread

From: Bossart, Nathan @ 2021-12-10 01:06 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; John Naylor <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On 12/9/21, 4:36 PM, "Peter Geoghegan" <[email protected]> wrote:
> We could then apply this criteria in new code that implements this
> "big red button" (maybe this is a new option for the postgres
> executable, a little like --single?). Something that's reasonably
> targeted, and dead simple to use.

+1

Nathan



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
@ 2021-12-10 01:11           ` Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Bossart, Nathan @ 2021-12-10 01:11 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; John Naylor <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On 12/9/21, 5:06 PM, "Bossart, Nathan" <[email protected]> wrote:
> On 12/9/21, 4:36 PM, "Peter Geoghegan" <[email protected]> wrote:
>> We could then apply this criteria in new code that implements this
>> "big red button" (maybe this is a new option for the postgres
>> executable, a little like --single?). Something that's reasonably
>> targeted, and dead simple to use.
>
> +1

As Andres noted, such a feature might be useful during normal
operation, too.  Perhaps the vacuumdb --min-xid-age stuff should be
moved to a new VACUUM option.

Nathan



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
@ 2021-12-10 01:25             ` Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-10 01:25 UTC (permalink / raw)
  To: Bossart, Nathan <[email protected]>; +Cc: John Naylor <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 5:12 PM Bossart, Nathan <[email protected]> wrote:
> As Andres noted, such a feature might be useful during normal
> operation, too.  Perhaps the vacuumdb --min-xid-age stuff should be
> moved to a new VACUUM option.

I was thinking of something like pg_import_system_collations() for
this: a function that's built-in, and can be called in single user
mode, that nevertheless doesn't make any assumptions about how it may
be called. Nothing stops a superuser from calling
pg_import_system_collations() themselves, outside of initdb. That
isn't particularly common, but it works in the way you'd expect it to
work. It's easy to test.

I imagine that this new function (to handle maintenance tasks in the
event of a wraparound emergency) would output information about its
progress. For example, it would make an up-front decision about which
tables needed to be vacuumed in order for the current DB's
datfrozenxid to be sufficiently new, before it started anything (with
handling for edge-cases with many tables, perhaps). It might also show
the size of each table, and show another line for each table that has
been processed so far, as a rudimentary progress indicator.

We could still have a separate option for the postgres executable,
just to invoke single-user mode and call this function. It would
mostly just be window dressing, of course, but that still seems
useful.

--
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-10 04:41               ` Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Bossart, Nathan @ 2021-12-10 04:41 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: John Naylor <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On 12/9/21, 5:27 PM, "Peter Geoghegan" <[email protected]> wrote:
> I imagine that this new function (to handle maintenance tasks in the
> event of a wraparound emergency) would output information about its
> progress. For example, it would make an up-front decision about which
> tables needed to be vacuumed in order for the current DB's
> datfrozenxid to be sufficiently new, before it started anything (with
> handling for edge-cases with many tables, perhaps). It might also show
> the size of each table, and show another line for each table that has
> been processed so far, as a rudimentary progress indicator.

I like the idea of having a built-in function that does the bare
minimum to resolve wraparound emergencies, and I think providing some
sort of simple progress indicator (even if rudimentary) would be very
useful.  I imagine the decision logic could be pretty simple.  If
we're only interested in getting the cluster out of a wraparound
emergency, we can probably just look for all tables with an age over
~2B.

Nathan



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
@ 2021-12-21 01:17                 ` Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-21 01:17 UTC (permalink / raw)
  To: Bossart, Nathan <[email protected]>; +Cc: John Naylor <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 8:41 PM Bossart, Nathan <[email protected]> wrote:
> I like the idea of having a built-in function that does the bare
> minimum to resolve wraparound emergencies, and I think providing some
> sort of simple progress indicator (even if rudimentary) would be very
> useful.

If John doesn't have time to work on this during the Postgres 15
cycle, and if nobody else picks it up, then we should at least do the
bare minimum here: force the use of the failsafe in single user mode
(regardless of the age of relfrozenxid/relminmxid, which in general
might not be that old in tables where VACUUM might need to do a lot of
work). Attached quick and dirty patch shows what this would take. If
nothing else, it seems natural to define running any VACUUM in single
user mode as an emergency.

This is really the least we could do -- it's much better than nothing,
but still really lazy^M^M^M^M conservative. I haven't revised the
assumption that the user should do a top-level "VACUUM" in databases
that can no longer allocate XIDs due to wraparound, despite the fact
that we could do far better with moderate effort. Although it might
make sense to commit something like the attached as part of a more
worked out solution (assuming it didn't fully remove single user mode
from the equation, which would be better still).

-- 
Peter Geoghegan


Attachments:

  [application/octet-stream] v1-0001-Always-trigger-failsafe-in-single-user-mode.patch (2.3K, ../../CAH2-Wz=Eb-jrhFK6=_WNT6V1JAK-qFOTu_4-yOpAhkEES4VeNA@mail.gmail.com/2-v1-0001-Always-trigger-failsafe-in-single-user-mode.patch)
  download | inline diff:
From b7a3ded18d1303e64951439437bcd5925d5c6f44 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <[email protected]>
Date: Mon, 20 Dec 2021 16:58:40 -0800
Subject: [PATCH v1] Always trigger failsafe in single user mode.

---
 src/backend/access/heap/vacuumlazy.c | 12 ++++++++----
 src/backend/commands/vacuum.c        |  6 +++++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index db6becfed..2ba882951 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2605,6 +2605,14 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
 		vacrel->do_index_cleanup = false;
 		vacrel->do_rel_truncate = false;
 
+		/* Stop applying cost limits from this point on */
+		VacuumCostActive = false;
+		VacuumCostBalance = 0;
+
+		/* Single user mode uses failsafe as standard, so no WARNING needed */
+		if (!IsUnderPostmaster)
+			return true;
+
 		ereport(WARNING,
 				(errmsg("bypassing nonessential maintenance of table \"%s.%s.%s\" as a failsafe after %d index scans",
 						get_database_name(MyDatabaseId),
@@ -2615,10 +2623,6 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
 				 errhint("Consider increasing configuration parameter \"maintenance_work_mem\" or \"autovacuum_work_mem\".\n"
 						 "You might also need to consider other ways for VACUUM to keep up with the allocation of transaction IDs.")));
 
-		/* Stop applying cost limits from this point on */
-		VacuumCostActive = false;
-		VacuumCostBalance = 0;
-
 		return true;
 	}
 
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 5c4bc15b4..217bb6965 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1153,7 +1153,8 @@ vacuum_set_xid_limits(Relation rel,
 /*
  * vacuum_xid_failsafe_check() -- Used by VACUUM's wraparound failsafe
  * mechanism to determine if its table's relfrozenxid and relminmxid are now
- * dangerously far in the past.
+ * dangerously far in the past.  This is assumed to always be the case when
+ * the backend is run in single user mode.
  *
  * Input parameters are the target relation's relfrozenxid and relminmxid.
  *
@@ -1203,6 +1204,9 @@ vacuum_xid_failsafe_check(TransactionId relfrozenxid, MultiXactId relminmxid)
 		return true;
 	}
 
+	if (!IsUnderPostmaster)
+		return true;
+
 	return false;
 }
 
-- 
2.30.2



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-21 03:46                   ` Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Andres Freund @ 2021-12-21 03:46 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

Hi,

On 2021-12-20 17:17:26 -0800, Peter Geoghegan wrote:
> On Thu, Dec 9, 2021 at 8:41 PM Bossart, Nathan <[email protected]> wrote:
> > I like the idea of having a built-in function that does the bare
> > minimum to resolve wraparound emergencies, and I think providing some
> > sort of simple progress indicator (even if rudimentary) would be very
> > useful.
> 
> If John doesn't have time to work on this during the Postgres 15
> cycle, and if nobody else picks it up, then we should at least do the
> bare minimum here: force the use of the failsafe in single user mode
> (regardless of the age of relfrozenxid/relminmxid, which in general
> might not be that old in tables where VACUUM might need to do a lot of
> work). Attached quick and dirty patch shows what this would take. If
> nothing else, it seems natural to define running any VACUUM in single
> user mode as an emergency.

As I said before I think this is a bad idea. I'm fine with adding a vacuum
parameter forcing failsafe mode. And perhaps a hint to suggest it in single
user mode. But forcing it is a bad idea - single user isn't just used for
emergencies (c.f. initdb, which this patch would regress) and not every
emergency making single user mode useful is related to wraparound.

Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
@ 2021-12-21 04:40                     ` Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Masahiko Sawada @ 2021-12-21 04:40 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Bossart, Nathan <[email protected]>; John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 12:46 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2021-12-20 17:17:26 -0800, Peter Geoghegan wrote:
> > On Thu, Dec 9, 2021 at 8:41 PM Bossart, Nathan <[email protected]> wrote:
> > > I like the idea of having a built-in function that does the bare
> > > minimum to resolve wraparound emergencies, and I think providing some
> > > sort of simple progress indicator (even if rudimentary) would be very
> > > useful.
> >
> > If John doesn't have time to work on this during the Postgres 15
> > cycle, and if nobody else picks it up, then we should at least do the
> > bare minimum here: force the use of the failsafe in single user mode
> > (regardless of the age of relfrozenxid/relminmxid, which in general
> > might not be that old in tables where VACUUM might need to do a lot of
> > work). Attached quick and dirty patch shows what this would take. If
> > nothing else, it seems natural to define running any VACUUM in single
> > user mode as an emergency.
>
> As I said before I think this is a bad idea. I'm fine with adding a vacuum
> parameter forcing failsafe mode. And perhaps a hint to suggest it in single
> user mode. But forcing it is a bad idea - single user isn't just used for
> emergencies (c.f. initdb, which this patch would regress) and not every
> emergency making single user mode useful is related to wraparound.

+1

BTW a vacuum automatically enters failsafe mode under the situation
where the user has to run a vacuum in the single-user mode, right?

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
@ 2021-12-21 04:52                       ` Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-21 04:52 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

On Mon, Dec 20, 2021 at 8:40 PM Masahiko Sawada <[email protected]> wrote:
> BTW a vacuum automatically enters failsafe mode under the situation
> where the user has to run a vacuum in the single-user mode, right?

Only for the table that had the problem. Maybe there are no other
tables that a database level "VACUUM" will need to spend much time on,
or maybe there are, and they will make it take much much longer (it
all depends).

The goal of the patch is to make sure that when we're in single user
mode, we'll consistently trigger the failsafe, for every VACUUM
against every table -- not just the table (or tables) whose
relfrozenxid is very old. That's still naive, but much less naive than
simply telling users to VACUUM the whole database in single user mode
while vacuuming indexes, etc.


--
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-21 07:56                         ` Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: Masahiko Sawada @ 2021-12-21 07:56 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 1:53 PM Peter Geoghegan <[email protected]> wrote:
>
> On Mon, Dec 20, 2021 at 8:40 PM Masahiko Sawada <[email protected]> wrote:
> > BTW a vacuum automatically enters failsafe mode under the situation
> > where the user has to run a vacuum in the single-user mode, right?
>
> Only for the table that had the problem. Maybe there are no other
> tables that a database level "VACUUM" will need to spend much time on,
> or maybe there are, and they will make it take much much longer (it
> all depends).
>
> The goal of the patch is to make sure that when we're in single user
> mode, we'll consistently trigger the failsafe, for every VACUUM
> against every table -- not just the table (or tables) whose
> relfrozenxid is very old. That's still naive, but much less naive than
> simply telling users to VACUUM the whole database in single user mode
> while vacuuming indexes, etc.

I understand the patch, thank you for the explanation!

I remember Simon proposed a VACUUM command option[1], called
FAST_FREEZE, to turn off index cleanup and heap truncation. Now that
we have failsafe mechanism probably we can have a VACUUM command
option to turn on failsafe mode instead.

Regards,

[1] https://commitfest.postgresql.org/32/2908/

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
@ 2021-12-21 17:36                           ` John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: John Naylor @ 2021-12-21 17:36 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 3:56 AM Masahiko Sawada <[email protected]> wrote:
>
> On Tue, Dec 21, 2021 at 1:53 PM Peter Geoghegan <[email protected]> wrote:
> >
> > On Mon, Dec 20, 2021 at 8:40 PM Masahiko Sawada <[email protected]> wrote:
> > > BTW a vacuum automatically enters failsafe mode under the situation
> > > where the user has to run a vacuum in the single-user mode, right?
> >
> > Only for the table that had the problem. Maybe there are no other
> > tables that a database level "VACUUM" will need to spend much time on,
> > or maybe there are, and they will make it take much much longer (it
> > all depends).
> >
> > The goal of the patch is to make sure that when we're in single user
> > mode, we'll consistently trigger the failsafe, for every VACUUM
> > against every table -- not just the table (or tables) whose
> > relfrozenxid is very old. That's still naive, but much less naive than
> > simply telling users to VACUUM the whole database in single user mode
> > while vacuuming indexes, etc.
>
> I understand the patch, thank you for the explanation!
>
> I remember Simon proposed a VACUUM command option[1], called
> FAST_FREEZE, to turn off index cleanup and heap truncation. Now that
> we have failsafe mechanism probably we can have a VACUUM command
> option to turn on failsafe mode instead.

I've been thinking a bit more about this, and I see two desirable
goals of anti-wraparound vacuum in single-user mode:

1. Get out of single-user mode as quickly as possible.

2. Minimize the catch-up work we have to do once we're out.

Currently, a naive vacuum does as much work as possible and leaves a
bunch of WAL streaming and archiving work for later, so that much is
easy to improve upon and we don't have to be terribly sophisticated.
Keeping in mind Andres' point that we don't want to force possibly
unwanted behavior just because we're in single-user mode, it makes
sense to have some kind of option that has the above two goals.
Instead of a boolean, it seems like the new option should specify some
age below which VACUUM will skip the table entirely, and above which
will enter fail-safe mode. As mentioned earlier, the shutdown hint
could spell out the exact command. With this design, it would specify
the fail-safe default, or something else, to use with the option. That
seems doable for v15 -- any thoughts on that approach?

In standard operation, the above goals could be restated as "advance
xmin as quickly as possible" and "generate as little future
'work/debt' as possible, whether dirty pages or WAL". There are some
more sophisticated things we can do in this regard, but something like
the above could also be useful in normal operation. In fact, that
"normal" could be just after we restarted after doing the bare-minimum
in single-user mode, and want to continue freezing and keep things
under control.

--
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-21 21:35                             ` John Naylor <[email protected]>
  2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 32+ messages in thread

From: John Naylor @ 2021-12-21 21:35 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

I wrote:

> Instead of a boolean, it seems like the new option should specify some
> age below which VACUUM will skip the table entirely, and above which
> will enter fail-safe mode. As mentioned earlier, the shutdown hint
> could spell out the exact command. With this design, it would specify
> the fail-safe default, or something else, to use with the option.

On second thought, we don't really need another number here. We could
simply go by the existing failsafe parameter, and if the admin wants a
different value, it's already possible to specify
vacuum_failsafe_age/vacuum_multixact_failsafe_age in a session,
including in single-user mode. Perhaps a new boolean called
FAILSAFE_ONLY. If no table is specified, then when generating the list
of tables, include only those with relfrozenxid/relminmxid greater
than their failsafe thresholds.

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-21 21:56                               ` Peter Geoghegan <[email protected]>
  2021-12-21 23:02                                 ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-22 02:39                                 ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  0 siblings, 2 replies; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-21 21:56 UTC (permalink / raw)
  To: John Naylor <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 1:31 PM John Naylor
<[email protected]> wrote:
> On second thought, we don't really need another number here. We could
> simply go by the existing failsafe parameter, and if the admin wants a
> different value, it's already possible to specify
> vacuum_failsafe_age/vacuum_multixact_failsafe_age in a session,
> including in single-user mode. Perhaps a new boolean called
> FAILSAFE_ONLY. If no table is specified, then when generating the list
> of tables, include only those with relfrozenxid/relminmxid greater
> than their failsafe thresholds.

That's equivalent to the quick and dirty patch I wrote (assuming that
the user actually uses this new FAILSAFE_ONLY thing).

But if we're going to add a new option to the VACUUM command (or
something of similar scope), then we might as well add a new behavior
that is reasonably exact -- something that (say) only *starts* a
VACUUM for those tables whose relfrozenxid age currently exceeds half
the autovacuum_freeze_max_age for the table (usually taken from the
GUC, sometimes taken from the reloption), which also forces the
failsafe. And with similar handling for
relminmxid/autovacuum_multixact_freeze_max_age.

In other words, while triggering the failsafe is important, simply *not
starting* VACUUM for relations where there is really no need for it is
at least as important. We shouldn't even think about pruning or
freezing with these tables. (ISTM that the only thing that might be a
bit controversial about any of this is my definition of "safe", which
seems like about the right trade-off to me.)

This new command/facility should probably not be a new flag to the
VACUUM command, as such. Rather, I think that it should either be an
SQL-callable function, or a dedicated top-level command (that doesn't
accept any tables). The only reason to have this is for scenarios
where the user is already in a tough spot with wraparound failure,
like that client of yours. Nobody wants to force the failsafe for one
specific table. It's not general purpose, at all, and shouldn't claim
to be.


--
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-21 23:02                                 ` John Naylor <[email protected]>
  1 sibling, 0 replies; 32+ messages in thread

From: John Naylor @ 2021-12-21 23:02 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 5:56 PM Peter Geoghegan <[email protected]> wrote:
>
> On Tue, Dec 21, 2021 at 1:31 PM John Naylor
> <[email protected]> wrote:
> > On second thought, we don't really need another number here. We could
> > simply go by the existing failsafe parameter, and if the admin wants a
> > different value, it's already possible to specify
> > vacuum_failsafe_age/vacuum_multixact_failsafe_age in a session,
> > including in single-user mode. Perhaps a new boolean called
> > FAILSAFE_ONLY. If no table is specified, then when generating the list
> > of tables, include only those with relfrozenxid/relminmxid greater
> > than their failsafe thresholds.
>
> That's equivalent to the quick and dirty patch I wrote (assuming that
> the user actually uses this new FAILSAFE_ONLY thing).

Equivalent but optional.

> But if we're going to add a new option to the VACUUM command (or
> something of similar scope), then we might as well add a new behavior
> that is reasonably exact -- something that (say) only *starts* a
> VACUUM for those tables whose relfrozenxid age currently exceeds half
> the autovacuum_freeze_max_age for the table (usually taken from the
> GUC, sometimes taken from the reloption), which also forces the
> failsafe. And with similar handling for
> relminmxid/autovacuum_multixact_freeze_max_age.
>
> In other words, while triggering the failsafe is important, simply *not
> starting* VACUUM for relations where there is really no need for it is
> at least as important. We shouldn't even think about pruning or
> freezing with these tables.

Right, not starting where not necessary is crucial for getting out of
single-user mode as quickly as possible. We're in agreement there.

> (ISTM that the only thing that might be a
> bit controversial about any of this is my definition of "safe", which
> seems like about the right trade-off to me.)

It seems reasonable to want to start back up and not immediately have
anti-wraparound vacuums kick in. On the other hand, it's not good to
do work while unable to monitor progress, and while more WAL is piling
up. I'm not sure where the right trade off is.

> This new command/facility should probably not be a new flag to the
> VACUUM command, as such. Rather, I think that it should either be an
> SQL-callable function, or a dedicated top-level command (that doesn't
> accept any tables). The only reason to have this is for scenarios
> where the user is already in a tough spot with wraparound failure,
> like that client of yours. Nobody wants to force the failsafe for one
> specific table. It's not general purpose, at all, and shouldn't claim
> to be.

Makes sense, I'll have a think about what that would look like.

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-22 02:39                                 ` Masahiko Sawada <[email protected]>
  2021-12-22 15:07                                   ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-22 21:33                                   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  1 sibling, 2 replies; 32+ messages in thread

From: Masahiko Sawada @ 2021-12-22 02:39 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: John Naylor <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Wed, Dec 22, 2021 at 6:56 AM Peter Geoghegan <[email protected]> wrote:
>
> On Tue, Dec 21, 2021 at 1:31 PM John Naylor
> <[email protected]> wrote:
> > On second thought, we don't really need another number here. We could
> > simply go by the existing failsafe parameter, and if the admin wants a
> > different value, it's already possible to specify
> > vacuum_failsafe_age/vacuum_multixact_failsafe_age in a session,
> > including in single-user mode. Perhaps a new boolean called
> > FAILSAFE_ONLY. If no table is specified, then when generating the list
> > of tables, include only those with relfrozenxid/relminmxid greater
> > than their failsafe thresholds.
>
> That's equivalent to the quick and dirty patch I wrote (assuming that
> the user actually uses this new FAILSAFE_ONLY thing).
>
> But if we're going to add a new option to the VACUUM command (or
> something of similar scope), then we might as well add a new behavior
> that is reasonably exact -- something that (say) only *starts* a
> VACUUM for those tables whose relfrozenxid age currently exceeds half
> the autovacuum_freeze_max_age for the table (usually taken from the
> GUC, sometimes taken from the reloption), which also forces the
> failsafe. And with similar handling for
> relminmxid/autovacuum_multixact_freeze_max_age.
>
> In other words, while triggering the failsafe is important, simply *not
> starting* VACUUM for relations where there is really no need for it is
> at least as important. We shouldn't even think about pruning or
> freezing with these tables. (ISTM that the only thing that might be a
> bit controversial about any of this is my definition of "safe", which
> seems like about the right trade-off to me.)

+1

>
> This new command/facility should probably not be a new flag to the
> VACUUM command, as such. Rather, I think that it should either be an
> SQL-callable function, or a dedicated top-level command (that doesn't
> accept any tables). The only reason to have this is for scenarios
> where the user is already in a tough spot with wraparound failure,
> like that client of yours. Nobody wants to force the failsafe for one
> specific table. It's not general purpose, at all, and shouldn't claim
> to be.

Even not in the situation where the database has to run as the
single-user mode to freeze tuples, I think there would be some use
cases where users want to run vacuum (in failsafe mode) on tables with
relfrozenxid/relminmxid greater than their failsafe thresholds before
falling into that situation. I think it’s common that users are
somehow monitoring relfrozenxid/relminmxid and want to manually run
vacuum on them rather than relying on autovacuums. --min-xid-age
option and --min-mxid-age option of vacuumdb command would be good
examples. So I think this new command/facility might not necessarily
need to be specific to single-user mode.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-22 02:39                                 ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
@ 2021-12-22 15:07                                   ` John Naylor <[email protected]>
  1 sibling, 0 replies; 32+ messages in thread

From: John Naylor @ 2021-12-22 15:07 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 10:39 PM Masahiko Sawada <[email protected]> wrote:
>
> On Wed, Dec 22, 2021 at 6:56 AM Peter Geoghegan <[email protected]> wrote:
> >
> > This new command/facility should probably not be a new flag to the
> > VACUUM command, as such. Rather, I think that it should either be an
> > SQL-callable function, or a dedicated top-level command (that doesn't
> > accept any tables). The only reason to have this is for scenarios
> > where the user is already in a tough spot with wraparound failure,
> > like that client of yours. Nobody wants to force the failsafe for one
> > specific table. It's not general purpose, at all, and shouldn't claim
> > to be.
>
> Even not in the situation where the database has to run as the
> single-user mode to freeze tuples, I think there would be some use
> cases where users want to run vacuum (in failsafe mode) on tables with
> relfrozenxid/relminmxid greater than their failsafe thresholds before
> falling into that situation. I think it’s common that users are
> somehow monitoring relfrozenxid/relminmxid and want to manually run
> vacuum on them rather than relying on autovacuums. --min-xid-age
> option and --min-mxid-age option of vacuumdb command would be good
> examples. So I think this new command/facility might not necessarily
> need to be specific to single-user mode.

If we want to leave open the possibility to specify these parameters,
a SQL-callable function seems like the way to go. And even if we
don't, a function is fine.

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
  2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
  2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-22 02:39                                 ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
@ 2021-12-22 21:33                                   ` Peter Geoghegan <[email protected]>
  1 sibling, 0 replies; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-22 21:33 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: John Naylor <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Dec 21, 2021 at 6:39 PM Masahiko Sawada <[email protected]> wrote:
> Even not in the situation where the database has to run as the
> single-user mode to freeze tuples, I think there would be some use
> cases where users want to run vacuum (in failsafe mode) on tables with
> relfrozenxid/relminmxid greater than their failsafe thresholds before
> falling into that situation. I think it’s common that users are
> somehow monitoring relfrozenxid/relminmxid and want to manually run
> vacuum on them rather than relying on autovacuums. --min-xid-age
> option and --min-mxid-age option of vacuumdb command would be good
> examples. So I think this new command/facility might not necessarily
> need to be specific to single-user mode.

It wouldn't be specific to single-user mode, since that is not really
special. It's only special in a way that's quite artificial (it can
continue to allocate XIDs past the point where we usually deem it
unsafe).

So, I think we agree; this new emergency vacuuming feature shouldn't
be restricted to single-user mode in any way, and shouldn't care about
whether we're in single user mode or not when run. OTOH, it probably
will be presented as something that is typically run in single user
mode, in situations like the one John's customer found themselves in
-- disastrous, unpleasant situations. It's not just a good policy
(that makes testing easy). The same kind of problem can easily be
caught a little earlier, before the system actually becomes unable to
allocate new XIDs (when not in single-user mode) -- that's quite
likely, and almost as scary.

As I said before, ISTM that the important thing is to have something
dead simple -- something that is easy to use when woken at 4am, when
the DBA is tired and stressed. Something that makes generic choices,
that are not way too conservative, but also don't risk making the
problem worse instead of better.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
@ 2021-12-10 01:56         ` Andres Freund <[email protected]>
  2021-12-10 02:32           ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2022-03-31 09:51           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2022-06-27 19:36           ` Re: do only critical work during single-user vacuum? Justin Pryzby <[email protected]>
  2 siblings, 3 replies; 32+ messages in thread

From: Andres Freund @ 2021-12-10 01:56 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

Hi,

On 2021-12-09 16:34:53 -0800, Peter Geoghegan wrote:
> But even still, why not have some variant of single-user mode just for
> this task?

> Something that's easy to use when the DBA is rudely
> awakened at 4am -- something a little bit like a big red button that
> fixes the exact problem of XID exhaustion, in a reasonably targeted
> way? I don't think that this needs to involve the VACUUM command
> itself.

I think we should move *away* from single user mode, rather than the
opposite. It's a substantial code burden and it's hard to use.

I don't think single user mode is a good fit for this anyway - it's inherently
focussed on connecting to a single database. But wraparound issues often
involve more than one database (often just because of shared catalogs).


Also, requiring a restart will often exascerbate the problem - the cache will
be cold, there's no walwriter, etc, making the vacuum slower. Making vacuum
not consume an xid seems like a lot more promising - and quite doable. Then
there's no need to restart at all.

Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
@ 2021-12-10 02:32           ` Peter Geoghegan <[email protected]>
  2 siblings, 0 replies; 32+ messages in thread

From: Peter Geoghegan @ 2021-12-10 02:32 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Dec 9, 2021 at 5:56 PM Andres Freund <[email protected]> wrote:
> I think we should move *away* from single user mode, rather than the
> opposite. It's a substantial code burden and it's hard to use.

I wouldn't say that this is moving closer to single user mode.

> I don't think single user mode is a good fit for this anyway - it's inherently
> focussed on connecting to a single database. But wraparound issues often
> involve more than one database (often just because of shared catalogs).

I don't disagree with any of that. My suggestions were based on the
assumption that it might be unrealistic to expect somebody to spend a
huge amount of time on this, given that (in a certain sense) it's
never really supposed to be used. Even a very simple approach would be
a big improvement.

> Also, requiring a restart will often exascerbate the problem - the cache will
> be cold, there's no walwriter, etc, making the vacuum slower. Making vacuum
> not consume an xid seems like a lot more promising - and quite doable. Then
> there's no need to restart at all.

I didn't give too much consideration to what it would take to keep the
system partially online, without introducing excessive complexity.
Maybe it wouldn't be that hard to teach the system to stop allocating
XIDs, while still allowing autovacuum workers to continue to get the
system functioning again. With the av workers taking a particular
emphasis on doing whatever work is required for the system to be able
to allocate XIDs again -- but not too much more (not until things are
back to normal). Now the plan is starting to get ambitious relative to
how often it'll be seen by users, though.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
@ 2022-03-31 09:51           ` John Naylor <[email protected]>
  2 siblings, 0 replies; 32+ messages in thread

From: John Naylor @ 2022-03-31 09:51 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; Robert Haas <[email protected]>; PostgreSQL Hackers <[email protected]>

On Wed, Mar 16, 2022 at 4:48 AM Peter Geoghegan <[email protected]> wrote:
>
> On Wed, Feb 16, 2022 at 12:43 AM John Naylor
> <[email protected]> wrote:
> > I'll put some effort in finding any way that it might not be robust.
> > After that, changing the message and docs is trivial.
>
> It would be great to be able to totally drop the idea of using
> single-user mode before Postgres 15 feature freeze. How's that going?

Unfortunately, I was distracted from this work for a time, and just as
I had intended to focus on it during March, I was out sick for 2-3
weeks. I gather from subsequent discussion that a full solution goes
beyond just a new warning message and documentation. Either way I'm
not quite prepared to address this in time for v15.

> I suggest that we apply the following patch as part of that work. It
> adds one last final failsafe check at the point that VACUUM makes a
> final decision on rel truncation.

That is one thing that was in the back of my mind, and it seems
reasonable to me.

-- 
John Naylor
EDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
@ 2022-06-27 19:36           ` Justin Pryzby <[email protected]>
  2022-06-27 20:36             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2 siblings, 1 reply; 32+ messages in thread

From: Justin Pryzby @ 2022-06-27 19:36 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; John Naylor <[email protected]>; [email protected]; Hannu Krosing <[email protected]>

On Thu, Feb 03, 2022 at 01:05:50PM -0500, Robert Haas wrote:
> On Thu, Dec 9, 2021 at 8:56 PM Andres Freund <[email protected]> wrote:
> > I think we should move *away* from single user mode, rather than the
> > opposite. It's a substantial code burden and it's hard to use.
> 
> Yes. This thread seems to be largely devoted to the topic of making
> single-user vacuum work better, but I don't see anyone asking the
> question "why do we have a message that tells people to vacuum in
> single user mode in the first place?". It's basically bad advice,

> The correct thing to do would be to remove
> the hint as bad advice that we never should have offered in the first
> place. And so here. We should not try to make vacuum in single
> user-mode work better or differently, or at least that shouldn't be
> our primary objective. We should just stop telling people to do it. We
> should probably add messages and documentation *discouraging* the use
> of single user mode for recovering from wraparound trouble, exactly
> the opposite of what we do now. There's nothing we can do in
> single-user mode that we can't do equally well in multi-user mode. If
> people try to fix wraparound problems in multi-user mode, they still
> have read-only access to their database, they can use parallelism,
> they can use command line utilities like vacuumdb, and they can use
> psql which has line editing and allows remote access and is a way
> nicer user experience than running postgres --single. We need a really
> compelling reason to tell people to give up all those advantages, and
> there is no such reason. It makes just as much sense as telling people
> to deal with wraparound problems by angering a live anaconda.

By chance, I came across this prior thread which advocated the same thing in a
initially (rather than indirectly as in this year's thread).

https://www.postgresql.org/message-id/flat/CAMT0RQTmRj_Egtmre6fbiMA9E2hM3BsLULiV8W00stwa3URvzA%40mai...
|We should stop telling users to "vacuum that database in single-user mode"





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
  2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
  2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
  2022-06-27 19:36           ` Re: do only critical work during single-user vacuum? Justin Pryzby <[email protected]>
@ 2022-06-27 20:36             ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 32+ messages in thread

From: Peter Geoghegan @ 2022-06-27 20:36 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; John Naylor <[email protected]>; PostgreSQL Hackers <[email protected]>; Hannu Krosing <[email protected]>

On Mon, Jun 27, 2022 at 12:36 PM Justin Pryzby <[email protected]> wrote:
> By chance, I came across this prior thread which advocated the same thing in a
> initially (rather than indirectly as in this year's thread).

Revisiting this topic reminded me that PostgreSQL 14 (the first
version that had the wraparound failsafe mechanism controlled by
vacuum_failsafe_age) has been a stable release for 9 months now. As of
today I am still not aware of even one user that ran into the failsafe
mechanism in production. It might well have happened by now, of
course, but I am not aware of any specific case. Perhaps this will
change soon enough -- maybe somebody else will read this and enlighten
me.

To me the fact that the failsafe seems to seldom kick-in in practice
suggests something about workload characteristics in general: that it
isn't all that common for users to try to get away with putting off
freezing until a table attains an age that is significantly above 1
billion XIDs.

When people talk about things like 64-bit XIDs, I tend to wonder: if 2
billion XIDs wasn't enough, why should 4 billion or 8 billion be
enough? *Maybe* the system can do better by getting even further into
debt than it can today, but you can't expect to avoid freezing
altogether (without significant work elsewhere). My general sense is
that freezing isn't a particularly good thing to try to do lazily --
even if we ignore the risk of an eventual wraparound failure.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: do only critical work during single-user vacuum?
  2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
@ 2021-12-09 22:08 ` Andres Freund <[email protected]>
  2 siblings, 0 replies; 32+ messages in thread

From: Andres Freund @ 2021-12-09 22:08 UTC (permalink / raw)
  To: John Naylor <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

Hi,

On 2021-12-09 15:28:18 -0400, John Naylor wrote:
> When a user must shut down and restart in single-user mode to run
> vacuum on an entire database, that does a lot of work that's
> unnecessary for getting the system online again, even without
> index_cleanup. We had a recent case where a single-user vacuum took
> around 3 days to complete.
> 
> Now that we have a concept of a fail-safe vacuum, maybe it would be
> beneficial to skip a vacuum in single-user mode if the fail-safe
> criteria were not met at the beginning of vacuuming a relation. This
> is not without risk, of course, but it should be much faster than
> today and once up and running the admin would have a chance to get a
> handle on things. Thoughts?

What if the user tried to reclaim space by vacuuming (via truncation)? Or is
working around some corruption or such?  I think this is too much magic.

That said, having a VACUUM "selector" that selects the oldest tables could be
quite useful. And address this usecase both for single-user and normal
operation.

Another thing that might be worth doing is to update relfrozenxid earlier. We
definitely should update it before doing truncation (that can be quite
expensive). But we probably should do it even before the final
lazy_cleanup_all_indexes() pass - often that'll be the only pass, and there's
really no reason to delay relfrozenxid advancement till after that.

Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* [PATCH] do only critical work during single-user vacuum?
@ 2022-01-21 22:41 John Naylor <[email protected]>
  0 siblings, 0 replies; 32+ messages in thread

From: John Naylor @ 2022-01-21 22:41 UTC (permalink / raw)

Jan 21 John Naylor
---
 src/backend/commands/vacuum.c | 76 +++++++++++++++++++++++++++++++++--
 src/include/commands/vacuum.h |  1 +
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index d1dadc54e47..c7bc97d3f76 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -52,6 +52,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/acl.h"
+#include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
@@ -114,6 +115,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		full = false;
 	bool		disable_page_skipping = false;
 	bool		process_toast = true;
+	bool		wraparound = false;
 	ListCell   *lc;
 
 	/* index_cleanup and truncate values unspecified for now */
@@ -200,6 +202,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 					params.nworkers = nworkers;
 			}
 		}
+		else if (strcmp(opt->defname, "wraparound") == 0)
+			wraparound = defGetBoolean(opt);
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -246,17 +250,51 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		}
 	}
 
+	if (wraparound)
+	{
+		/* exclude incompatible options */
+		foreach(lc, vacstmt->options)
+		{
+			DefElem    *opt = (DefElem *) lfirst(lc);
+
+			// WIP is there a better way?
+			if (strcmp(opt->defname, "wraparound") != 0 &&
+				strcmp(opt->defname, "verbose") != 0 &&
+				defGetBoolean(opt))
+
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+								errmsg("option \"%s\" is incompatible with WRAPAROUND", opt->defname),
+								parser_errposition(pstate, opt->location)));
+		}
+
+		/* skip unnecessary work, as in failsafe mode */
+		params.index_cleanup = VACOPTVALUE_DISABLED;
+		params.truncate = VACOPTVALUE_DISABLED;
+	}
+
 	/*
-	 * All freeze ages are zero if the FREEZE option is given; otherwise pass
-	 * them as -1 which means to use the default values.
+	 * Set freeze ages to zero where appropriate; otherwise pass
+	 * them as -1 which means to use the configured values.
 	 */
 	if (params.options & VACOPT_FREEZE)
 	{
+		/* All freeze ages are zero if the FREEZE option is given */
 		params.freeze_min_age = 0;
 		params.freeze_table_age = 0;
 		params.multixact_freeze_min_age = 0;
 		params.multixact_freeze_table_age = 0;
 	}
+	else if (params.options & VACOPT_MINIMAL)
+	{
+		/* it's unlikely any selected table will not be eligible for aggressive vacuum, but make sure */
+		params.freeze_table_age = 0;
+		params.multixact_freeze_table_age = 0;
+
+		// WIP: It might be worth trying to do less work here, or at least hard-coding the default values
+		params.freeze_min_age = -1;
+		params.multixact_freeze_min_age = -1;
+	}
 	else
 	{
 		params.freeze_min_age = -1;
@@ -894,6 +932,8 @@ get_all_vacuum_rels(int options)
 	Relation	pgclass;
 	TableScanDesc scan;
 	HeapTuple	tuple;
+	int32 		table_xid_age,
+				table_mxid_age;
 
 	pgclass = table_open(RelationRelationId, AccessShareLock);
 
@@ -909,12 +949,42 @@ get_all_vacuum_rels(int options)
 		if (!vacuum_is_relation_owner(relid, classForm, options))
 			continue;
 
+		if (options & VACOPT_MINIMAL)
+		{
+			/*
+			* Only consider relations able to hold unfrozen XIDs (anything else
+			* should have InvalidTransactionId in relfrozenxid anyway).
+			*/
+			if (classForm->relkind != RELKIND_RELATION &&
+				classForm->relkind != RELKIND_MATVIEW &&
+				classForm->relkind != RELKIND_TOASTVALUE)
+			{
+				Assert(!TransactionIdIsValid(classForm->relfrozenxid));
+				Assert(!MultiXactIdIsValid(classForm->relminmxid));
+				continue;
+			}
+
+			table_xid_age = DirectFunctionCall1(xid_age, classForm->relfrozenxid);
+			table_mxid_age = DirectFunctionCall1(mxid_age, classForm->relminmxid);
+
+			/* Hard-code 1 billion for the thresholds to avoid making assumptions
+			 * about the configuration. This leaves some headroom for when the user
+			 * returns to normal mode while also minimizing work.
+			 * WIP: consider passing these constants via the params struct
+			 */
+			// FIXME to speed up testing
+			// if ((table_xid_age < 1000 * 1000 * 1000) &&
+			// 	(table_mxid_age < 1000 * 1000 * 1000))
+			if ((table_xid_age < 1000 * 1000) &&
+				(table_mxid_age < 1000 * 1000))
+				continue;
+		}
 		/*
 		 * We include partitioned tables here; depending on which operation is
 		 * to be performed, caller will decide whether to process or ignore
 		 * them.
 		 */
-		if (classForm->relkind != RELKIND_RELATION &&
+		else if (classForm->relkind != RELKIND_RELATION &&
 			classForm->relkind != RELKIND_MATVIEW &&
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 5d0bdfa4279..3d8b8fbbb1a 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -188,6 +188,7 @@ typedef struct VacAttrStats
 #define VACOPT_SKIP_LOCKED 0x20 /* skip if cannot get lock */
 #define VACOPT_PROCESS_TOAST 0x40	/* process the TOAST table, if any */
 #define VACOPT_DISABLE_PAGE_SKIPPING 0x80	/* don't skip any pages */
+#define VACOPT_MINIMAL 0x100	/* do minimal freezing work to prevent or get out of shutdown */
 
 /*
  * Values used by index_cleanup and truncate params.
-- 
2.17.1


--CtqPPqYpYc7tL2y7--





^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* [PATCH 1/3] do only critical work during single-user vacuum?
@ 2022-02-01 21:50 John Naylor <[email protected]>
  0 siblings, 0 replies; 32+ messages in thread

From: John Naylor @ 2022-02-01 21:50 UTC (permalink / raw)

Feb 01 John Naylor
---
 doc/src/sgml/maintenance.sgml       |  12 ++--
 doc/src/sgml/ref/vacuum.sgml        |  22 ++++++
 src/backend/access/transam/varsup.c |   4 +-
 src/backend/commands/vacuum.c       | 107 +++++++++++++++++++++++++---
 src/include/commands/vacuum.h       |   5 ++
 5 files changed, 134 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml
index 36f975b1e5b..5c360499504 100644
--- a/doc/src/sgml/maintenance.sgml
+++ b/doc/src/sgml/maintenance.sgml
@@ -629,17 +629,19 @@ HINT:  To avoid a database shutdown, execute a database-wide VACUUM in that data
 
 <programlisting>
 ERROR:  database is not accepting commands to avoid wraparound data loss in database "mydb"
-HINT:  Stop the postmaster and vacuum that database in single-user mode.
+HINT:  Stop the postmaster and run "VACUUM (EMERGENCY)" in that database in single-user mode.
 </programlisting>
 
     The three-million-transaction safety margin exists to let the
     administrator recover without data loss, by manually executing the
-    required <command>VACUUM</command> commands.  However, since the system will not
+    required <command>VACUUM</command> command.  However, since the system will not
     execute commands once it has gone into the safety shutdown mode,
     the only way to do this is to stop the server and start the server in single-user
-    mode to execute <command>VACUUM</command>.  The shutdown mode is not enforced
-    in single-user mode.  See the <xref linkend="app-postgres"/> reference
-    page for details about using single-user mode.
+    mode to execute <command>VACUUM (EMERGENCY)</command>. The <literal>EMERGENCY</literal> option
+    is recommended, since it enables the vacuum to complete as quickly as possible
+    while still leaving a safety margin for when the system comes back online again.
+    The shutdown mode is not enforced in single-user mode.
+    See the <xref linkend="app-postgres"/> reference page for details about using single-user mode.
    </para>
 
    <sect3 id="vacuum-for-multixact-wraparound">
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 3df32b58ee6..2dab01ff376 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -295,6 +295,28 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>EMERGENCY</literal></term>
+    <listitem>
+     <para>
+      Special vacuum mode intended for when a forced shutdown has happened to avoid transaction ID wraparound.
+      It performs a database-wide vacuum on tables, toast tables, and materialized views whose
+      xid age or mxid age is older than 1 billion. To complete as quickly as possible, an emergency
+      vacuum will skip truncation and index cleanup, and will skip toast tables whose age has not
+      exceeded the cutoff.
+     </para>
+
+     <para>
+      While this option could be used while the postmaster is running, it is expected that the wraparound
+      failsafe mechanism will automatically work in the same way to prevent imminent shutdown.
+      When <literal>EMERGENCY</literal> is specified no tables may be listed, since it is designed to
+      select candidate relations from the entire database.
+      The only other option that may be combined with <literal>VERBOSE</literal>, although in single-user mode no client messages are
+      output.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c
index 748120a0125..ee9d33dba37 100644
--- a/src/backend/access/transam/varsup.c
+++ b/src/backend/access/transam/varsup.c
@@ -128,14 +128,14 @@ GetNewTransactionId(bool isSubXact)
 						(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						 errmsg("database is not accepting commands to avoid wraparound data loss in database \"%s\"",
 								oldest_datname),
-						 errhint("Stop the postmaster and vacuum that database in single-user mode.\n"
+						 errhint("Stop the postmaster and run \"VACUUM (EMERGENCY)\" in that database in single-user mode.\n"
 								 "You might also need to commit or roll back old prepared transactions, or drop stale replication slots.")));
 			else
 				ereport(ERROR,
 						(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						 errmsg("database is not accepting commands to avoid wraparound data loss in database with OID %u",
 								oldest_datoid),
-						 errhint("Stop the postmaster and vacuum that database in single-user mode.\n"
+						 errhint("Stop the postmaster and run \"VACUUM (EMERGENCY)\" in that database in single-user mode.\n"
 								 "You might also need to commit or roll back old prepared transactions, or drop stale replication slots.")));
 		}
 		else if (TransactionIdFollowsOrEquals(xid, xidWarnLimit))
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index d1dadc54e47..e36159e827f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -52,6 +52,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/acl.h"
+#include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
@@ -86,7 +87,7 @@ int			VacuumCostBalanceLocal = 0;
 
 /* non-export function prototypes */
 static List *expand_vacuum_rel(VacuumRelation *vrel, int options);
-static List *get_all_vacuum_rels(int options);
+static List *get_all_vacuum_rels(VacuumParams *params);
 static void vac_truncate_clog(TransactionId frozenXID,
 							  MultiXactId minMulti,
 							  TransactionId lastSaneFrozenXid,
@@ -114,6 +115,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		full = false;
 	bool		disable_page_skipping = false;
 	bool		process_toast = true;
+	bool		emergency = false;
 	ListCell   *lc;
 
 	/* index_cleanup and truncate values unspecified for now */
@@ -123,6 +125,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	/* By default parallel vacuum is enabled */
 	params.nworkers = 0;
 
+	/* By default don't skip any tables */
+	params.min_xid_age = 0;
+	params.min_mxid_age = 0;
+
 	/* Parse options list */
 	foreach(lc, vacstmt->options)
 	{
@@ -200,6 +206,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 					params.nworkers = nworkers;
 			}
 		}
+		else if (strcmp(opt->defname, "emergency") == 0)
+			emergency = defGetBoolean(opt);
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -216,7 +224,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		(freeze ? VACOPT_FREEZE : 0) |
 		(full ? VACOPT_FULL : 0) |
 		(disable_page_skipping ? VACOPT_DISABLE_PAGE_SKIPPING : 0) |
-		(process_toast ? VACOPT_PROCESS_TOAST : 0);
+		(process_toast ? VACOPT_PROCESS_TOAST : 0) |
+		(emergency ? VACOPT_EMERGENCY : 0);
 
 	/* sanity checks on options */
 	Assert(params.options & (VACOPT_VACUUM | VACOPT_ANALYZE));
@@ -246,17 +255,70 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		}
 	}
 
+	if (emergency)
+	{
+		/* exclude incompatible options */
+		foreach(lc, vacstmt->options)
+		{
+			DefElem    *opt = (DefElem *) lfirst(lc);
+
+			if (strcmp(opt->defname, "emergency") != 0 &&
+				strcmp(opt->defname, "verbose") != 0 &&
+				defGetBoolean(opt))
+
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+								errmsg("option \"%s\" is incompatible with EMERGENCY", opt->defname),
+								parser_errposition(pstate, opt->location)));
+		}
+
+		/* prevent specifying a list of tables, to keep it simple */
+		if (vacstmt->rels != NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("a relation list is not supported with the EMERGENCY option")));
+
+		/* skip unnecessary work, similar to failsafe mode */
+
+		params.index_cleanup = VACOPTVALUE_DISABLED;
+		params.truncate = VACOPTVALUE_DISABLED;
+
+		/* Hard-code 1 billion for the thresholds to avoid making assumptions
+		* about the configuration. This leaves some headroom for when the user
+		* returns to normal mode while also minimizing work.
+		* WIP: consider passing these constants via the params struct
+		*/
+		// params.min_xid_age = 1000 * 1000 * 1000;
+		// params.min_mxid_age = 1000 * 1000 * 1000;
+		// FIXME to speed up testing
+		params.min_xid_age = 1000 * 1000;
+		params.min_mxid_age = 1000 * 1000;
+	}
+
 	/*
-	 * All freeze ages are zero if the FREEZE option is given; otherwise pass
-	 * them as -1 which means to use the default values.
+	 * Set freeze ages to zero where appropriate; otherwise pass
+	 * them as -1 which means to use the configured values.
 	 */
 	if (params.options & VACOPT_FREEZE)
 	{
+		/* All freeze ages are zero if the FREEZE option is given */
 		params.freeze_min_age = 0;
 		params.freeze_table_age = 0;
 		params.multixact_freeze_min_age = 0;
 		params.multixact_freeze_table_age = 0;
 	}
+	else if (params.options & VACOPT_EMERGENCY)
+	{
+		/* It's highly likely any table selected will be eligible for aggressive vacuum, but make sure */
+		params.freeze_table_age = 0;
+		params.multixact_freeze_table_age = 0;
+
+		// WIP: It might be worth trying to do less work here, such as max age / 2 :
+		// params.freeze_min_age = 100 * 1000 * 1000;
+		// params.multixact_freeze_min_age = 200 * 1000 * 1000;
+		params.freeze_min_age = -1;
+		params.multixact_freeze_min_age = -1;
+	}
 	else
 	{
 		params.freeze_min_age = -1;
@@ -404,7 +466,7 @@ vacuum(List *relations, VacuumParams *params,
 		relations = newrels;
 	}
 	else
-		relations = get_all_vacuum_rels(params->options);
+		relations = get_all_vacuum_rels(params);
 
 	/*
 	 * Decide whether we need to start/commit our own transactions.
@@ -461,7 +523,10 @@ vacuum(List *relations, VacuumParams *params,
 		ListCell   *cur;
 
 		in_vacuum = true;
-		VacuumCostActive = (VacuumCostDelay > 0);
+		if (params->VACOPT_EMERGENCY)
+			VacuumCostActive = false;
+		else
+			VacuumCostActive = (VacuumCostDelay > 0);
 		VacuumCostBalance = 0;
 		VacuumPageHit = 0;
 		VacuumPageMiss = 0;
@@ -888,12 +953,14 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
  * the current database.  The list is built in vac_context.
  */
 static List *
-get_all_vacuum_rels(int options)
+get_all_vacuum_rels(VacuumParams *params)
 {
 	List	   *vacrels = NIL;
 	Relation	pgclass;
 	TableScanDesc scan;
 	HeapTuple	tuple;
+	int32 		table_xid_age,
+				table_mxid_age;
 
 	pgclass = table_open(RelationRelationId, AccessShareLock);
 
@@ -906,15 +973,37 @@ get_all_vacuum_rels(int options)
 		Oid			relid = classForm->oid;
 
 		/* check permissions of relation */
-		if (!vacuum_is_relation_owner(relid, classForm, options))
+		if (!vacuum_is_relation_owner(relid, classForm, params->options))
 			continue;
 
+		if (params->options & VACOPT_EMERGENCY)
+		{
+			/*
+			* Only consider relations able to hold unfrozen XIDs (anything else
+			* should have InvalidTransactionId in relfrozenxid anyway).
+			*/
+			if (classForm->relkind != RELKIND_RELATION &&
+				classForm->relkind != RELKIND_MATVIEW &&
+				classForm->relkind != RELKIND_TOASTVALUE)
+			{
+				Assert(!TransactionIdIsValid(classForm->relfrozenxid));
+				Assert(!MultiXactIdIsValid(classForm->relminmxid));
+				continue;
+			}
+
+			table_xid_age = DirectFunctionCall1(xid_age, classForm->relfrozenxid);
+			table_mxid_age = DirectFunctionCall1(mxid_age, classForm->relminmxid);
+
+			if ((table_xid_age < params->min_xid_age) &&
+				(table_mxid_age < params->min_mxid_age))
+				continue;
+		}
 		/*
 		 * We include partitioned tables here; depending on which operation is
 		 * to be performed, caller will decide whether to process or ignore
 		 * them.
 		 */
-		if (classForm->relkind != RELKIND_RELATION &&
+		else if (classForm->relkind != RELKIND_RELATION &&
 			classForm->relkind != RELKIND_MATVIEW &&
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 5d0bdfa4279..8aab6641628 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -188,6 +188,7 @@ typedef struct VacAttrStats
 #define VACOPT_SKIP_LOCKED 0x20 /* skip if cannot get lock */
 #define VACOPT_PROCESS_TOAST 0x40	/* process the TOAST table, if any */
 #define VACOPT_DISABLE_PAGE_SKIPPING 0x80	/* don't skip any pages */
+#define VACOPT_EMERGENCY 0x100	/* do minimal freezing work to prevent or get out of shutdown */
 
 /*
  * Values used by index_cleanup and truncate params.
@@ -233,6 +234,10 @@ typedef struct VacuumParams
 	 * disabled.
 	 */
 	int			nworkers;
+
+	/* cutoff ages for selecting tables to vacuum, 0 is the default */
+	int 		min_xid_age;
+	int 		min_mxid_age;
 } VacuumParams;
 
 /*
-- 
2.17.1


--+mVf78ABtUsDvBix
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0002-fix-compile-error.patch"



^ permalink  raw  reply  [nested|flat] 32+ messages in thread


end of thread, other threads:[~2022-06-27 20:36 UTC | newest]

Thread overview: 32+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-01-20 08:08 [PATCH v18 14/18] tableam: Sample Scan Support. Andres Freund <[email protected]>
2021-12-09 19:28 do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-09 20:32 ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
2021-12-09 21:04 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-09 21:12   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-09 23:53     ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-10 00:34       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-10 01:05         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
2021-12-10 01:06         ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
2021-12-10 01:11           ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
2021-12-10 01:25             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-10 04:41               ` Re: do only critical work during single-user vacuum? Bossart, Nathan <[email protected]>
2021-12-21 01:17                 ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-21 03:46                   ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
2021-12-21 04:40                     ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
2021-12-21 04:52                       ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-21 07:56                         ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
2021-12-21 17:36                           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-21 21:35                             ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-21 21:56                               ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-21 23:02                                 ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-22 02:39                                 ` Re: do only critical work during single-user vacuum? Masahiko Sawada <[email protected]>
2021-12-22 15:07                                   ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2021-12-22 21:33                                   ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-10 01:56         ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
2021-12-10 02:32           ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2022-03-31 09:51           ` Re: do only critical work during single-user vacuum? John Naylor <[email protected]>
2022-06-27 19:36           ` Re: do only critical work during single-user vacuum? Justin Pryzby <[email protected]>
2022-06-27 20:36             ` Re: do only critical work during single-user vacuum? Peter Geoghegan <[email protected]>
2021-12-09 22:08 ` Re: do only critical work during single-user vacuum? Andres Freund <[email protected]>
2022-01-21 22:41 [PATCH] do only critical work during single-user vacuum? John Naylor <[email protected]>
2022-02-01 21:50 [PATCH 1/3] do only critical work during single-user vacuum? John Naylor <[email protected]>

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