public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 4/4] Implement vacuum full/cluster (INDEX_TABLESPACE <tablespace>)
26+ messages / 8 participants
[nested] [flat]

* [PATCH 4/4] Implement vacuum full/cluster (INDEX_TABLESPACE <tablespace>)
@ 2020-04-01 01:35 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Justin Pryzby @ 2020-04-01 01:35 UTC (permalink / raw)

---
 doc/src/sgml/ref/cluster.sgml             | 12 ++++-
 doc/src/sgml/ref/vacuum.sgml              | 12 ++++-
 src/backend/commands/cluster.c            | 56 ++++++++++++++---------
 src/backend/commands/matview.c            |  3 +-
 src/backend/commands/tablecmds.c          |  2 +-
 src/backend/commands/vacuum.c             | 40 ++++++++--------
 src/backend/postmaster/autovacuum.c       |  1 +
 src/include/commands/cluster.h            |  4 +-
 src/include/commands/vacuum.h             |  5 +-
 src/test/regress/input/tablespace.source  | 13 ++++++
 src/test/regress/output/tablespace.source | 20 ++++++++
 11 files changed, 117 insertions(+), 51 deletions(-)

diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 6758ef97a6..9c0f5add59 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -29,6 +29,7 @@ CLUSTER [VERBOSE]
 
     VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
     TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
+    INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
 
 </synopsis>
  </refsynopsisdiv>
@@ -106,6 +107,15 @@ CLUSTER [VERBOSE]
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>INDEX_TABLESPACE</literal></term>
+    <listitem>
+     <para>
+      Specifies that the table's indexes will be rebuilt on a new tablespace.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>TABLESPACE</literal></term>
     <listitem>
@@ -128,7 +138,7 @@ CLUSTER [VERBOSE]
     <term><replaceable class="parameter">new_tablespace</replaceable></term>
     <listitem>
      <para>
-      The tablespace where the table will be rebuilt.
+      The tablespace where the table or its indexes will be rebuilt.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 5261a7c727..28cab119b6 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -36,6 +36,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ]
     PARALLEL <replaceable class="parameter">integer</replaceable>
     TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
+    INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -265,6 +266,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>INDEX_TABLESPACE</literal></term>
+    <listitem>
+     <para>
+      Specifies that the relation's indexes will be rebuilt on a new tablespace.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
@@ -314,7 +324,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     <term><replaceable class="parameter">new_tablespace</replaceable></term>
     <listitem>
      <para>
-      The tablespace where the relation will be rebuilt.
+      The tablespace where the relation or its indexes will be rebuilt.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 626321e3ac..7fc6b69e95 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -71,7 +71,7 @@ typedef struct
 
 
 static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose,
-							 Oid NewTableSpaceOid);
+							 Oid NewTableSpaceOid, Oid NewIdxTableSpaceOid);
 static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
 							bool verbose, bool *pSwapToastByContent,
 							TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
@@ -108,8 +108,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 	ListCell   *lc;
 	ClusterParams params = {0};
 	bool		verbose = false;
-	/* Name of tablespace to use for clustered relation. */
-	char		*tablespaceName = NULL;
+	/* Names of tablespaces to use for clustered relations. */
+	char		*tablespaceName = NULL,
+				*idxtablespaceName = NULL;
 
 	/* Parse option list */
 	foreach(lc, stmt->params)
@@ -120,6 +121,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 			verbose = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "tablespace") == 0)
 			tablespaceName = defGetString(opt);
+		else if (strcmp(opt->defname, "index_tablespace") == 0)
+			idxtablespaceName = defGetString(opt);
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -130,18 +133,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 
 	params.options = (verbose ? CLUOPT_VERBOSE : 0);
 
-	/* Select tablespace Oid to use. */
-	if (tablespaceName)
-	{
-		params.tablespaceOid = get_tablespace_oid(tablespaceName, false);
-
-		/* Can't move a non-shared relation into pg_global */
-		if (params.tablespaceOid == GLOBALTABLESPACE_OID)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot move non-shared relation to tablespace \"%s\"",
-							tablespaceName)));
-	}
+	/* Get tablespaces to use. */
+	params.tablespaceOid = tablespaceName ?
+		get_tablespace_oid(tablespaceName, false) : InvalidOid;
+	params.idxtablespaceOid = idxtablespaceName ?
+		get_tablespace_oid(idxtablespaceName, false) : InvalidOid;
 
 	if (stmt->relation != NULL)
 	{
@@ -293,8 +289,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
  * instead of index order.  This is the new implementation of VACUUM FULL,
  * and error messages should refer to the operation as VACUUM not CLUSTER.
  *
- * "tablespaceOid" is the tablespace where the relation will be rebuilt, or
- * InvalidOid to use its current tablespace.
+ * "tablespaceOid" and "idxtablespaceOid" are the tablespaces where the relation
+ * and its indexes will be rebuilt, or InvalidOid to use their current
+ * tablespaces.
  */
 void
 cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
@@ -464,7 +461,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	TransferPredicateLocksToHeapRelation(OldHeap);
 
 	/* rebuild_relation does all the dirty work */
-	rebuild_relation(OldHeap, indexOid, verbose, params->tablespaceOid);
+	rebuild_relation(OldHeap, indexOid, verbose, params->tablespaceOid,
+			params->idxtablespaceOid);
 
 	/* NB: rebuild_relation does table_close() on OldHeap */
 
@@ -613,10 +611,11 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
  * NB: this routine closes OldHeap at the right time; caller should not.
  */
 static void
-rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespaceOid)
+rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespaceOid, Oid NewIdxTablespaceOid)
 {
 	Oid			tableOid = RelationGetRelid(OldHeap);
 	Oid			tableSpace = OldHeap->rd_rel->reltablespace;
+	Oid			idxtableSpace;
 	Oid			OIDNewHeap;
 	char		relpersistence;
 	bool		is_system_catalog;
@@ -626,7 +625,20 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespace
 
 	/* Use new tablespace if passed. */
 	if (OidIsValid(NewTablespaceOid))
+	{
 		tableSpace = NewTablespaceOid;
+		/* It's not a shared catalog, so refuse to move it to shared tablespace */
+		if (tableSpace == GLOBALTABLESPACE_OID)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot move non-shared relation to tablespace \"%s\"",
+							get_tablespace_name(tableSpace))));
+	}
+
+	if (OidIsValid(NewIdxTablespaceOid))
+		idxtableSpace = NewIdxTablespaceOid;
+	else
+		idxtableSpace = get_rel_tablespace(indexOid);
 
 	/* Mark the correct index as clustered */
 	if (OidIsValid(indexOid))
@@ -655,7 +667,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose, Oid NewTablespace
 	finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
 					 swap_toast_by_content, false, true,
 					 frozenXid, cutoffMulti,
-					 relpersistence);
+					 relpersistence, idxtableSpace);
 }
 
 
@@ -1403,12 +1415,12 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 				 bool is_internal,
 				 TransactionId frozenXid,
 				 MultiXactId cutoffMulti,
-				 char newrelpersistence)
+				 char newrelpersistence, Oid idxtableSpace)
 {
 	ObjectAddress object;
 	Oid			mapped_tables[4];
 	int			reindex_flags;
-	ReindexParams reindex_params = {0};
+	ReindexParams reindex_params = { .tablespaceOid = idxtableSpace };
 	int			i;
 
 	/* Report that we are now swapping relation files */
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index c5c25ce11d..ad1bfdc0d2 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -855,7 +855,8 @@ static void
 refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence)
 {
 	finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true,
-					 RecentXmin, ReadNextMultiXactId(), relpersistence);
+					 RecentXmin, ReadNextMultiXactId(), relpersistence,
+					 InvalidOid);
 }
 
 /*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 75b04eb161..eba8478f91 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5077,7 +5077,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
 							 !OidIsValid(tab->newTableSpace),
 							 RecentXmin,
 							 ReadNextMultiXactId(),
-							 persistence);
+							 persistence, InvalidOid);
 		}
 		else
 		{
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 61565c3f14..0eb9786bea 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -109,9 +109,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		disable_page_skipping = false;
 	ListCell   *lc;
 
-	/* Name and Oid of tablespace to use for relations after VACUUM FULL. */
-	char		*tablespacename = NULL;
-	Oid 		tablespaceOid = InvalidOid;
+	/* Tablespace to use for relations after VACUUM FULL. */
+	char		*tablespacename = NULL,
+				*idxtablespacename = NULL;
 
 	/* Set default value */
 	params.index_cleanup = VACOPT_TERNARY_DEFAULT;
@@ -151,6 +151,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			params.truncate = get_vacopt_ternary_value(opt);
 		else if (strcmp(opt->defname, "tablespace") == 0)
 			tablespacename = defGetString(opt);
+		else if (strcmp(opt->defname, "index_tablespace") == 0)
+			idxtablespacename = defGetString(opt);
 		else if (strcmp(opt->defname, "parallel") == 0)
 		{
 			if (opt->arg == NULL)
@@ -211,26 +213,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("VACUUM FULL cannot be performed in parallel")));
 
-	/* Get tablespace Oid to use. */
-	if (tablespacename)
-	{
-		if ((params.options & VACOPT_FULL) == 0)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("incompatible TABLESPACE option"),
-					errdetail("TABLESPACE can only be used with VACUUM FULL.")));
-
-		tablespaceOid = get_tablespace_oid(tablespacename, false);
-
-		/* Can't move a non-shared relation into pg_global */
-		if (tablespaceOid == GLOBALTABLESPACE_OID)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					errmsg("cannot move non-shared relation to tablespace \"%s\"",
-							tablespacename)));
+	if ((params.options & VACOPT_FULL) == 0 &&
+		(tablespacename || idxtablespacename))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("incompatible TABLESPACE option"),
+				errdetail("TABLESPACE can only be used with VACUUM FULL.")));
 
-	}
-	params.tablespace_oid = tablespaceOid;
+	/* Get tablespace Oids to use. */
+	params.tablespace_oid = tablespacename ?
+		get_tablespace_oid(tablespacename, false) : InvalidOid;
+	params.idxtablespace_oid = idxtablespacename ?
+		get_tablespace_oid(idxtablespacename, false) : InvalidOid;
 
 	/*
 	 * Make sure VACOPT_ANALYZE is specified if any column lists are present.
@@ -1964,6 +1958,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	{
 		ClusterParams cluster_params = {
 			.tablespaceOid = params->tablespace_oid,
+			.idxtablespaceOid = params->idxtablespace_oid,
+			/* Other params initialized to 0/false/NULL */
 		};
 
 		/* close relation before vacuuming, but hold lock until commit */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index a5d98c8c95..fd9eb75e11 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2933,6 +2933,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		tab->at_params.is_wraparound = wraparound;
 		tab->at_params.log_min_duration = log_min_duration;
 		tab->at_params.tablespace_oid = InvalidOid;
+		tab->at_params.idxtablespace_oid = InvalidOid;
 		tab->at_vacuum_cost_limit = vac_cost_limit;
 		tab->at_vacuum_cost_delay = vac_cost_delay;
 		tab->at_relname = NULL;
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index 868dea5ecc..c04a2ce20b 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -28,6 +28,7 @@ typedef struct ClusterParams
 {
 	bits32		options;		/* bitmask of CLUOPT_* */
 	Oid			tablespaceOid;	/* tablespace to rebuild relation, or InvalidOid */
+	Oid			idxtablespaceOid;	/* tablespace to rebuild relation's indexes */
 } ClusterParams;
 
 extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel);
@@ -45,6 +46,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 							 bool is_internal,
 							 TransactionId frozenXid,
 							 MultiXactId minMulti,
-							 char newrelpersistence);
+							 char newrelpersistence,
+							 Oid idxtablespaceOid);
 
 #endif							/* CLUSTER_H */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 0934487d4b..6c4e085a45 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -227,8 +227,9 @@ typedef struct VacuumParams
 	 * disabled.
 	 */
 	int			nworkers;
-	Oid			tablespace_oid; /* tablespace to use for relations
-								 * rebuilt by VACUUM FULL */
+	/* tablespaces to use for relations rebuilt by VACUUM FULL */
+	Oid			tablespace_oid;
+	Oid			idxtablespace_oid;
 } VacuumParams;
 
 /* GUC parameters */
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 2244c1d51d..9a13112605 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -80,6 +80,19 @@ REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; --
 SELECT relname FROM pg_class
 WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
 
+-- check CLUSTER with INDEX_TABLESPACE change to non-default location
+CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok
+-- check relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+-- check VACUUM with INDEX_TABLESPACE change
+VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok
+-- check relations moved back to pg_default
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+
 -- create a schema we can use
 CREATE SCHEMA testschema;
 
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 889933a9c8..2f8ecf514d 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -114,6 +114,26 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa
 ---------
 (0 rows)
 
+-- check CLUSTER with INDEX_TABLESPACE change to non-default location
+CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok
+-- check relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+            relname            
+-------------------------------
+ regress_tblspace_test_tbl_idx
+(1 row)
+
+-- check VACUUM with INDEX_TABLESPACE change
+VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok
+-- check relations moved back to pg_default
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname 
+---------
+(0 rows)
+
 -- create a schema we can use
 CREATE SCHEMA testschema;
 -- try a table
-- 
2.17.0


--tsOsTdHNUZQcU9Ye--





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

* Testing autovacuum wraparound (including failsafe)
@ 2021-04-23 20:43 Andres Freund <[email protected]>
  2021-04-23 23:08 ` Re: Testing autovacuum wraparound (including failsafe) Justin Pryzby <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  0 siblings, 3 replies; 26+ messages in thread

From: Andres Freund @ 2021-04-23 20:43 UTC (permalink / raw)
  To: pgsql-hackers; Peter Geoghegan <[email protected]>

Hi,

I started to write a test for $Subject, which I think we sorely need.

Currently my approach is to:
- start a cluster, create a few tables with test data
- acquire SHARE UPDATE EXCLUSIVE in a prepared transaction, to prevent
  autovacuum from doing anything
- cause dead tuples to exist
- restart
- run pg_resetwal -x 2000027648
- do things like acquiring pins on pages that block vacuum from progressing
- commit prepared transaction
- wait for template0, template1 datfrozenxid to increase
- wait for relfrozenxid for most relations in postgres to increase
- release buffer pin
- wait for postgres datfrozenxid to increase

So far so good. But I've encountered a few things that stand in the way of
enabling such a test by default:

1) During startup StartupSUBTRANS() zeroes out all pages between
   oldestActiveXID and nextXid. That takes 8s on my workstation, but only
   because I have plenty memory - pg_subtrans ends up 14GB as I currently do
   the test. Clearly not something we could do on the BF.

2) FAILSAFE_MIN_PAGES is 4GB - which seems to make it infeasible to test the
   failsafe mode, we can't really create 4GB relations on the BF. While
   writing the tests I've lowered this to 4MB...

3) pg_resetwal -x requires to carefully choose an xid: It needs to be the
   first xid on a clog page. It's not hard to determine which xids are but it
   depends on BLCKSZ and a few constants in clog.c. I've for now hardcoded a
   value appropriate for 8KB, but ...


I have 2 1/2 ideas about addressing 1);

- We could exposing functionality to do advance nextXid to a future value at
  runtime, without filling in clog/subtrans pages. Would probably have to live
  in varsup.c and be exposed via regress.so or such?

- The only reason StartupSUBTRANS() does that work is because of the prepared
  transaction holding back oldestActiveXID. That transaction in turn exists to
  prevent autovacuum from doing anything before we do test setup
  steps.

  Perhaps it'd be sufficient to set autovacuum_naptime really high initially,
  perform the test setup, set naptime to something lower, reload config. But
  I'm worried that might not be reliable: If something ends up allocating an
  xid we'd potentially reach the path in GetNewTransaction() that wakes up the
  launcher?  But probably there wouldn't be anything doing so?

  Another aspect that might not make this a good choice is that it actually
  seems relevant to be able to test cases where there are very old still
  running transactions...

- As a variant of the previous idea: If that turns out to be unreliable, we
  could instead set nextxid, start in single user mode, create a blocking 2PC
  transaction, start normally. Because there's no old active xid we'd not run
  into the StartupSUBTRANS problem.


For 2), I don't really have a better idea than making that configurable
somehow?

3) is probably tolerable for now, we could skip the test if BLCKSZ isn't 8KB,
or we could hardcode the calculation for different block sizes.



I noticed one minor bug that's likely new:

2021-04-23 13:32:30.899 PDT [2027738] LOG:  automatic aggressive vacuum to prevent wraparound of table "postgres.public.small_trunc": index scans: 1
        pages: 400 removed, 28 remain, 0 skipped due to pins, 0 skipped frozen
        tuples: 14000 removed, 1000 remain, 0 are dead but not yet removable, oldest xmin: 2000027651
        buffer usage: 735 hits, 1262 misses, 874 dirtied
        index scan needed: 401 pages from table (1432.14% of total) had 14000 dead item identifiers removed
        index "small_trunc_pkey": pages: 43 in total, 37 newly deleted, 37 currently deleted, 0 reusable
        avg read rate: 559.048 MB/s, avg write rate: 387.170 MB/s
        system usage: CPU: user: 0.01 s, system: 0.00 s, elapsed: 0.01 s
        WAL usage: 1809 records, 474 full page images, 3977538 bytes

'1432.14% of total' - looks like removed pages need to be added before the
percentage calculation?

Greetings,

Andres Freund





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-04-23 23:08 ` Justin Pryzby <[email protected]>
  2021-04-23 23:26   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2 siblings, 1 reply; 26+ messages in thread

From: Justin Pryzby @ 2021-04-23 23:08 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; Peter Geoghegan <[email protected]>

On Fri, Apr 23, 2021 at 01:43:06PM -0700, Andres Freund wrote:
> 2) FAILSAFE_MIN_PAGES is 4GB - which seems to make it infeasible to test the
>    failsafe mode, we can't really create 4GB relations on the BF. While
>    writing the tests I've lowered this to 4MB...

> For 2), I don't really have a better idea than making that configurable
> somehow?

Does it work to shut down the cluster and create the .0,.1,.2,.3 segments of a
new, empty relation with zero blocks using something like truncate -s 1G ?

-- 
Justin





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:08 ` Re: Testing autovacuum wraparound (including failsafe) Justin Pryzby <[email protected]>
@ 2021-04-23 23:26   ` Andres Freund <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Andres Freund @ 2021-04-23 23:26 UTC (permalink / raw)
  To: Justin Pryzby <[email protected]>; +Cc: pgsql-hackers; Peter Geoghegan <[email protected]>

Hi,

On 2021-04-23 18:08:12 -0500, Justin Pryzby wrote:
> On Fri, Apr 23, 2021 at 01:43:06PM -0700, Andres Freund wrote:
> > 2) FAILSAFE_MIN_PAGES is 4GB - which seems to make it infeasible to test the
> >    failsafe mode, we can't really create 4GB relations on the BF. While
> >    writing the tests I've lowered this to 4MB...
> 
> > For 2), I don't really have a better idea than making that configurable
> > somehow?
> 
> Does it work to shut down the cluster and create the .0,.1,.2,.3 segments of a
> new, empty relation with zero blocks using something like truncate -s 1G ?

I'd like this to be portable to at least windows - I don't know how well
that deals with sparse files. But the bigger issue is that that IIRC
will trigger vacuum to try to initialize all those pages, which will
then force all that space to be allocated anyway...

Greetings,

Andres Freund





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-04-23 23:12 ` Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2 siblings, 1 reply; 26+ messages in thread

From: Peter Geoghegan @ 2021-04-23 23:12 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers

On Fri, Apr 23, 2021 at 1:43 PM Andres Freund <[email protected]> wrote:
> I started to write a test for $Subject, which I think we sorely need.

+1

> Currently my approach is to:
> - start a cluster, create a few tables with test data
> - acquire SHARE UPDATE EXCLUSIVE in a prepared transaction, to prevent
>   autovacuum from doing anything
> - cause dead tuples to exist
> - restart
> - run pg_resetwal -x 2000027648
> - do things like acquiring pins on pages that block vacuum from progressing
> - commit prepared transaction
> - wait for template0, template1 datfrozenxid to increase
> - wait for relfrozenxid for most relations in postgres to increase
> - release buffer pin
> - wait for postgres datfrozenxid to increase

Just having a standard-ish way to do stress testing like this would
add something.

> 2) FAILSAFE_MIN_PAGES is 4GB - which seems to make it infeasible to test the
>    failsafe mode, we can't really create 4GB relations on the BF. While
>    writing the tests I've lowered this to 4MB...

The only reason that I chose 4GB for FAILSAFE_MIN_PAGES is because the
related VACUUM_FSM_EVERY_PAGES constant was 8GB -- the latter limits
how often we'll consider the failsafe in the single-pass/no-indexes
case.

I see no reason why it cannot be changed now. VACUUM_FSM_EVERY_PAGES
also frustrates FSM testing in the single-pass case in about the same
way, so maybe that should be considered as well? Note that the FSM
handling for the single pass case is actually a bit different to the
two pass/has-indexes case, since the single pass case calls
lazy_vacuum_heap_page() directly in its first and only pass over the
heap (that's the whole point of having it of course).

> 3) pg_resetwal -x requires to carefully choose an xid: It needs to be the
>    first xid on a clog page. It's not hard to determine which xids are but it
>    depends on BLCKSZ and a few constants in clog.c. I've for now hardcoded a
>    value appropriate for 8KB, but ...

Ugh.

> For 2), I don't really have a better idea than making that configurable
> somehow?

That could make sense as a developer/testing option, I suppose. I just
doubt that it makes sense as anything else.

> 2021-04-23 13:32:30.899 PDT [2027738] LOG:  automatic aggressive vacuum to prevent wraparound of table "postgres.public.small_trunc": index scans: 1
>         pages: 400 removed, 28 remain, 0 skipped due to pins, 0 skipped frozen
>         tuples: 14000 removed, 1000 remain, 0 are dead but not yet removable, oldest xmin: 2000027651
>         buffer usage: 735 hits, 1262 misses, 874 dirtied
>         index scan needed: 401 pages from table (1432.14% of total) had 14000 dead item identifiers removed
>         index "small_trunc_pkey": pages: 43 in total, 37 newly deleted, 37 currently deleted, 0 reusable
>         avg read rate: 559.048 MB/s, avg write rate: 387.170 MB/s
>         system usage: CPU: user: 0.01 s, system: 0.00 s, elapsed: 0.01 s
>         WAL usage: 1809 records, 474 full page images, 3977538 bytes
>
> '1432.14% of total' - looks like removed pages need to be added before the
> percentage calculation?

Clearly this needs to account for removed heap pages in order to
consistently express the percentage of pages with LP_DEAD items in
terms of a percentage of the original table size. I can fix this
shortly.

--
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-04-24 00:29   ` Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andres Freund @ 2021-04-24 00:29 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: pgsql-hackers

Hi,

On 2021-04-23 16:12:33 -0700, Peter Geoghegan wrote:
> The only reason that I chose 4GB for FAILSAFE_MIN_PAGES is because the
> related VACUUM_FSM_EVERY_PAGES constant was 8GB -- the latter limits
> how often we'll consider the failsafe in the single-pass/no-indexes
> case.

I don't really understand why it makes sense to tie FAILSAFE_MIN_PAGES
and VACUUM_FSM_EVERY_PAGES together? They seem pretty independent to me?



> I see no reason why it cannot be changed now. VACUUM_FSM_EVERY_PAGES
> also frustrates FSM testing in the single-pass case in about the same
> way, so maybe that should be considered as well? Note that the FSM
> handling for the single pass case is actually a bit different to the
> two pass/has-indexes case, since the single pass case calls
> lazy_vacuum_heap_page() directly in its first and only pass over the
> heap (that's the whole point of having it of course).

I'm not opposed to lowering VACUUM_FSM_EVERY_PAGES (the costs don't seem
all that high compared to vacuuming?), but I don't think there's as
clear a need for testing around that as there is around wraparound.


The failsafe mode affects the table scan itself by disabling cost
limiting. As far as I can see the ways it triggers for the table scan (vs
truncation or index processing) are:

1) Before vacuuming starts, for heap phases and indexes, if already
   necessary at that point
2) For a table with indexes, before/after each index vacuum, if now
   necessary
3) On a table without indexes, every 8GB, iff there are dead tuples, if now necessary

Why would we want to trigger the failsafe mode during a scan of a table
with dead tuples and no indexes, but not on a table without dead tuples
or with indexes but fewer than m_w_m dead tuples? That makes little
sense to me.


It seems that for the no-index case the warning message is quite off?

		ereport(WARNING,
				(errmsg("abandoned index vacuuming of table \"%s.%s.%s\" as a failsafe after %d index scans",

Doesn't exactly make one understand that vacuum cost limiting now is
disabled? And is confusing because there would never be index vacuuming?

And even in the cases indexes exist, it's odd to talk about abandoning
index vacuuming that hasn't even started yet?


> > For 2), I don't really have a better idea than making that configurable
> > somehow?
> 
> That could make sense as a developer/testing option, I suppose. I just
> doubt that it makes sense as anything else.

Yea, I only was thinking of making it configurable to be able to test
it. If we change the limit to something considerably lower I wouldn't
see a need for that anymore.

Greetings,

Andres Freund





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-04-24 02:15     ` Peter Geoghegan <[email protected]>
  2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Peter Geoghegan @ 2021-04-24 02:15 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers

On Fri, Apr 23, 2021 at 5:29 PM Andres Freund <[email protected]> wrote:
> On 2021-04-23 16:12:33 -0700, Peter Geoghegan wrote:
> > The only reason that I chose 4GB for FAILSAFE_MIN_PAGES is because the
> > related VACUUM_FSM_EVERY_PAGES constant was 8GB -- the latter limits
> > how often we'll consider the failsafe in the single-pass/no-indexes
> > case.
>
> I don't really understand why it makes sense to tie FAILSAFE_MIN_PAGES
> and VACUUM_FSM_EVERY_PAGES together? They seem pretty independent to me?

VACUUM_FSM_EVERY_PAGES controls how often VACUUM does work that
usually takes place right after the two pass case finishes a round of
index and heap vacuuming. This is work that we certainly don't want to
do every time we process a single heap page in the one-pass/no-indexes
case. Initially this just meant FSM vacuuming, but it now includes a
failsafe check.

Of course all of the precise details here are fairly arbitrary
(including VACUUM_FSM_EVERY_PAGES, which has been around for a couple
of releases now). The overall goal that I had in mind was to make the
one-pass case's use of the failsafe have analogous behavior to the
two-pass/has-indexes case -- a goal which was itself somewhat
arbitrary.

> The failsafe mode affects the table scan itself by disabling cost
> limiting. As far as I can see the ways it triggers for the table scan (vs
> truncation or index processing) are:
>
> 1) Before vacuuming starts, for heap phases and indexes, if already
>    necessary at that point
> 2) For a table with indexes, before/after each index vacuum, if now
>    necessary
> 3) On a table without indexes, every 8GB, iff there are dead tuples, if now necessary
>
> Why would we want to trigger the failsafe mode during a scan of a table
> with dead tuples and no indexes, but not on a table without dead tuples
> or with indexes but fewer than m_w_m dead tuples? That makes little
> sense to me.

What alternative does make sense to you?

It seemed important to put the failsafe check at points where we do
other analogous work in all cases. We made a pragmatic trade-off. In
theory almost any scheme might not check often enough, and/or might
check too frequently.

> It seems that for the no-index case the warning message is quite off?

I'll fix that up some point soon. FWIW this happened because the
support for one-pass VACUUM was added quite late, at Robert's request.

Another issue with the failsafe commit is that we haven't considered
the autovacuum_multixact_freeze_max_age table reloption -- we only
check the GUC. That might have accidentally been the right thing to
do, though, since the reloption is interpreted as lower than the GUC
in all cases anyway -- arguably the
autovacuum_multixact_freeze_max_age GUC should be all we care about
anyway. I will need to think about this question some more, though.

> > > For 2), I don't really have a better idea than making that configurable
> > > somehow?
> >
> > That could make sense as a developer/testing option, I suppose. I just
> > doubt that it makes sense as anything else.
>
> Yea, I only was thinking of making it configurable to be able to test
> it. If we change the limit to something considerably lower I wouldn't
> see a need for that anymore.

It would probably be okay to just lower it significantly. Not sure if
that's the best approach, though. Will pick it up next week.

-- 
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-04-24 02:33       ` Andres Freund <[email protected]>
  2021-04-24 02:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Andres Freund @ 2021-04-24 02:33 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: pgsql-hackers

Hi,

On 2021-04-23 19:15:43 -0700, Peter Geoghegan wrote:
> > The failsafe mode affects the table scan itself by disabling cost
> > limiting. As far as I can see the ways it triggers for the table scan (vs
> > truncation or index processing) are:
> >
> > 1) Before vacuuming starts, for heap phases and indexes, if already
> >    necessary at that point
> > 2) For a table with indexes, before/after each index vacuum, if now
> >    necessary
> > 3) On a table without indexes, every 8GB, iff there are dead tuples, if now necessary
> >
> > Why would we want to trigger the failsafe mode during a scan of a table
> > with dead tuples and no indexes, but not on a table without dead tuples
> > or with indexes but fewer than m_w_m dead tuples? That makes little
> > sense to me.
> 
> What alternative does make sense to you?

Check it every so often, independent of whether there are indexes or
dead tuples? Or just check it at the boundaries.

I'd make it dependent on the number of pages scanned, rather than the
block distance to the last check - otherwise we might end up doing it
way too often when there's only a few individual pages not in the freeze
map.

Greetings,

Andres Freund





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-04-24 02:42         ` Peter Geoghegan <[email protected]>
  2021-04-24 02:53           ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Peter Geoghegan @ 2021-04-24 02:42 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers

On Fri, Apr 23, 2021 at 7:33 PM Andres Freund <[email protected]> wrote:
> Check it every so often, independent of whether there are indexes or
> dead tuples? Or just check it at the boundaries.

I think that the former suggestion might be better -- I actually
thought about doing it that way myself.

The latter suggestion sounds like you're suggesting that we just check
it at the beginning and the end in all cases (we do the beginning in
all cases already, but now we'd also do the end outside of the loop in
all cases). Is that right? If that is what you meant, then you should
note that there'd hardly be any check in the one-pass case with that
scheme (apart from the initial check that we do already). The only
work we'd be skipping at the end (in the event of that check
triggering the failsafe) would be heap truncation, which (as you've
pointed out yourself) doesn't seem particularly likely to matter.

-- 
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-04-24 02:53           ` Andres Freund <[email protected]>
  2021-04-24 02:56             ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andres Freund @ 2021-04-24 02:53 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: pgsql-hackers

Hi,

On 2021-04-23 19:42:30 -0700, Peter Geoghegan wrote:
> On Fri, Apr 23, 2021 at 7:33 PM Andres Freund <[email protected]> wrote:
> > Check it every so often, independent of whether there are indexes or
> > dead tuples? Or just check it at the boundaries.
>
> I think that the former suggestion might be better -- I actually
> thought about doing it that way myself.

Cool.


> The latter suggestion sounds like you're suggesting that we just check
> it at the beginning and the end in all cases (we do the beginning in
> all cases already, but now we'd also do the end outside of the loop in
> all cases). Is that right?

Yes.


> If that is what you meant, then you should note that there'd hardly be
> any check in the one-pass case with that scheme (apart from the
> initial check that we do already). The only work we'd be skipping at
> the end (in the event of that check triggering the failsafe) would be
> heap truncation, which (as you've pointed out yourself) doesn't seem
> particularly likely to matter.

I mainly suggested it because to me the current seems hard to
understand. I do think it'd be better to check more often. But checking
depending on the amount of dead tuples at the right time doesn't strike
me as a good idea - a lot of anti-wraparound vacuums will mainly be
freezing tuples, rather than removing a lot of dead rows. Which makes it
hard to understand when the failsafe kicks in.

Greetings,

Andres Freund





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:53           ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-04-24 02:56             ` Peter Geoghegan <[email protected]>
  2021-05-14 01:03               ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Peter Geoghegan @ 2021-04-24 02:56 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers

On Fri, Apr 23, 2021 at 7:53 PM Andres Freund <[email protected]> wrote:
> I mainly suggested it because to me the current seems hard to
> understand. I do think it'd be better to check more often. But checking
> depending on the amount of dead tuples at the right time doesn't strike
> me as a good idea - a lot of anti-wraparound vacuums will mainly be
> freezing tuples, rather than removing a lot of dead rows. Which makes it
> hard to understand when the failsafe kicks in.

I'm convinced -- decoupling the logic from the one-pass-not-two pass
case seems likely to be simpler and more useful. For both the one pass
and two pass/has indexes case.

-- 
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 02:53           ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:56             ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-05-14 01:03               ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Peter Geoghegan @ 2021-05-14 01:03 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers

On Fri, Apr 23, 2021 at 7:56 PM Peter Geoghegan <[email protected]> wrote:
> I'm convinced -- decoupling the logic from the one-pass-not-two pass
> case seems likely to be simpler and more useful. For both the one pass
> and two pass/has indexes case.

Attached draft patch does it that way.

-- 
Peter Geoghegan


Attachments:

  [application/octet-stream] v1-0001-Consider-triggering-failsafe-during-first-scan.patch (4.0K, ../../CAH2-WznB82TWSy7_5tH0ByvY=w+CLDqK+DPGTE3i9jqBYEPuvw@mail.gmail.com/2-v1-0001-Consider-triggering-failsafe-during-first-scan.patch)
  download | inline diff:
From 2a67208c7f660f23eb302288b0b74cbb0e839011 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <[email protected]>
Date: Thu, 13 May 2021 17:53:10 -0700
Subject: [PATCH v1] Consider triggering failsafe during first scan.

---
 src/backend/access/heap/vacuumlazy.c | 34 ++++++++++++----------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 9f1f8e340d..2dd3fbe07a 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -110,10 +110,9 @@
 #define BYPASS_THRESHOLD_PAGES	0.02	/* i.e. 2% of rel_pages */
 
 /*
- * When a table is small (i.e. smaller than this), save cycles by avoiding
- * repeated failsafe checks
+ * Perform failsafe checks every 4GB, approximately
  */
-#define FAILSAFE_MIN_PAGES \
+#define FAILSAFE_EVERY_PAGES \
 	((BlockNumber) (((uint64) 4 * 1024 * 1024 * 1024) / BLCKSZ))
 
 /*
@@ -890,6 +889,7 @@ lazy_scan_heap(LVRelState *vacrel, VacuumParams *params, bool aggressive)
 	BlockNumber nblocks,
 				blkno,
 				next_unskippable_block,
+				next_failsafe_block,
 				next_fsm_block_to_vacuum;
 	PGRUsage	ru0;
 	Buffer		vmbuffer = InvalidBuffer;
@@ -919,6 +919,7 @@ lazy_scan_heap(LVRelState *vacrel, VacuumParams *params, bool aggressive)
 
 	nblocks = RelationGetNumberOfBlocks(vacrel->rel);
 	next_unskippable_block = 0;
+	next_failsafe_block = 0;
 	next_fsm_block_to_vacuum = 0;
 	vacrel->rel_pages = nblocks;
 	vacrel->scanned_pages = 0;
@@ -1168,6 +1169,15 @@ lazy_scan_heap(LVRelState *vacrel, VacuumParams *params, bool aggressive)
 										 PROGRESS_VACUUM_PHASE_SCAN_HEAP);
 		}
 
+		/*
+		 * Regularly consider if wraparound failsafe should trigger
+		 */
+		if (blkno - next_failsafe_block >= FAILSAFE_EVERY_PAGES)
+		{
+			lazy_check_wraparound_failsafe(vacrel);
+			next_failsafe_block  = blkno;
+		}
+
 		/*
 		 * Set up visibility map page as needed.
 		 *
@@ -1375,17 +1385,12 @@ lazy_scan_heap(LVRelState *vacrel, VacuumParams *params, bool aggressive)
 				 * Periodically perform FSM vacuuming to make newly-freed
 				 * space visible on upper FSM pages.  Note we have not yet
 				 * performed FSM processing for blkno.
-				 *
-				 * Call lazy_check_wraparound_failsafe() here, too, since we
-				 * also don't want to do that too frequently, or too
-				 * infrequently.
 				 */
 				if (blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES)
 				{
 					FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum,
 											blkno);
 					next_fsm_block_to_vacuum = blkno;
-					lazy_check_wraparound_failsafe(vacrel);
 				}
 
 				/*
@@ -2567,22 +2572,13 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup, LVRelState *vacrel)
  * that it started out with.
  *
  * Returns true when failsafe has been triggered.
- *
- * Caller is expected to call here before and after vacuuming each index in
- * the case of two-pass VACUUM, or every VACUUM_FSM_EVERY_PAGES blocks in the
- * case of no-indexes/one-pass VACUUM.
- *
- * There is also a precheck before the first pass over the heap begins, which
- * is helpful when the failsafe initially triggers during a non-aggressive
- * VACUUM -- the automatic aggressive vacuum to prevent wraparound that
- * follows can independently trigger the failsafe right away.
  */
 static bool
 lazy_check_wraparound_failsafe(LVRelState *vacrel)
 {
 	/* Avoid calling vacuum_xid_failsafe_check() very frequently */
 	if (vacrel->num_index_scans == 0 &&
-		vacrel->rel_pages <= FAILSAFE_MIN_PAGES)
+		vacrel->rel_pages <= FAILSAFE_EVERY_PAGES)
 		return false;
 
 	/* Don't warn more than once per VACUUM */
@@ -2600,7 +2596,7 @@ lazy_check_wraparound_failsafe(LVRelState *vacrel)
 		vacrel->do_failsafe = true;
 
 		ereport(WARNING,
-				(errmsg("abandoned index vacuuming of table \"%s.%s.%s\" as a failsafe after %d index scans",
+				(errmsg("bypassing nonessential maintenance of table \"%s.%s.%s\" as a failsafe after %d index scans",
 						get_database_name(MyDatabaseId),
 						vacrel->relnamespace,
 						vacrel->relname,
-- 
2.27.0



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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-05-18 05:28       ` Masahiko Sawada <[email protected]>
  2021-05-18 05:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2021-05-18 05:28 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers

On Sat, Apr 24, 2021 at 11:16 AM Peter Geoghegan <[email protected]> wrote:
>
> On Fri, Apr 23, 2021 at 5:29 PM Andres Freund <[email protected]> wrote:
> > On 2021-04-23 16:12:33 -0700, Peter Geoghegan wrote:
> > > The only reason that I chose 4GB for FAILSAFE_MIN_PAGES is because the
> > > related VACUUM_FSM_EVERY_PAGES constant was 8GB -- the latter limits
> > > how often we'll consider the failsafe in the single-pass/no-indexes
> > > case.
> >
> > I don't really understand why it makes sense to tie FAILSAFE_MIN_PAGES
> > and VACUUM_FSM_EVERY_PAGES together? They seem pretty independent to me?
>
> VACUUM_FSM_EVERY_PAGES controls how often VACUUM does work that
> usually takes place right after the two pass case finishes a round of
> index and heap vacuuming. This is work that we certainly don't want to
> do every time we process a single heap page in the one-pass/no-indexes
> case. Initially this just meant FSM vacuuming, but it now includes a
> failsafe check.
>
> Of course all of the precise details here are fairly arbitrary
> (including VACUUM_FSM_EVERY_PAGES, which has been around for a couple
> of releases now). The overall goal that I had in mind was to make the
> one-pass case's use of the failsafe have analogous behavior to the
> two-pass/has-indexes case -- a goal which was itself somewhat
> arbitrary.
>
> > The failsafe mode affects the table scan itself by disabling cost
> > limiting. As far as I can see the ways it triggers for the table scan (vs
> > truncation or index processing) are:
> >
> > 1) Before vacuuming starts, for heap phases and indexes, if already
> >    necessary at that point
> > 2) For a table with indexes, before/after each index vacuum, if now
> >    necessary
> > 3) On a table without indexes, every 8GB, iff there are dead tuples, if now necessary
> >
> > Why would we want to trigger the failsafe mode during a scan of a table
> > with dead tuples and no indexes, but not on a table without dead tuples
> > or with indexes but fewer than m_w_m dead tuples? That makes little
> > sense to me.
>
> What alternative does make sense to you?
>
> It seemed important to put the failsafe check at points where we do
> other analogous work in all cases. We made a pragmatic trade-off. In
> theory almost any scheme might not check often enough, and/or might
> check too frequently.
>
> > It seems that for the no-index case the warning message is quite off?
>
> I'll fix that up some point soon. FWIW this happened because the
> support for one-pass VACUUM was added quite late, at Robert's request.

+1 to fix this. Are you already working on fixing this? If not, I'll
post a patch.

>
> Another issue with the failsafe commit is that we haven't considered
> the autovacuum_multixact_freeze_max_age table reloption -- we only
> check the GUC. That might have accidentally been the right thing to
> do, though, since the reloption is interpreted as lower than the GUC
> in all cases anyway -- arguably the
> autovacuum_multixact_freeze_max_age GUC should be all we care about
> anyway. I will need to think about this question some more, though.

FWIW, I intentionally ignored the reloption there since they're
interpreted as lower than the GUC as you mentioned and the situation
where we need to enter the failsafe mode is not the table-specific
problem but a system-wide problem.

Regards,

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





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2021-05-18 05:42         ` Peter Geoghegan <[email protected]>
  2021-05-18 05:46           ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Peter Geoghegan @ 2021-05-18 05:42 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers

On Mon, May 17, 2021 at 10:29 PM Masahiko Sawada <[email protected]> wrote:
> +1 to fix this. Are you already working on fixing this? If not, I'll
> post a patch.

I posted a patch recently (last Thursday my time). Perhaps you can review it?

-- 
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2021-05-18 05:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
@ 2021-05-18 05:46           ` Masahiko Sawada <[email protected]>
  2021-05-18 07:09             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2021-05-18 05:46 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers

On Tue, May 18, 2021 at 2:42 PM Peter Geoghegan <[email protected]> wrote:
>
> On Mon, May 17, 2021 at 10:29 PM Masahiko Sawada <[email protected]> wrote:
> > +1 to fix this. Are you already working on fixing this? If not, I'll
> > post a patch.
>
> I posted a patch recently (last Thursday my time). Perhaps you can review it?

Oh, I missed that the patch includes that fix. I'll review the patch.

Regards,

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





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2021-05-18 05:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:46           ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2021-05-18 07:09             ` Masahiko Sawada <[email protected]>
  2021-05-25 00:14               ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2021-05-18 07:09 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers

On Tue, May 18, 2021 at 2:46 PM Masahiko Sawada <[email protected]> wrote:
>
> On Tue, May 18, 2021 at 2:42 PM Peter Geoghegan <[email protected]> wrote:
> >
> > On Mon, May 17, 2021 at 10:29 PM Masahiko Sawada <[email protected]> wrote:
> > > +1 to fix this. Are you already working on fixing this? If not, I'll
> > > post a patch.
> >
> > I posted a patch recently (last Thursday my time). Perhaps you can review it?
>
> Oh, I missed that the patch includes that fix. I'll review the patch.
>

I've reviewed the patch. Here is one comment:

    if (vacrel->num_index_scans == 0 &&
-       vacrel->rel_pages <= FAILSAFE_MIN_PAGES)
+       vacrel->rel_pages <= FAILSAFE_EVERY_PAGES)
        return false;

Since there is the condition "vacrel->num_index_scans == 0" we could
enter the failsafe mode even if the table is less than 4GB, if we
enter lazy_check_wraparound_failsafe() after executing more than one
index scan. Whereas a vacuum on the table that is less than 4GB and
has no index never enters the failsafe mode. I think we can remove
this condition since I don't see the reason why we don't allow to
enter the failsafe mode only when the first-time index scan in the
case of such tables. What do you think?

Regards,

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





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2021-05-18 05:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  2021-05-18 05:46           ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2021-05-18 07:09             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2021-05-25 00:14               ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Peter Geoghegan @ 2021-05-25 00:14 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers

On Tue, May 18, 2021 at 12:10 AM Masahiko Sawada <[email protected]> wrote:
> Since there is the condition "vacrel->num_index_scans == 0" we could
> enter the failsafe mode even if the table is less than 4GB, if we
> enter lazy_check_wraparound_failsafe() after executing more than one
> index scan. Whereas a vacuum on the table that is less than 4GB and
> has no index never enters the failsafe mode. I think we can remove
> this condition since I don't see the reason why we don't allow to
> enter the failsafe mode only when the first-time index scan in the
> case of such tables. What do you think?

I'm convinced -- this does seem like premature optimization now.

I pushed a version of the patch that removes that code just now.

Thanks
-- 
Peter Geoghegan





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2021-06-10 13:42 ` Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2 siblings, 1 reply; 26+ messages in thread

From: Anastasia Lubennikova @ 2021-06-10 13:42 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: pgsql-hackers; Peter Geoghegan <[email protected]>

On Thu, Jun 10, 2021 at 10:52 AM Andres Freund <[email protected]> wrote:

>
> I started to write a test for $Subject, which I think we sorely need.
>
> Currently my approach is to:
> - start a cluster, create a few tables with test data
> - acquire SHARE UPDATE EXCLUSIVE in a prepared transaction, to prevent
>   autovacuum from doing anything
> - cause dead tuples to exist
> - restart
> - run pg_resetwal -x 2000027648
> - do things like acquiring pins on pages that block vacuum from progressing
> - commit prepared transaction
> - wait for template0, template1 datfrozenxid to increase
> - wait for relfrozenxid for most relations in postgres to increase
> - release buffer pin
> - wait for postgres datfrozenxid to increase
>
>
Cool. Thank you for working on that!
Could you please share a WIP patch for the $subj? I'd be happy to help with
it.

So far so good. But I've encountered a few things that stand in the way of
> enabling such a test by default:
>
> 1) During startup StartupSUBTRANS() zeroes out all pages between
>    oldestActiveXID and nextXid. That takes 8s on my workstation, but only
>    because I have plenty memory - pg_subtrans ends up 14GB as I currently
> do
>    the test. Clearly not something we could do on the BF.
>  ....
>
3) pg_resetwal -x requires to carefully choose an xid: It needs to be the
>    first xid on a clog page. It's not hard to determine which xids are but
> it
>    depends on BLCKSZ and a few constants in clog.c. I've for now hardcoded
> a
>    value appropriate for 8KB, but ...
>
> Maybe we can add new pg_resetwal option?  Something like pg_resetwal
--xid-near-wraparound, which will ask pg_resetwal to calculate exact xid
value using values from pg_control and clog macros?
I think it might come in handy for manual testing too.


> I have 2 1/2 ideas about addressing 1);
>
> - We could exposing functionality to do advance nextXid to a future value
> at
>   runtime, without filling in clog/subtrans pages. Would probably have to
> live
>   in varsup.c and be exposed via regress.so or such?
>
> This option looks scary to me. Several functions rely on the fact that
StartupSUBTRANS() have zeroed pages.
And if we will do it conditional just for tests, it means that we won't
test the real code path.

- The only reason StartupSUBTRANS() does that work is because of the
> prepared
>   transaction holding back oldestActiveXID. That transaction in turn
> exists to
>   prevent autovacuum from doing anything before we do test setup
>   steps.
>


>
>   Perhaps it'd be sufficient to set autovacuum_naptime really high
> initially,
>   perform the test setup, set naptime to something lower, reload config.
> But
>   I'm worried that might not be reliable: If something ends up allocating
> an
>   xid we'd potentially reach the path in GetNewTransaction() that wakes up
> the
>   launcher?  But probably there wouldn't be anything doing so?
>
>
  Another aspect that might not make this a good choice is that it actually
>   seems relevant to be able to test cases where there are very old still
>   running transactions...
>
> Maybe this exact scenario can be covered with a separate long-running
test, not included in buildfarm test suite?

-- 
Best regards,
Lubennikova Anastasia


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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
@ 2021-06-11 01:18   ` Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Andres Freund @ 2021-06-11 01:18 UTC (permalink / raw)
  To: Anastasia Lubennikova <[email protected]>; +Cc: pgsql-hackers; Peter Geoghegan <[email protected]>

Hi,

On 2021-06-10 16:42:01 +0300, Anastasia Lubennikova wrote:
> Cool. Thank you for working on that!
> Could you please share a WIP patch for the $subj? I'd be happy to help with
> it.

I've attached the current WIP state, which hasn't evolved much since
this message... I put the test in src/backend/access/heap/t/001_emergency_vacuum.pl
but I'm not sure that's the best place. But I didn't think
src/test/recovery is great either.

Regards,

Andres


Attachments:

  [text/x-perl] 001_emergency_vacuum.pl (3.7K, ../../[email protected]/2-001_emergency_vacuum.pl)
  download | inline:
use strict;
use warnings;
use PostgresNode;
use TestLib;
use Test::More tests => 4;

# Initialize primary node
my $node_primary = get_new_node('primary');

$node_primary->init(allows_streaming => 1);
$node_primary->append_conf('postgresql.conf', qq/
max_prepared_transactions=10
autovacuum_naptime = 1s
# So it's easier to verify the order of operations
autovacuum_max_workers=1
autovacuum_vacuum_cost_delay=0
log_autovacuum_min_duration=0
/);
$node_primary->start;

#
# Create tables for a few different test scenarios
#

$node_primary->safe_psql('postgres', qq/
	CREATE TABLE large(id serial primary key, data text, filler text default repeat(random()::text, 10));
    INSERT INTO large(data) SELECT generate_series(1,30000);

	CREATE TABLE large_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
    INSERT INTO large_trunc(data) SELECT generate_series(1,30000);

	CREATE TABLE small(id serial primary key, data text, filler text default repeat(random()::text, 10));
    INSERT INTO small(data) SELECT generate_series(1,15000);

	CREATE TABLE small_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
    INSERT INTO small_trunc(data) SELECT generate_series(1,15000);

    CREATE TABLE autovacuum_disabled(id serial primary key, data text) WITH (autovacuum_enabled=false);
    INSERT INTO autovacuum_disabled(data) SELECT generate_series(1,1000);
/);


# To prevent autovacuum from handling the tables immediately after
# restart, acquire locks in a 2PC transaction. That allows us to test
# interactions with running commands.

$node_primary->safe_psql('postgres', qq(
    BEGIN;
    LOCK TABLE large IN SHARE UPDATE EXCLUSIVE MODE;
    LOCK TABLE large_trunc IN SHARE UPDATE EXCLUSIVE MODE;
    LOCK TABLE small IN SHARE UPDATE EXCLUSIVE MODE;
    LOCK TABLE small_trunc IN SHARE UPDATE EXCLUSIVE MODE;
    LOCK TABLE autovacuum_disabled IN SHARE UPDATE EXCLUSIVE MODE;
	PREPARE TRANSACTION 'prevent_vacuum';
));


# Delete a few rows to ensure that vacuum has work to do.
$node_primary->safe_psql('postgres', qq/
    DELETE FROM large WHERE id % 2 = 0;
    DELETE FROM large_trunc WHERE id > 10000;
    DELETE FROM small WHERE id % 2 = 0;
    DELETE FROM small_trunc WHERE id > 1000;
    DELETE FROM autovacuum_disabled WHERE id % 2 = 0;
/);


$node_primary->stop;

$node_primary->append_conf('postgresql.conf', qq/
wal_debug=0
log_min_messages=debug2
/);

# Need to reset to a clog page boundary, otherwise we'll get errors
# about the file not existing. With default compilation settings
# CLOG_XACTS_PER_PAGE is 32768. The value below is 32768 *
# (2000000000/32768 + 1), with 2000000000 being the max value for
# autovacuum_freeze_max_age.

command_like([ 'pg_resetwal', '-x2000027648', $node_primary->data_dir ],
	qr/Write-ahead log reset/, 'pg_resetwal -x to');

#
# Now test autovacuum behaviour. Because of the 2PC transaction
# acquiring locks we can perform setup without autovacuum racing
# ahead.
#
$node_primary->start;

diag($node_primary->safe_psql('postgres',
							   qq/SELECT datname, datfrozenxid, age(datfrozenxid) FROM pg_database ORDER BY datname/));

$node_primary->safe_psql('postgres', qq/
    COMMIT PREPARED 'prevent_vacuum';
/);

ok($node_primary->poll_query_until('postgres', qq/
    SELECT NOT EXISTS (
        SELECT *
        FROM pg_database
        WHERE age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int)
/),
   "xid horizon increased");

diag($node_primary->safe_psql('postgres',
							   qq/SELECT datname, datfrozenxid, age(datfrozenxid) FROM pg_database ORDER BY datname/));

diag($node_primary->safe_psql('postgres', qq/
    SELECT oid::regclass, relfrozenxid
    FROM pg_class
    WHERE oid = ANY(ARRAY['large'::regclass, 'large_trunc', 'autovacuum_disabled'])
/));

$node_primary->stop;

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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
@ 2022-02-01 02:58     ` Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2022-02-01 02:58 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

On Fri, Jun 11, 2021 at 10:19 AM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2021-06-10 16:42:01 +0300, Anastasia Lubennikova wrote:
> > Cool. Thank you for working on that!
> > Could you please share a WIP patch for the $subj? I'd be happy to help with
> > it.
>
> I've attached the current WIP state, which hasn't evolved much since
> this message... I put the test in src/backend/access/heap/t/001_emergency_vacuum.pl
> but I'm not sure that's the best place. But I didn't think
> src/test/recovery is great either.
>

Thank you for sharing the WIP patch.

Regarding point (1) you mentioned (StartupSUBTRANS() takes a long time
for zeroing out all pages), how about using single-user mode instead
of preparing the transaction? That is, after pg_resetwal we check the
ages of datfrozenxid by executing a query in single-user mode. That
way, we don’t need to worry about autovacuum concurrently running
while checking the ages of frozenxids. I’ve attached a PoC patch that
does the scenario like:

1. start cluster with autovacuum=off and create tables with a few data
and make garbage on them
2. stop cluster and do pg_resetwal
3. start cluster in single-user mode
4. check age(datfrozenxid)
5. stop cluster
6. start cluster and wait for autovacuums to increase template0,
template1, and postgres datfrozenxids

I put new tests in src/test/module/heap since we already have tests
for brin in src/test/module/brin.

I think that tap test facility to run queries in single-user mode will
also be helpful for testing a new vacuum option/command that is
intended to use in emergency cases and proposed here[1].

Regards,

[1]  https://www.postgresql.org/message-id/flat/20220128012842.GZ23027%40telsasoft.com#b76c13554f90d1c8bb...


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


Attachments:

  [application/x-patch] tap_tests_for_wraparound_emregency.patch (7.2K, ../../CAD21AoC-xMv1pModWvUQ1g1qByxMmu6yLghsKVb-R35rmShiwA@mail.gmail.com/2-tap_tests_for_wraparound_emregency.patch)
  download | inline diff:
diff --git a/src/test/modules/heap/.gitignore b/src/test/modules/heap/.gitignore
new file mode 100644
index 0000000000..716e17f5a2
--- /dev/null
+++ b/src/test/modules/heap/.gitignore
@@ -0,0 +1,2 @@
+# Generated subdirectories
+/tmp_check/
diff --git a/src/test/modules/heap/Makefile b/src/test/modules/heap/Makefile
new file mode 100644
index 0000000000..d3c08a04b7
--- /dev/null
+++ b/src/test/modules/heap/Makefile
@@ -0,0 +1,14 @@
+# src/test/modules/heap/Makefile
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/heap
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/heap/t/001_emergency_vacuum.pl b/src/test/modules/heap/t/001_emergency_vacuum.pl
new file mode 100644
index 0000000000..3229f99921
--- /dev/null
+++ b/src/test/modules/heap/t/001_emergency_vacuum.pl
@@ -0,0 +1,131 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# Test for wraparound emergency situation
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 8;
+use IPC::Run qw(pump finish timer);
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf('postgresql.conf', qq/
+autovacuum = off # run autovacuum only when to anti wraparound
+max_prepared_transactions=10
+autovacuum_naptime = 1s
+# So it's easier to verify the order of operations
+autovacuum_max_workers=1
+autovacuum_vacuum_cost_delay=0
+log_autovacuum_min_duration=0
+/);
+$node_primary->start;
+
+#
+# Create tables for a few different test scenarios
+#
+
+$node_primary->safe_psql('postgres', qq/
+CREATE TABLE large(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large(data) SELECT generate_series(1,30000);
+
+CREATE TABLE large_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large_trunc(data) SELECT generate_series(1,30000);
+
+CREATE TABLE small(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small(data) SELECT generate_series(1,15000);
+
+CREATE TABLE small_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small_trunc(data) SELECT generate_series(1,15000);
+
+CREATE TABLE autovacuum_disabled(id serial primary key, data text) WITH (autovacuum_enabled=false);
+INSERT INTO autovacuum_disabled(data) SELECT generate_series(1,1000);
+/);
+
+# Delete a few rows to ensure that vacuum has work to do.
+$node_primary->safe_psql('postgres', qq/
+DELETE FROM large WHERE id % 2 = 0;
+DELETE FROM large_trunc WHERE id > 10000;
+DELETE FROM small WHERE id % 2 = 0;
+DELETE FROM small_trunc WHERE id > 1000;
+DELETE FROM autovacuum_disabled WHERE id % 2 = 0;
+/);
+
+
+# Stop the server and temporarily disable log_statement while running in single-user mode
+$node_primary->stop;
+$node_primary->append_conf('postgresql.conf', qq/
+log_statement = 'none'
+/);
+
+# Need to reset to a clog page boundary, otherwise we'll get errors
+# about the file not existing. With default compilation settings
+# CLOG_XACTS_PER_PAGE is 32768. The value below is 32768 *
+# (2000000000/32768 + 1), with 2000000000 being the max value for
+# autovacuum_freeze_max_age.
+
+command_like([ 'pg_resetwal', '-x2000027648', $node_primary->data_dir ],
+	     qr/Write-ahead log reset/, 'pg_resetwal -x to');
+
+my $in  = '';
+my $out = '';
+my $timer = timer(5);
+
+# Start the server in single-user mode.  That allows us to test interactions
+# without autovacuums.
+my $h = $node_primary->start_single_user_mode('postgres', \$in, \$out, $timer);
+
+$out = "";
+# Must be a single line with a new line at the end.
+$in .=
+    "SELECT datname, " .
+    "age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int as old ".
+    "FROM pg_database ORDER BY 1;\n";
+
+# Pump until we got the result.
+pump $h until ($out != "" || $timer->is_expired);
+
+# Check all database are old enough.
+like($out, qr/1: datname = "postgres"[^\r\n]+\r\n\t 2: old = "t"/,
+     "postgres database is old enough");
+like($out, qr/1: datname = "template0"[^\r\n]+\r\n\t 2: old = "t"/,
+     "template0 database is old enough");
+like($out, qr/1: datname = "template1"[^\r\n]+\r\n\t 2: old = "t"/,
+     "template1 database is old enough");
+
+# Terminate single user mode.
+$in .= "\cD";
+finish $h or die "postgres --single returned $?";
+
+# Revert back the logging setting.
+$node_primary->append_conf('postgresql.conf', qq/
+log_statement = 'all'
+/);
+
+# Now test autovacuum behaviour.
+$node_primary->start;
+
+ok($node_primary->poll_query_until('postgres', qq/
+    SELECT NOT EXISTS (
+        SELECT *
+        FROM pg_database
+        WHERE age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int)
+/),
+   "xid horizon increased");
+
+my $ret = $node_primary->safe_psql('postgres', qq/
+SELECT relname, age(relfrozenxid) > current_setting('autovacuum_freeze_max_age')::int
+FROM pg_class
+WHERE oid = ANY(ARRAY['large'::regclass, 'large_trunc', 'small', 'small_trunc', 'autovacuum_disabled'])
+ORDER BY 1
+/);
+is($ret, "autovacuum_disabled|f
+large|f
+large_trunc|f
+small|f
+small_trunc|f", "all tables are vacuumed");
+
+$node_primary->stop;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 265f3ae657..2d35978bac 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -858,6 +858,40 @@ sub start
 	return 1;
 }
 
+sub start_single_user_mode
+{
+    my ($self, $dbname, $stdin, $stdout, $timer) = @_;
+    my $name = $self->name;
+
+    BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid};
+
+    print("### Starting node \"$name\" in single-user mode\n");
+
+    local %ENV = $self->_get_env();
+
+    my @postgres_params = (
+	$self->installed_command('postgres'),
+	'--single', '-D', $self->data_dir, 'postgres');
+
+    # Ensure there is no data waiting to be sent:
+    $$stdin = "" if ref($stdin);
+    # IPC::Run would otherwise append to existing contents:
+    $$stdout = "" if ref($stdout);
+
+    my $harness = IPC::Run::start \@postgres_params,
+	'<pty<', $stdin, '>pty>', $stdout, $timer;
+
+    # Pump until we see the startup banner.  This ensures that callers won't
+    # write write anything to the ptr before it's ready, avoiding an
+    # implementation issue in IPC::RUN.
+    pump $harness
+	until $$stdout =~ /PostgreSQL stand-alone backend/ || $timer->is_expired;
+
+    die "postgres --single startup timed out" if $timer->is_expired;
+
+    return $harness;
+}
+
 =pod
 
 =item $node->kill9()
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index a310bcb28c..f94c5ea8cb 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -50,7 +50,8 @@ my @contrib_excludes = (
 	'sepgsql',
 	'brin',             'test_extensions',
 	'test_misc',        'test_pg_dump',
-	'snapshot_too_old', 'unsafe_tests');
+	'snapshot_too_old', 'unsafe_tests',
+	'heap');
 
 # Set of variables for frontend modules
 my $frontend_defines = { 'initdb' => 'FRONTEND' };


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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2022-06-30 01:40       ` Masahiko Sawada <[email protected]>
  2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2022-06-30 01:40 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

Hi,

On Tue, Feb 1, 2022 at 11:58 AM Masahiko Sawada <[email protected]> wrote:
>
> On Fri, Jun 11, 2021 at 10:19 AM Andres Freund <[email protected]> wrote:
> >
> > Hi,
> >
> > On 2021-06-10 16:42:01 +0300, Anastasia Lubennikova wrote:
> > > Cool. Thank you for working on that!
> > > Could you please share a WIP patch for the $subj? I'd be happy to help with
> > > it.
> >
> > I've attached the current WIP state, which hasn't evolved much since
> > this message... I put the test in src/backend/access/heap/t/001_emergency_vacuum.pl
> > but I'm not sure that's the best place. But I didn't think
> > src/test/recovery is great either.
> >
>
> Thank you for sharing the WIP patch.
>
> Regarding point (1) you mentioned (StartupSUBTRANS() takes a long time
> for zeroing out all pages), how about using single-user mode instead
> of preparing the transaction? That is, after pg_resetwal we check the
> ages of datfrozenxid by executing a query in single-user mode. That
> way, we don’t need to worry about autovacuum concurrently running
> while checking the ages of frozenxids. I’ve attached a PoC patch that
> does the scenario like:
>
> 1. start cluster with autovacuum=off and create tables with a few data
> and make garbage on them
> 2. stop cluster and do pg_resetwal
> 3. start cluster in single-user mode
> 4. check age(datfrozenxid)
> 5. stop cluster
> 6. start cluster and wait for autovacuums to increase template0,
> template1, and postgres datfrozenxids

The above steps are wrong.

I think we can expose a function in an extension used only by this
test in order to set nextXid to a future value with zeroing out
clog/subtrans pages. We don't need to fill all clog/subtrans pages
between oldestActiveXID and nextXid. I've attached a PoC patch for
adding this regression test and am going to register it to the next
CF.

BTW, while testing the emergency situation, I found there is a race
condition where anti-wraparound vacuum isn't invoked with the settings
autovacuum = off, autovacuum_max_workers = 1. AN autovacuum worker
sends a signal to the postmaster after advancing datfrozenxid in
SetTransactionIdLimit(). But with the settings, if the autovacuum
launcher attempts to launch a worker before the autovacuum worker who
has signaled to the postmaster finishes, the launcher exits without
launching a worker due to no free workers. The new launcher won’t be
launched until new XID is generated (and only when new XID % 65536 ==
0). Although autovacuum_max_workers = 1 is not mandatory for this
test, it's easier to verify the order of operations.

Regards,

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


Attachments:

  [application/octet-stream] v1-0001-Add-regression-tests-for-emergency-vacuums.patch (10.0K, ../../CAD21AoDVhkXp8HjpFO-gp3TgL6tCKcZQNxn04m01VAtcSi-5sA@mail.gmail.com/2-v1-0001-Add-regression-tests-for-emergency-vacuums.patch)
  download | inline diff:
From 9f686cb3d7edfc5b214c2eddbc20f0ccd6bcda7f Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Mon, 27 Jun 2022 16:44:41 +0900
Subject: [PATCH v1 1/2] Add regression tests for emergency vacuums.

---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/heap/.gitignore              |   4 +
 src/test/modules/heap/Makefile                |  20 +++
 .../modules/heap/t/001_emergency_vacuum.pl    | 116 ++++++++++++++++++
 src/test/modules/heap/test_heap--1.0.sql      |   9 ++
 src/test/modules/heap/test_heap.c             |  72 +++++++++++
 src/test/modules/heap/test_heap.control       |   4 +
 src/tools/msvc/Mkvcbuild.pm                   |   2 +-
 8 files changed, 227 insertions(+), 1 deletion(-)
 create mode 100644 src/test/modules/heap/.gitignore
 create mode 100644 src/test/modules/heap/Makefile
 create mode 100644 src/test/modules/heap/t/001_emergency_vacuum.pl
 create mode 100644 src/test/modules/heap/test_heap--1.0.sql
 create mode 100644 src/test/modules/heap/test_heap.c
 create mode 100644 src/test/modules/heap/test_heap.control

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 9090226daa..3d53edc1d2 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -10,6 +10,7 @@ SUBDIRS = \
 		  delay_execution \
 		  dummy_index_am \
 		  dummy_seclabel \
+		  heap \
 		  libpq_pipeline \
 		  plsample \
 		  snapshot_too_old \
diff --git a/src/test/modules/heap/.gitignore b/src/test/modules/heap/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/heap/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/heap/Makefile b/src/test/modules/heap/Makefile
new file mode 100644
index 0000000000..aeae3f938e
--- /dev/null
+++ b/src/test/modules/heap/Makefile
@@ -0,0 +1,20 @@
+# src/test/modules/heap/Makefile
+
+MODULES = test_heap
+PGFILEDESC = "test_heap - regression test for heap"
+
+EXTENSION = test_heap
+DATA = test_heap--1.0.sql
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/heap
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/heap/t/001_emergency_vacuum.pl b/src/test/modules/heap/t/001_emergency_vacuum.pl
new file mode 100644
index 0000000000..791b97f217
--- /dev/null
+++ b/src/test/modules/heap/t/001_emergency_vacuum.pl
@@ -0,0 +1,116 @@
+# Copyright (c) 2022, PostgreSQL Global Development Group
+
+# Test for wraparound emergency situation
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize node
+my $node = PostgreSQL::Test::Cluster->new('main');
+
+$node->init;
+$node->append_conf('postgresql.conf', qq[
+autovacuum = off # run autovacuum only when to anti wraparound
+max_prepared_transactions = 10
+autovacuum_naptime = 1s
+# so it's easier to verify the order of operations
+autovacuum_max_workers = 1
+log_autovacuum_min_duration = 0
+]);
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE EXTENSION test_heap');
+
+# Create tables for a few different test scenarios
+$node->safe_psql('postgres', qq[
+CREATE TABLE large(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large(data) SELECT generate_series(1,30000);
+
+CREATE TABLE large_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large_trunc(data) SELECT generate_series(1,30000);
+
+CREATE TABLE small(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small(data) SELECT generate_series(1,15000);
+
+CREATE TABLE small_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small_trunc(data) SELECT generate_series(1,15000);
+
+CREATE TABLE autovacuum_disabled(id serial primary key, data text) WITH (autovacuum_enabled=false);
+INSERT INTO autovacuum_disabled(data) SELECT generate_series(1,1000);
+]);
+
+# To prevent autovacuum from handling the tables immediately after
+# restart, acquire locks in a 2PC transaction. That allows us to test
+# interactions with running commands.
+$node->safe_psql('postgres', qq[
+BEGIN;
+LOCK TABLE large IN SHARE UPDATE EXCLUSIVE MODE;
+LOCK TABLE large_trunc IN SHARE UPDATE EXCLUSIVE MODE;
+LOCK TABLE small IN SHARE UPDATE EXCLUSIVE MODE;
+LOCK TABLE small_trunc IN SHARE UPDATE EXCLUSIVE MODE;
+LOCK TABLE autovacuum_disabled IN SHARE UPDATE EXCLUSIVE MODE;
+PREPARE TRANSACTION 'prevent-vacuum';
+]);
+
+# Delete a few rows to ensure that vacuum has work to do.
+$node->safe_psql('postgres', qq[
+DELETE FROM large WHERE id % 2 = 0;
+DELETE FROM large_trunc WHERE id > 10000;
+DELETE FROM small WHERE id % 2 = 0;
+DELETE FROM small_trunc WHERE id > 1000;
+DELETE FROM autovacuum_disabled WHERE id % 2 = 0;
+]);
+
+# New XID needs to be a clog page boundary, otherwise we'll get errors about
+# the file not exisitng error. With default compilation settings
+# CLOG_XACTS_PER_PAGE is 32768. The value below is 32768 *
+# (2000000000/32768 + 1), with 2000000000 being the max value for
+# autovacuum_freeze_max_age.  Since the prepared transaction keeps holding the
+# lock on tables above, autovacuum won't run
+$node->safe_psql('postgres', qq[SELECT set_next_xid('2000027648'::xid)]);
+
+# Make sure updating the latest completed with the advanced XID.
+$node->safe_psql('postgres', qq[INSERT INTO small(data) SELECT 1]);
+
+# Check if all databases became old now.
+my $ret = $node->safe_psql('postgres',
+			   qq[
+SELECT datname,
+       age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int as old
+FROM pg_database ORDER BY 1
+]);
+is($ret, "postgres|t
+template0|t
+template1|t", "all tables became old");
+
+# Allow autovacuum to start working on these tables.
+$node->safe_psql('postgres', qq[COMMIT PREPARED 'prevent-vacuum']);
+
+$node->poll_query_until('postgres',
+			qq[
+SELECT NOT EXISTS (
+	SELECT *
+	FROM pg_database
+	WHERE age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int)
+]) or die "timeout waiting all database are vacuumed";
+
+# Check if these tables are vacuumed.
+$ret = $node->safe_psql('postgres', qq[
+SELECT relname, age(relfrozenxid) > current_setting('autovacuum_freeze_max_age')::int
+FROM pg_class
+WHERE oid = ANY(ARRAY['large'::regclass, 'large_trunc', 'small', 'small_trunc', 'autovacuum_disabled'])
+ORDER BY 1
+]);
+
+is($ret, "autovacuum_disabled|f
+large|f
+large_trunc|f
+small|f
+small_trunc|f", "all tables are vacuumed");
+
+$node->stop;
+
+done_testing();
diff --git a/src/test/modules/heap/test_heap--1.0.sql b/src/test/modules/heap/test_heap--1.0.sql
new file mode 100644
index 0000000000..b7be733bfe
--- /dev/null
+++ b/src/test/modules/heap/test_heap--1.0.sql
@@ -0,0 +1,9 @@
+/* src/test/modules/heap/test_heap--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION test_heap" to load this file. \quit
+
+CREATE FUNCTION set_next_xid(xid)
+    RETURNS void
+    AS 'MODULE_PATHNAME'
+    LANGUAGE C STRICT VOLATILE;
diff --git a/src/test/modules/heap/test_heap.c b/src/test/modules/heap/test_heap.c
new file mode 100644
index 0000000000..66f73edb76
--- /dev/null
+++ b/src/test/modules/heap/test_heap.c
@@ -0,0 +1,72 @@
+/*----------------------------------------------------------------------
+ * test_heap.c
+ *		Support test functions for the heap
+ *
+ * Copyright (c) 2014-2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/test/modules/heap/test_heap.c
+ *----------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "access/clog.h"
+#include "access/commit_ts.h"
+#include "access/subtrans.h"
+#include "funcapi.h"
+#include "miscadmin.h"
+#include "storage/lwlock.h"
+#include "storage/pmsignal.h"
+#include "utils/builtins.h"
+
+PG_MODULE_MAGIC;
+
+/*
+ * Set the given XID in the current epoch to the next XID
+ */
+PG_FUNCTION_INFO_V1(set_next_xid);
+Datum
+set_next_xid(PG_FUNCTION_ARGS)
+{
+	TransactionId next_xid = PG_GETARG_TRANSACTIONID(0);
+	uint32 epoch;
+
+	if (!TransactionIdIsNormal(next_xid))
+		elog(ERROR, "cannot set invalid transaction id");
+
+	LWLockAcquire(XidGenLock, LW_EXCLUSIVE);
+
+	if (TransactionIdPrecedes(next_xid,
+							  XidFromFullTransactionId(ShmemVariableCache->nextXid)))
+	{
+		LWLockRelease(XidGenLock);
+		elog(ERROR, "cannot set transaction id older than the current transaction id");
+	}
+
+	/*
+	 * If the new XID is past xidVacLimit, start trying to force autovacuum
+	 * cycles.
+	 */
+	if (TransactionIdFollowsOrEquals(next_xid, ShmemVariableCache->xidVacLimit))
+	{
+		/* For safety, we release XidGenLock while sending signal */
+		LWLockRelease(XidGenLock);
+		SendPostmasterSignal(PMSIGNAL_START_AUTOVAC_LAUNCHER);
+		LWLockAcquire(XidGenLock, LW_EXCLUSIVE);
+	}
+
+	/* Construct the new XID in the current epoch */
+	epoch = EpochFromFullTransactionId(ShmemVariableCache->nextXid);
+	ShmemVariableCache->nextXid =
+		FullTransactionIdFromEpochAndXid(epoch, next_xid);
+
+	ExtendCLOG(next_xid);
+	ExtendCommitTs(next_xid);
+	ExtendSUBTRANS(next_xid);
+
+	LWLockRelease(XidGenLock);
+
+	PG_RETURN_VOID();
+}
+
diff --git a/src/test/modules/heap/test_heap.control b/src/test/modules/heap/test_heap.control
new file mode 100644
index 0000000000..7d089bb6d1
--- /dev/null
+++ b/src/test/modules/heap/test_heap.control
@@ -0,0 +1,4 @@
+comment = 'Test code for heap'
+default_version = '1.0'
+module_pathname = '$libdir/test_heap'
+relocatable = true
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index e4feda10fd..022a2fa5f7 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -50,7 +50,7 @@ my @contrib_excludes       = (
 	'sepgsql',         'brin',
 	'test_extensions', 'test_misc',
 	'test_pg_dump',    'snapshot_too_old',
-	'unsafe_tests');
+	'unsafe_tests',     'heap');
 
 # Set of variables for frontend modules
 my $frontend_defines = { 'initdb' => 'FRONTEND' };
-- 
2.24.3 (Apple Git-128)



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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2022-11-16 04:38         ` Ian Lawrence Barwick <[email protected]>
  2023-03-03 11:34           ` Re: Testing autovacuum wraparound (including failsafe) Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 26+ messages in thread

From: Ian Lawrence Barwick @ 2022-11-16 04:38 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; +Cc: Andres Freund <[email protected]>; Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

2022年6月30日(木) 10:40 Masahiko Sawada <[email protected]>:
>
> Hi,
>
> On Tue, Feb 1, 2022 at 11:58 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Fri, Jun 11, 2021 at 10:19 AM Andres Freund <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On 2021-06-10 16:42:01 +0300, Anastasia Lubennikova wrote:
> > > > Cool. Thank you for working on that!
> > > > Could you please share a WIP patch for the $subj? I'd be happy to help with
> > > > it.
> > >
> > > I've attached the current WIP state, which hasn't evolved much since
> > > this message... I put the test in src/backend/access/heap/t/001_emergency_vacuum.pl
> > > but I'm not sure that's the best place. But I didn't think
> > > src/test/recovery is great either.
> > >
> >
> > Thank you for sharing the WIP patch.
> >
> > Regarding point (1) you mentioned (StartupSUBTRANS() takes a long time
> > for zeroing out all pages), how about using single-user mode instead
> > of preparing the transaction? That is, after pg_resetwal we check the
> > ages of datfrozenxid by executing a query in single-user mode. That
> > way, we don’t need to worry about autovacuum concurrently running
> > while checking the ages of frozenxids. I’ve attached a PoC patch that
> > does the scenario like:
> >
> > 1. start cluster with autovacuum=off and create tables with a few data
> > and make garbage on them
> > 2. stop cluster and do pg_resetwal
> > 3. start cluster in single-user mode
> > 4. check age(datfrozenxid)
> > 5. stop cluster
> > 6. start cluster and wait for autovacuums to increase template0,
> > template1, and postgres datfrozenxids
>
> The above steps are wrong.
>
> I think we can expose a function in an extension used only by this
> test in order to set nextXid to a future value with zeroing out
> clog/subtrans pages. We don't need to fill all clog/subtrans pages
> between oldestActiveXID and nextXid. I've attached a PoC patch for
> adding this regression test and am going to register it to the next
> CF.
>
> BTW, while testing the emergency situation, I found there is a race
> condition where anti-wraparound vacuum isn't invoked with the settings
> autovacuum = off, autovacuum_max_workers = 1. AN autovacuum worker
> sends a signal to the postmaster after advancing datfrozenxid in
> SetTransactionIdLimit(). But with the settings, if the autovacuum
> launcher attempts to launch a worker before the autovacuum worker who
> has signaled to the postmaster finishes, the launcher exits without
> launching a worker due to no free workers. The new launcher won’t be
> launched until new XID is generated (and only when new XID % 65536 ==
> 0). Although autovacuum_max_workers = 1 is not mandatory for this
> test, it's easier to verify the order of operations.

Hi

Thanks for the patch. While reviewing the patch backlog, we have determined that
the latest version of this patch was submitted before meson support was
implemented, so it should have a "meson.build" file added for consideration for
inclusion in PostgreSQL 16.

Regards

Ian Barwick





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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
@ 2023-03-03 11:34           ` Heikki Linnakangas <[email protected]>
  2023-03-08 04:52             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2023-03-08 05:21             ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
  0 siblings, 2 replies; 26+ messages in thread

From: Heikki Linnakangas @ 2023-03-03 11:34 UTC (permalink / raw)
  To: Ian Lawrence Barwick <[email protected]>; Masahiko Sawada <[email protected]>; +Cc: Andres Freund <[email protected]>; Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

On 16/11/2022 06:38, Ian Lawrence Barwick wrote:
> Thanks for the patch. While reviewing the patch backlog, we have determined that
> the latest version of this patch was submitted before meson support was
> implemented, so it should have a "meson.build" file added for consideration for
> inclusion in PostgreSQL 16.

I wanted to do some XID wraparound testing again, to test the 64-bit 
SLRUs patches [1], and revived this.

I took a different approach to consuming the XIDs. Instead of setting 
nextXID directly, bypassing GetNewTransactionId(), this patch introduces 
a helper function to call GetNewTransactionId() repeatedly. But because 
that's slow, it does include a shortcut to skip over "uninteresting" 
XIDs. Whenever nextXid is close to an SLRU page boundary or XID 
wraparound, it calls GetNewTransactionId(), and otherwise it bumps up 
nextXid close to the next "interesting" value. That's still a lot slower 
than just setting nextXid, but exercises the code more realistically.

I've written some variant of this helper function many times over the 
years, for ad hoc testing. I'd love to have it permanently in the git tree.

In addition to Masahiko's test for emergency vacuum, this includes two 
other tests. 002_limits.pl tests the "warn limit" and "stop limit" in 
GetNewTransactionId(), and 003_wraparound.pl burns through 10 billion 
transactions in total, exercising XID wraparound in general. 
Unfortunately these tests are pretty slow; the tests run for about 4 
minutes on my laptop in total, and use about 20 GB of disk space. So 
perhaps these need to be put in a special test suite that's not run as 
part of "check-world". Or perhaps leave out the 003_wraparounds.pl test, 
that's the slowest of the tests. But I'd love to have these in the git 
tree in some form.

[1] 
https://www.postgresql.org/message-id/[email protected]...)

- Heikki







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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
  2023-03-03 11:34           ` Re: Testing autovacuum wraparound (including failsafe) Heikki Linnakangas <[email protected]>
@ 2023-03-08 04:52             ` Masahiko Sawada <[email protected]>
  2023-03-14 06:01               ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  1 sibling, 1 reply; 26+ messages in thread

From: Masahiko Sawada @ 2023-03-08 04:52 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: Ian Lawrence Barwick <[email protected]>; Andres Freund <[email protected]>; Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

On Fri, Mar 3, 2023 at 8:34 PM Heikki Linnakangas <[email protected]> wrote:
>
> On 16/11/2022 06:38, Ian Lawrence Barwick wrote:
> > Thanks for the patch. While reviewing the patch backlog, we have determined that
> > the latest version of this patch was submitted before meson support was
> > implemented, so it should have a "meson.build" file added for consideration for
> > inclusion in PostgreSQL 16.
>
> I wanted to do some XID wraparound testing again, to test the 64-bit
> SLRUs patches [1], and revived this.

Thank you for reviving this thread!

>
> I took a different approach to consuming the XIDs. Instead of setting
> nextXID directly, bypassing GetNewTransactionId(), this patch introduces
> a helper function to call GetNewTransactionId() repeatedly. But because
> that's slow, it does include a shortcut to skip over "uninteresting"
> XIDs. Whenever nextXid is close to an SLRU page boundary or XID
> wraparound, it calls GetNewTransactionId(), and otherwise it bumps up
> nextXid close to the next "interesting" value. That's still a lot slower
> than just setting nextXid, but exercises the code more realistically.
>
> I've written some variant of this helper function many times over the
> years, for ad hoc testing. I'd love to have it permanently in the git tree.

These functions seem to be better than mine.

> In addition to Masahiko's test for emergency vacuum, this includes two
> other tests. 002_limits.pl tests the "warn limit" and "stop limit" in
> GetNewTransactionId(), and 003_wraparound.pl burns through 10 billion
> transactions in total, exercising XID wraparound in general.
> Unfortunately these tests are pretty slow; the tests run for about 4
> minutes on my laptop in total, and use about 20 GB of disk space. So
> perhaps these need to be put in a special test suite that's not run as
> part of "check-world". Or perhaps leave out the 003_wraparounds.pl test,
> that's the slowest of the tests. But I'd love to have these in the git
> tree in some form.

cbfot reports some failures. The main reason seems that meson.build in
xid_wraparound directory adds the regression tests but the .sql and
.out files are missing in the patch. Perhaps the patch wants to add
only tap tests as Makefile doesn't define REGRESS?

Even after fixing this issue, CI tests (Cirrus CI) are not happy and
report failures due to a disk full. The size of xid_wraparound test
directory is 105MB out of 262MB:

% du -sh testrun
262M    testrun
% du -sh testrun/xid_wraparound/
105M    testrun/xid_wraparound/
% du -sh testrun/xid_wraparound/*
460K    testrun/xid_wraparound/001_emergency_vacuum
93M     testrun/xid_wraparound/002_limits
12M     testrun/xid_wraparound/003_wraparounds
% ls -lh testrun/xid_wraparound/002_limits/log*
total 93M
-rw-------. 1 masahiko masahiko 93M Mar  7 17:34 002_limits_wraparound.log
-rw-rw-r--. 1 masahiko masahiko 20K Mar  7 17:34 regress_log_002_limits

The biggest file is the server logs since an autovacuum worker writes
autovacuum logs for every table for every second (autovacuum_naptime
is 1s). Maybe we can set log_autovacuum_min_duration reloption for the
test tables instead of globally enabling it

The 001 test uses the 2PC transaction that holds locks on tables but
since we can consume xids while the server running, we don't need
that. Instead I think we can keep a transaction open in the background
like 002 test does.

I'll try these ideas.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com






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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
  2023-03-03 11:34           ` Re: Testing autovacuum wraparound (including failsafe) Heikki Linnakangas <[email protected]>
  2023-03-08 04:52             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
@ 2023-03-14 06:01               ` Masahiko Sawada <[email protected]>
  0 siblings, 0 replies; 26+ messages in thread

From: Masahiko Sawada @ 2023-03-14 06:01 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: Ian Lawrence Barwick <[email protected]>; Andres Freund <[email protected]>; Anastasia Lubennikova <[email protected]>; pgsql-hackers; Peter Geoghegan <[email protected]>

On Wed, Mar 8, 2023 at 1:52 PM Masahiko Sawada <[email protected]> wrote:
>
> On Fri, Mar 3, 2023 at 8:34 PM Heikki Linnakangas <[email protected]> wrote:
> >
> > On 16/11/2022 06:38, Ian Lawrence Barwick wrote:
> > > Thanks for the patch. While reviewing the patch backlog, we have determined that
> > > the latest version of this patch was submitted before meson support was
> > > implemented, so it should have a "meson.build" file added for consideration for
> > > inclusion in PostgreSQL 16.
> >
> > I wanted to do some XID wraparound testing again, to test the 64-bit
> > SLRUs patches [1], and revived this.
>
> Thank you for reviving this thread!
>
> >
> > I took a different approach to consuming the XIDs. Instead of setting
> > nextXID directly, bypassing GetNewTransactionId(), this patch introduces
> > a helper function to call GetNewTransactionId() repeatedly. But because
> > that's slow, it does include a shortcut to skip over "uninteresting"
> > XIDs. Whenever nextXid is close to an SLRU page boundary or XID
> > wraparound, it calls GetNewTransactionId(), and otherwise it bumps up
> > nextXid close to the next "interesting" value. That's still a lot slower
> > than just setting nextXid, but exercises the code more realistically.
> >
> > I've written some variant of this helper function many times over the
> > years, for ad hoc testing. I'd love to have it permanently in the git tree.
>
> These functions seem to be better than mine.
>
> > In addition to Masahiko's test for emergency vacuum, this includes two
> > other tests. 002_limits.pl tests the "warn limit" and "stop limit" in
> > GetNewTransactionId(), and 003_wraparound.pl burns through 10 billion
> > transactions in total, exercising XID wraparound in general.
> > Unfortunately these tests are pretty slow; the tests run for about 4
> > minutes on my laptop in total, and use about 20 GB of disk space. So
> > perhaps these need to be put in a special test suite that's not run as
> > part of "check-world". Or perhaps leave out the 003_wraparounds.pl test,
> > that's the slowest of the tests. But I'd love to have these in the git
> > tree in some form.
>
> cbfot reports some failures. The main reason seems that meson.build in
> xid_wraparound directory adds the regression tests but the .sql and
> .out files are missing in the patch. Perhaps the patch wants to add
> only tap tests as Makefile doesn't define REGRESS?
>
> Even after fixing this issue, CI tests (Cirrus CI) are not happy and
> report failures due to a disk full. The size of xid_wraparound test
> directory is 105MB out of 262MB:
>
> % du -sh testrun
> 262M    testrun
> % du -sh testrun/xid_wraparound/
> 105M    testrun/xid_wraparound/
> % du -sh testrun/xid_wraparound/*
> 460K    testrun/xid_wraparound/001_emergency_vacuum
> 93M     testrun/xid_wraparound/002_limits
> 12M     testrun/xid_wraparound/003_wraparounds
> % ls -lh testrun/xid_wraparound/002_limits/log*
> total 93M
> -rw-------. 1 masahiko masahiko 93M Mar  7 17:34 002_limits_wraparound.log
> -rw-rw-r--. 1 masahiko masahiko 20K Mar  7 17:34 regress_log_002_limits
>
> The biggest file is the server logs since an autovacuum worker writes
> autovacuum logs for every table for every second (autovacuum_naptime
> is 1s). Maybe we can set log_autovacuum_min_duration reloption for the
> test tables instead of globally enabling it

I think it could be acceptable since 002 and 003 tests are executed
only when required. And 001 test seems to be able to pass on cfbot but
it takes more than 30 sec. In the attached patch, I made these tests
optional and these are enabled if envar ENABLE_XID_WRAPAROUND_TESTS is
defined (supporting only autoconf).

>
> The 001 test uses the 2PC transaction that holds locks on tables but
> since we can consume xids while the server running, we don't need
> that. Instead I think we can keep a transaction open in the background
> like 002 test does.

Updated in the new patch. Also, I added a check if the failsafe mode
is triggered.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/octet-stream] v2-0001-Add-tests-for-XID-wraparound.patch (22.5K, ../../CAD21AoAyYBZOiB1UPCPZJHTLk0-arrq5zqNGj+PrsbpdUy=g-g@mail.gmail.com/2-v2-0001-Add-tests-for-XID-wraparound.patch)
  download | inline diff:
From 851b856feb829c6a1bed041aec7febdc3928fc04 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Fri, 3 Mar 2023 12:01:28 +0200
Subject: [PATCH v2] Add tests for XID wraparound.

The test module includes helper functions to quickly burn through lots
of XIDs. They are used in the tests, and are also handy for manually
testing XID wraparound.

Author: Masahiko Sawada, Heikki Linnakangas
Discussion: https://www.postgresql.org/message-id/CAD21AoDVhkXp8HjpFO-gp3TgL6tCKcZQNxn04m01VAtcSi-5sA%40mail.gmail.com
---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/meson.build                  |   1 +
 src/test/modules/xid_wraparound/.gitignore    |   4 +
 src/test/modules/xid_wraparound/Makefile      |  28 +++
 src/test/modules/xid_wraparound/README        |   3 +
 src/test/modules/xid_wraparound/meson.build   |  43 ++++
 .../xid_wraparound/t/001_emergency_vacuum.pl  | 120 ++++++++++
 .../modules/xid_wraparound/t/002_limits.pl    | 114 +++++++++
 .../xid_wraparound/t/003_wraparounds.pl       |  46 ++++
 .../xid_wraparound/xid_wraparound--1.0.sql    |  12 +
 .../modules/xid_wraparound/xid_wraparound.c   | 221 ++++++++++++++++++
 .../xid_wraparound/xid_wraparound.control     |   4 +
 12 files changed, 597 insertions(+)
 create mode 100644 src/test/modules/xid_wraparound/.gitignore
 create mode 100644 src/test/modules/xid_wraparound/Makefile
 create mode 100644 src/test/modules/xid_wraparound/README
 create mode 100644 src/test/modules/xid_wraparound/meson.build
 create mode 100644 src/test/modules/xid_wraparound/t/001_emergency_vacuum.pl
 create mode 100644 src/test/modules/xid_wraparound/t/002_limits.pl
 create mode 100644 src/test/modules/xid_wraparound/t/003_wraparounds.pl
 create mode 100644 src/test/modules/xid_wraparound/xid_wraparound--1.0.sql
 create mode 100644 src/test/modules/xid_wraparound/xid_wraparound.c
 create mode 100644 src/test/modules/xid_wraparound/xid_wraparound.control

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index c629cbe383..99f5fa23f1 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -14,6 +14,7 @@ SUBDIRS = \
 		  plsample \
 		  snapshot_too_old \
 		  spgist_name_ops \
+		  xid_wraparound \
 		  test_bloomfilter \
 		  test_copy_callbacks \
 		  test_custom_rmgrs \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 1baa6b558d..39de43dee3 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -10,6 +10,7 @@ subdir('plsample')
 subdir('snapshot_too_old')
 subdir('spgist_name_ops')
 subdir('ssl_passphrase_callback')
+subdir('xid_wraparound')
 subdir('test_bloomfilter')
 subdir('test_copy_callbacks')
 subdir('test_custom_rmgrs')
diff --git a/src/test/modules/xid_wraparound/.gitignore b/src/test/modules/xid_wraparound/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/xid_wraparound/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/xid_wraparound/Makefile b/src/test/modules/xid_wraparound/Makefile
new file mode 100644
index 0000000000..fc5ead6cc5
--- /dev/null
+++ b/src/test/modules/xid_wraparound/Makefile
@@ -0,0 +1,28 @@
+# src/test/modules/xid_wraparound/Makefile
+
+MODULE_big = xid_wraparound
+OBJS = \
+	$(WIN32RES) \
+	xid_wraparound.o
+PGFILEDESC = "xid_wraparound - tests for XID wraparound"
+
+EXTENSION = xid_wraparound
+DATA = xid_wraparound--1.0.sql
+
+# Disabled by default because these tests could take a long time,
+# which typical installcheck users cannot tolerate (e.g. buildfarm
+# clients).
+ifdef ENABLE_XID_WRAPAROUND_TESTS
+TAP_TESTS = 1
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/xid_wraparound
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/xid_wraparound/README b/src/test/modules/xid_wraparound/README
new file mode 100644
index 0000000000..3aab464dec
--- /dev/null
+++ b/src/test/modules/xid_wraparound/README
@@ -0,0 +1,3 @@
+This module contains tests for XID wraparound. The tests use two
+helper functions to quickly consume lots of XIDs, to reach XID
+wraparound faster.
diff --git a/src/test/modules/xid_wraparound/meson.build b/src/test/modules/xid_wraparound/meson.build
new file mode 100644
index 0000000000..bdd55f22c4
--- /dev/null
+++ b/src/test/modules/xid_wraparound/meson.build
@@ -0,0 +1,43 @@
+# Copyright (c) 2022-2023, PostgreSQL Global Development Group
+
+# FIXME: prevent install during main install, but not during test :/
+
+xid_wraparound_sources = files(
+  'xid_wraparound.c',
+)
+
+if host_system == 'windows'
+  xid_wraparound_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'xid_wraparound',
+    '--FILEDESC', 'xid_wraparound - tests for XID wraparound',])
+endif
+
+xid_wraparound = shared_module('xid_wraparound',
+  xid_wraparound_sources,
+  kwargs: pg_mod_args,
+)
+testprep_targets += xid_wraparound
+
+install_data(
+  'xid_wraparound.control',
+  'xid_wraparound--1.0.sql',
+  kwargs: contrib_data_args,
+)
+
+# Disabled by default because these test could take a long time,
+# which typical installcheck users cannot tolerate (e.g. buildfarm
+# clients).
+if false
+  tests += {
+    'name': 'xid_wraparound',
+    'sd': meson.current_source_dir(),
+    'bd': meson.current_build_dir(),
+    'tap': {
+      'tests': [
+        't/001_emergency_vacuum.pl',
+        't/002_limits.pl',
+        't/003_wraparounds.pl',
+      ],
+    },
+  }
+endif
diff --git a/src/test/modules/xid_wraparound/t/001_emergency_vacuum.pl b/src/test/modules/xid_wraparound/t/001_emergency_vacuum.pl
new file mode 100644
index 0000000000..bd9bb4b98c
--- /dev/null
+++ b/src/test/modules/xid_wraparound/t/001_emergency_vacuum.pl
@@ -0,0 +1,120 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+# Test wraparound emergency autovacuum.
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize node
+my $node = PostgreSQL::Test::Cluster->new('main');
+
+$node->init;
+$node->append_conf('postgresql.conf', qq[
+autovacuum = off # run autovacuum only when to anti wraparound
+autovacuum_naptime = 1s
+# so it's easier to verify the order of operations
+autovacuum_max_workers = 1
+log_autovacuum_min_duration = 0
+]);
+$node->start;
+$node->safe_psql('postgres', 'CREATE EXTENSION xid_wraparound');
+
+# Create tables for a few different test scenarios
+$node->safe_psql('postgres', qq[
+CREATE TABLE large(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large(data) SELECT generate_series(1,30000);
+
+CREATE TABLE large_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO large_trunc(data) SELECT generate_series(1,30000);
+
+CREATE TABLE small(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small(data) SELECT generate_series(1,15000);
+
+CREATE TABLE small_trunc(id serial primary key, data text, filler text default repeat(random()::text, 10));
+INSERT INTO small_trunc(data) SELECT generate_series(1,15000);
+
+CREATE TABLE autovacuum_disabled(id serial primary key, data text) WITH (autovacuum_enabled=false);
+INSERT INTO autovacuum_disabled(data) SELECT generate_series(1,1000);
+]);
+
+# Start a background session, which holds a transaction open, preventing
+# autovacuum from advancing relfrozenxid and datfrozenxid.
+my $in  = '';
+my $out = '';
+my $timeout = IPC::Run::timer($PostgreSQL::Test::Utils::timeout_default);
+my $background_psql = $node->background_psql('postgres', \$in, \$out, $timeout);
+$in .= q{
+	BEGIN;
+	DELETE FROM large WHERE id % 2 = 0;
+	DELETE FROM large_trunc WHERE id > 10000;
+	DELETE FROM small WHERE id % 2 = 0;
+	DELETE FROM small_trunc WHERE id > 1000;
+	DELETE FROM autovacuum_disabled WHERE id % 2 = 0;
+};
+$background_psql->pump_nb;
+
+# Consume 2 billion XIDs, to get us very close to wraparound
+$node->safe_psql('postgres', qq[SELECT consume_xids_until('2000000000'::bigint)]);
+
+# Make sure the latest completed XID is advanced
+$node->safe_psql('postgres', qq[INSERT INTO small(data) SELECT 1]);
+
+# Check that all databases became old enough to trigger failsafe.
+my $ret = $node->safe_psql('postgres',
+			   qq[
+SELECT datname,
+       age(datfrozenxid) > current_setting('vacuum_failsafe_age')::int as old
+FROM pg_database ORDER BY 1
+]);
+is($ret, "postgres|t
+template0|t
+template1|t", "all tables became old");
+
+my $log_offset = -s $node->logfile;
+
+# Finish the old transaction, to allow vacuum freezing to advance
+# relfrozenxid and datfrozenxid again.
+$in .= q{
+COMMIT;
+\q
+};
+$background_psql->finish;
+
+# Wait until autovacuum processed all tables and advanced the
+# system-wide oldest-XID.
+$node->poll_query_until('postgres',
+			qq[
+SELECT NOT EXISTS (
+	SELECT *
+	FROM pg_database
+	WHERE age(datfrozenxid) > current_setting('autovacuum_freeze_max_age')::int)
+]) or die "timeout waiting for all databases to be vacuumed";
+
+# Check if these tables are vacuumed.
+$ret = $node->safe_psql('postgres', qq[
+SELECT relname, age(relfrozenxid) > current_setting('autovacuum_freeze_max_age')::int
+FROM pg_class
+WHERE relname IN ('large', 'large_trunc', 'small', 'small_trunc', 'autovacuum_disabled')
+ORDER BY 1
+]);
+
+is($ret, "autovacuum_disabled|f
+large|f
+large_trunc|f
+small|f
+small_trunc|f", "all tables are vacuumed");
+
+# Check if vacuum failsafe was triggered for each table.
+my $log_contents = slurp_file($node->logfile, $log_offset);
+foreach my $tablename ('large', 'large_trunc', 'small', 'small_trunc', 'autovacuum_disabled')
+{
+    like(
+	$log_contents,
+	qr/bypassing nonessential maintenance of table "postgres.public.$tablename" as a failsafe after \d+ index scans/,
+	"failsafe vacuum triggered for $tablename");
+}
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/xid_wraparound/t/002_limits.pl b/src/test/modules/xid_wraparound/t/002_limits.pl
new file mode 100644
index 0000000000..ff2743746b
--- /dev/null
+++ b/src/test/modules/xid_wraparound/t/002_limits.pl
@@ -0,0 +1,114 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+#
+# Test XID wraparound limits.
+#
+# When you get close to XID wraparound, you start to get warnings, and
+# when you get even closer, the system refuses to assign any more XIDs
+# until the oldest databases have been vacuumed and datfrozenxid has
+# been advanced.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use Time::HiRes qw(usleep);
+
+my $ret;
+
+# Initialize node
+my $node = PostgreSQL::Test::Cluster->new('wraparound');
+
+$node->init;
+$node->append_conf('postgresql.conf', qq[
+autovacuum = off # run autovacuum only to prevent wraparound
+autovacuum_naptime = 1s
+log_autovacuum_min_duration = 0
+]);
+$node->start;
+$node->safe_psql('postgres', 'CREATE EXTENSION xid_wraparound');
+
+# Create a test table
+$node->safe_psql('postgres', qq[
+CREATE TABLE wraparoundtest(t text);
+INSERT INTO wraparoundtest VALUES ('start');
+]);
+
+# Start a background session, which holds a transaction open, preventing
+# autovacuum from advancing relfrozenxid and datfrozenxid.
+my $in  = '';
+my $out = '';
+my $timeout = IPC::Run::timer($PostgreSQL::Test::Utils::timeout_default);
+my $background_psql = $node->background_psql('postgres', \$in, \$out, $timeout);
+$in .= q{
+	BEGIN;
+	INSERT INTO wraparoundtest VALUES ('oldxact');
+};
+$background_psql->pump_nb;
+
+# Consume 2 billion transactions, to get close to wraparound
+$node->safe_psql('postgres', qq[SELECT consume_xids(1000000000)]);
+$node->safe_psql('postgres', qq[INSERT INTO wraparoundtest VALUES ('after 1 billion')]);
+
+$node->safe_psql('postgres', qq[SELECT consume_xids(1000000000)]);
+$node->safe_psql('postgres', qq[INSERT INTO wraparoundtest VALUES ('after 2 billion')]);
+
+# We are now just under 150 million XIDs away from wraparound.
+# Continue consuming XIDs, in batches of 10 million, until we get
+# the warning:
+#
+#  WARNING:  database "postgres" must be vacuumed within 3000024 transactions
+#  HINT:  To avoid a database shutdown, execute a database-wide VACUUM in that database.
+#  You might also need to commit or roll back old prepared transactions, or drop stale replication slots.
+my $stderr;
+my $warn_limit = 0;
+for my $i (1 .. 15)
+{
+	$node->psql('postgres', qq[SELECT consume_xids(10000000)], stderr => \$stderr, on_error_die => 1);
+
+	if ($stderr =~ /WARNING:  database "postgres" must be vacuumed within [0-9]+ transactions/)
+	{
+		# Reached the warn-limit
+		$warn_limit = 1;
+		last;
+	}
+}
+ok($warn_limit == 1, "warn-limit reached");
+
+# We can still INSERT, despite the warnings.
+$node->safe_psql('postgres', qq[INSERT INTO wraparoundtest VALUES ('reached warn-limit')]);
+
+# Keep going. We'll hit the hard "stop" limit.
+$ret = $node->psql('postgres', qq[SELECT consume_xids(100000000)], stderr => \$stderr);
+like($stderr, qr/ERROR:  database is not accepting commands to avoid wraparound data loss/, "stop-limit");
+
+# Finish the old transaction, to allow vacuum freezing to advance
+# relfrozenxid and datfrozenxid again.
+$in .= q{
+COMMIT;
+\q
+};
+$background_psql->finish;
+
+# VACUUM, to freeze the tables and advance datfrozenxid.
+#
+# Autovacuum does this for the other databases, and would do it for
+# 'postgres' too, but let's test manual VACUUM.
+#
+$node->safe_psql('postgres', 'VACUUM');
+
+# Wait until autovacuum has processed the other databases and advanced
+# the system-wide oldest-XID.
+$ret = $node->poll_query_until('postgres', qq[INSERT INTO wraparoundtest VALUES ('after VACUUM')], 'INSERT 0 1');
+
+# Check the table contents
+$ret = $node->safe_psql('postgres', qq[SELECT * from wraparoundtest]);
+is($ret, "start
+oldxact
+after 1 billion
+after 2 billion
+reached warn-limit
+after VACUUM");
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/xid_wraparound/t/003_wraparounds.pl b/src/test/modules/xid_wraparound/t/003_wraparounds.pl
new file mode 100644
index 0000000000..e79adbd12d
--- /dev/null
+++ b/src/test/modules/xid_wraparound/t/003_wraparounds.pl
@@ -0,0 +1,46 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+#
+# Consume a lot of XIDs, wrapping around a few times.
+#
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use Time::HiRes qw(usleep);
+
+# Initialize node
+my $node = PostgreSQL::Test::Cluster->new('wraparound');
+
+$node->init;
+$node->append_conf('postgresql.conf', qq[
+autovacuum = off # run autovacuum only when to anti wraparound
+autovacuum_naptime = 1s
+# so it's easier to verify the order of operations
+autovacuum_max_workers = 1
+log_autovacuum_min_duration = 0
+]);
+$node->start;
+$node->safe_psql('postgres', 'CREATE EXTENSION xid_wraparound');
+
+# Create a test table
+$node->safe_psql('postgres', qq[
+CREATE TABLE wraparoundtest(t text);
+INSERT INTO wraparoundtest VALUES ('beginning');
+]);
+
+# Burn through 10 billion transactions in total, in batches of 100 million.
+my $ret;
+for my $i (1 .. 100)
+{
+	$ret = $node->safe_psql('postgres', qq[SELECT consume_xids(100000000)]);
+	$ret = $node->safe_psql('postgres', qq[INSERT INTO wraparoundtest VALUES ('after $i batches')]);
+}
+
+$ret = $node->safe_psql('postgres', qq[SELECT COUNT(*) FROM wraparoundtest]);
+is($ret, "101");
+
+$node->stop;
+
+done_testing();
diff --git a/src/test/modules/xid_wraparound/xid_wraparound--1.0.sql b/src/test/modules/xid_wraparound/xid_wraparound--1.0.sql
new file mode 100644
index 0000000000..f5577adfdb
--- /dev/null
+++ b/src/test/modules/xid_wraparound/xid_wraparound--1.0.sql
@@ -0,0 +1,12 @@
+/* src/test/modules/xid_wraparound/xid_wraparound--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION xid_wraparound" to load this file. \quit
+
+CREATE FUNCTION consume_xids(bigint)
+RETURNS bigint IMMUTABLE PARALLEL SAFE STRICT
+AS 'MODULE_PATHNAME' LANGUAGE C;
+
+CREATE FUNCTION consume_xids_until(bigint)
+RETURNS bigint IMMUTABLE PARALLEL SAFE STRICT
+AS 'MODULE_PATHNAME' LANGUAGE C;
diff --git a/src/test/modules/xid_wraparound/xid_wraparound.c b/src/test/modules/xid_wraparound/xid_wraparound.c
new file mode 100644
index 0000000000..c9d6034b55
--- /dev/null
+++ b/src/test/modules/xid_wraparound/xid_wraparound.c
@@ -0,0 +1,221 @@
+/*--------------------------------------------------------------------------
+ *
+ * xid_wraparound.c
+ *		Utilities for testing XID wraparound
+ *
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *		src/test/modules/xid_wraparound/xid_wraparound.c
+ *
+ * -------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/transam.h"
+#include "access/xact.h"
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "storage/lwlock.h"
+#include "storage/proc.h"
+
+PG_MODULE_MAGIC;
+
+static int64 consume_xids_shortcut(void);
+static FullTransactionId consume_xids_common(FullTransactionId untilxid, uint64 nxids);
+
+/*
+ * Consume the specified number of XIDs.
+ */
+PG_FUNCTION_INFO_V1(consume_xids);
+Datum
+consume_xids(PG_FUNCTION_ARGS)
+{
+	int64		nxids = PG_GETARG_INT64(0);
+	FullTransactionId lastxid;
+
+	if (nxids < 0)
+		elog(ERROR, "invalid nxids argument: %lld", (long long) nxids);
+
+	if (nxids == 0)
+		lastxid = ReadNextFullTransactionId();
+	else
+		lastxid = consume_xids_common(InvalidFullTransactionId, (uint64) nxids);
+
+	PG_RETURN_INT64((int64) U64FromFullTransactionId(lastxid));
+}
+
+/*
+ * Consume XIDs, up to the given XID.
+ */
+PG_FUNCTION_INFO_V1(consume_xids_until);
+Datum
+consume_xids_until(PG_FUNCTION_ARGS)
+{
+	FullTransactionId targetxid = FullTransactionIdFromU64((uint64) PG_GETARG_INT64(0));
+	FullTransactionId lastxid;
+
+	if (!FullTransactionIdIsNormal(targetxid))
+		elog(ERROR, "targetxid %llu is not normal", (unsigned long long) U64FromFullTransactionId(targetxid));
+
+	lastxid = consume_xids_common(targetxid, 0);
+
+	PG_RETURN_INT64((int64) U64FromFullTransactionId(lastxid));
+}
+
+/*
+ * Common functionality between the two public functions.
+ */
+static FullTransactionId
+consume_xids_common(FullTransactionId untilxid, uint64 nxids)
+{
+	FullTransactionId lastxid;
+	uint64		last_reported_at = 0;
+	uint64		consumed = 0;
+
+	/* Print a NOTICE every REPORT_INTERVAL xids */
+#define REPORT_INTERVAL (10*1000000)
+
+	/* initialize 'lastxid' with the system's current next XID */
+	lastxid = ReadNextFullTransactionId();
+
+	/*
+	 * We consume XIDs by calling GetNewTransactionId(true), which marks the
+	 * consumed XIDs as subtransactions of the current top-level transaction.
+	 * For that to work, this transaction must have a top-level XID.
+	 *
+	 * GetNewTransactionId registers them in the subxid cache in PGPROC, until
+	 * the cache overflows, but beyond that, we don't keep track of the
+	 * consumed XIDs.
+	 */
+	(void) GetTopTransactionId();
+
+	for (;;)
+	{
+		uint64		xids_left;
+
+		CHECK_FOR_INTERRUPTS();
+
+		/* How many XIDs do we have left to consume? */
+		if (nxids > 0)
+		{
+			if (consumed >= nxids)
+				break;
+			xids_left = nxids - consumed;
+		}
+		else
+		{
+			if (FullTransactionIdFollowsOrEquals(lastxid, untilxid))
+				break;
+			xids_left = U64FromFullTransactionId(untilxid) - U64FromFullTransactionId(lastxid);
+		}
+
+		/*
+		 * If we still have plenty of XIDs to consume, try to take a shortcut
+		 * and bump up the nextXid counter directly.
+		 */
+		if (xids_left > 2000 &&
+			consumed - last_reported_at < REPORT_INTERVAL &&
+			MyProc->subxidStatus.overflowed)
+		{
+			int64		consumed_by_shortcut = consume_xids_shortcut();
+
+			if (consumed_by_shortcut > 0)
+			{
+				consumed += consumed_by_shortcut;
+				continue;
+			}
+		}
+
+		/* Slow path: Call GetNewTransactionId to allocate a new XID. */
+		lastxid = GetNewTransactionId(true);
+		consumed++;
+
+		/* Report progress */
+		if (consumed - last_reported_at >= REPORT_INTERVAL)
+		{
+			if (nxids > 0)
+				elog(NOTICE, "consumed %llu / %llu XIDs, latest %u:%u",
+					 (unsigned long long) consumed, (unsigned long long) nxids,
+					 EpochFromFullTransactionId(lastxid),
+					 XidFromFullTransactionId(lastxid));
+			else
+				elog(NOTICE, "consumed up to %u:%u / %u:%u",
+					 EpochFromFullTransactionId(lastxid),
+					 XidFromFullTransactionId(lastxid),
+					 EpochFromFullTransactionId(untilxid),
+					 XidFromFullTransactionId(untilxid));
+			last_reported_at = consumed;
+		}
+	}
+
+	return lastxid;
+}
+
+
+/*
+ * These constants copied from .c files, because they're private.
+ */
+#define COMMIT_TS_XACTS_PER_PAGE (BLCKSZ / 10)
+#define SUBTRANS_XACTS_PER_PAGE (BLCKSZ / sizeof(TransactionId))
+#define CLOG_XACTS_PER_BYTE 4
+#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE)
+
+/*
+ * All the interesting action in GetNewTransactionId happens when we extend
+ * the SLRUs, or at the uint32 wraparound. If the nextXid counter is not close
+ * to any of those interesting values, take a shortcut and bump nextXID
+ * directly, close to the next "interesting" value.
+ */
+static inline uint32
+XidSkip(FullTransactionId fullxid)
+{
+	uint32		low = XidFromFullTransactionId(fullxid);
+	uint32		rem;
+	uint32		distance;
+
+	if (low < 5 || low >= UINT32_MAX - 5)
+		return 0;
+	distance = UINT32_MAX - 5 - low;
+
+	rem = low % COMMIT_TS_XACTS_PER_PAGE;
+	if (rem == 0)
+		return 0;
+	distance = Min(distance, COMMIT_TS_XACTS_PER_PAGE - rem);
+
+	rem = low % SUBTRANS_XACTS_PER_PAGE;
+	if (rem == 0)
+		return 0;
+	distance = Min(distance, SUBTRANS_XACTS_PER_PAGE - rem);
+
+	rem = low % CLOG_XACTS_PER_PAGE;
+	if (rem == 0)
+		return 0;
+	distance = Min(distance, CLOG_XACTS_PER_PAGE - rem);
+
+	return distance;
+}
+
+static int64
+consume_xids_shortcut(void)
+{
+	FullTransactionId nextXid;
+	uint32		consumed;
+
+	LWLockAcquire(XidGenLock, LW_EXCLUSIVE);
+	nextXid = ShmemVariableCache->nextXid;
+
+	/*
+	 * Go slow near the "interesting values". The interesting zones include 5
+	 * transactions before and after SLRU page switches.
+	 */
+	consumed = XidSkip(nextXid);
+	if (consumed > 0)
+		ShmemVariableCache->nextXid.value += (uint64) consumed;
+
+	LWLockRelease(XidGenLock);
+
+	return consumed;
+}
diff --git a/src/test/modules/xid_wraparound/xid_wraparound.control b/src/test/modules/xid_wraparound/xid_wraparound.control
new file mode 100644
index 0000000000..6c6964ed3d
--- /dev/null
+++ b/src/test/modules/xid_wraparound/xid_wraparound.control
@@ -0,0 +1,4 @@
+comment = 'Tests for XID wraparound'
+default_version = '1.0'
+module_pathname = '$libdir/xid_wraparound'
+relocatable = true
-- 
2.31.1



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

* Re: Testing autovacuum wraparound (including failsafe)
  2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
  2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
  2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
  2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
  2023-03-03 11:34           ` Re: Testing autovacuum wraparound (including failsafe) Heikki Linnakangas <[email protected]>
@ 2023-03-08 05:21             ` Peter Geoghegan <[email protected]>
  1 sibling, 0 replies; 26+ messages in thread

From: Peter Geoghegan @ 2023-03-08 05:21 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: Ian Lawrence Barwick <[email protected]>; Masahiko Sawada <[email protected]>; Andres Freund <[email protected]>; Anastasia Lubennikova <[email protected]>; pgsql-hackers

On Fri, Mar 3, 2023 at 3:34 AM Heikki Linnakangas <[email protected]> wrote:
> I took a different approach to consuming the XIDs. Instead of setting
> nextXID directly, bypassing GetNewTransactionId(), this patch introduces
> a helper function to call GetNewTransactionId() repeatedly. But because
> that's slow, it does include a shortcut to skip over "uninteresting"
> XIDs. Whenever nextXid is close to an SLRU page boundary or XID
> wraparound, it calls GetNewTransactionId(), and otherwise it bumps up
> nextXid close to the next "interesting" value. That's still a lot slower
> than just setting nextXid, but exercises the code more realistically.

Surely your tap test should be using single user mode?  Perhaps you
missed the obnoxious HINT, that's part of the WARNING that the test
parses?  ;-)

This is a very useful patch. I certainly don't want to make life
harder by (say) connecting it to the single user mode problem.
But...the single user mode thing really needs to go away. It's just
terrible advice, and actively harms users.

-- 
Peter Geoghegan






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


end of thread, other threads:[~2023-03-14 06:01 UTC | newest]

Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 01:35 [PATCH 4/4] Implement vacuum full/cluster (INDEX_TABLESPACE <tablespace>) Justin Pryzby <[email protected]>
2021-04-23 20:43 Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2021-04-23 23:08 ` Re: Testing autovacuum wraparound (including failsafe) Justin Pryzby <[email protected]>
2021-04-23 23:26   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2021-04-23 23:12 ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-04-24 00:29   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2021-04-24 02:15     ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-04-24 02:33       ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2021-04-24 02:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-04-24 02:53           ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2021-04-24 02:56             ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-05-14 01:03               ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-05-18 05:28       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2021-05-18 05:42         ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-05-18 05:46           ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2021-05-18 07:09             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2021-05-25 00:14               ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[email protected]>
2021-06-10 13:42 ` Re: Testing autovacuum wraparound (including failsafe) Anastasia Lubennikova <[email protected]>
2021-06-11 01:18   ` Re: Testing autovacuum wraparound (including failsafe) Andres Freund <[email protected]>
2022-02-01 02:58     ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2022-06-30 01:40       ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2022-11-16 04:38         ` Re: Testing autovacuum wraparound (including failsafe) Ian Lawrence Barwick <[email protected]>
2023-03-03 11:34           ` Re: Testing autovacuum wraparound (including failsafe) Heikki Linnakangas <[email protected]>
2023-03-08 04:52             ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2023-03-14 06:01               ` Re: Testing autovacuum wraparound (including failsafe) Masahiko Sawada <[email protected]>
2023-03-08 05:21             ` Re: Testing autovacuum wraparound (including failsafe) Peter Geoghegan <[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