0001-Propagate-counts-up-only-to-topmost-ancestor.patch

text/x-patch

Filename: 0001-Propagate-counts-up-only-to-topmost-ancestor.patch
Type: text/x-patch
Part: 0
Message: Re: Autovacuum on partitioned table (autoanalyze)

Patch

Format: format-patch
Series: patch 0001
Subject: Propagate counts up only to topmost ancestor
File+
src/backend/catalog/partition.c 53 0
src/backend/commands/analyze.c 2 1
src/backend/postmaster/autovacuum.c 108 114
src/backend/postmaster/pgstat.c 34 26
src/include/catalog/partition.h 1 0
src/include/pgstat.h 13 8
From 3e904de5f15cfc69692ad2aea64c0034445d957e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 10 Aug 2021 13:05:59 -0400
Subject: [PATCH 1/2] Propagate counts up only to topmost ancestor

Ignore intermediate partitions, to avoid redundant sampling of
partitions.  If needed, those intermediate partitions can be analyzed
manually.
---
 src/backend/catalog/partition.c     |  53 +++++++
 src/backend/commands/analyze.c      |   3 +-
 src/backend/postmaster/autovacuum.c | 222 ++++++++++++++--------------
 src/backend/postmaster/pgstat.c     |  60 ++++----
 src/include/catalog/partition.h     |   1 +
 src/include/pgstat.h                |  21 ++-
 6 files changed, 211 insertions(+), 149 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 790f4ccb92..017d5ba5a2 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -26,6 +26,7 @@
 #include "nodes/makefuncs.h"
 #include "optimizer/optimizer.h"
 #include "partitioning/partbounds.h"
+#include "pgstat.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/fmgroids.h"
 #include "utils/partcache.h"
@@ -166,6 +167,58 @@ get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors)
 	get_partition_ancestors_worker(inhRel, parentOid, ancestors);
 }
 
+/*
+ * Inform pgstats collector about the topmost ancestor of
+ * each of the given partitions.
+ */
+void
+partition_analyze_report_ancestors(List *partitions)
+{
+	List	   *report_parts = NIL;
+	List	   *ancestors = NIL;
+	Relation	inhRel;
+	ListCell   *lc;
+
+	inhRel = table_open(InheritsRelationId, AccessShareLock);
+
+	/*
+	 * Search pg_inherits for the topmost ancestor of each given partition,
+	 * and if found, store both their OIDs in lists.
+	 *
+	 * By the end of this loop, partitions and ancestors are lists to be
+	 * read in parallel, where the i'th element of ancestors is the topmost
+	 * ancestor of the i'th element of partitions.
+	 */
+	foreach(lc, partitions)
+	{
+		Oid		partition_id = lfirst_oid(lc);
+		Oid		cur_relid;
+
+		cur_relid = partition_id;
+		for (;;)
+		{
+			bool	detach_pending;
+			Oid		parent_relid;
+
+			parent_relid = get_partition_parent_worker(inhRel, cur_relid,
+													   &detach_pending);
+			if ((!OidIsValid(parent_relid) || detach_pending) &&
+				cur_relid != partition_id)
+			{
+				report_parts = lappend_oid(report_parts, partition_id);
+				ancestors = lappend_oid(ancestors, cur_relid);
+				break;
+			}
+
+			cur_relid = parent_relid;
+		}
+	}
+
+	table_close(inhRel, AccessShareLock);
+
+	pgstat_report_anl_ancestors(report_parts, ancestors);
+}
+
 /*
  * index_get_partition
  *		Return the OID of index of the given partition that is a child
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 0099a04bbe..c930e3e3cd 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -30,6 +30,7 @@
 #include "catalog/catalog.h"
 #include "catalog/index.h"
 #include "catalog/indexing.h"
+#include "catalog/partition.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_namespace.h"
@@ -708,7 +709,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 		onerel->rd_rel->relkind == RELKIND_RELATION &&
 		onerel->rd_rel->relpersistence == RELPERSISTENCE_PERMANENT)
 	{
-		pgstat_report_anl_ancestors(RelationGetRelid(onerel));
+		partition_analyze_report_ancestors(list_make1_oid(RelationGetRelid(onerel)));
 	}
 
 	/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 59d348b062..f686f1d39f 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -74,6 +74,7 @@
 #include "access/xact.h"
 #include "catalog/dependency.h"
 #include "catalog/namespace.h"
+#include "catalog/partition.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
 #include "commands/dbcommands.h"
@@ -180,7 +181,6 @@ typedef struct avw_dbase
 typedef struct av_relation
 {
 	Oid			ar_toastrelid;	/* hash key - must be first */
-	Oid			ar_relid;
 	bool		ar_hasrelopts;
 	AutoVacOpts ar_reloptions;	/* copy of AutoVacOpts from the main table's
 								 * reloptions, or NULL if none */
@@ -1959,18 +1959,17 @@ do_autovacuum(void)
 	Form_pg_database dbForm;
 	List	   *table_oids = NIL;
 	List	   *orphan_oids = NIL;
+	List	   *report_ancestors = NIL;
 	HASHCTL		ctl;
 	HTAB	   *table_toast_map;
 	ListCell   *volatile cell;
 	PgStat_StatDBEntry *shared;
 	PgStat_StatDBEntry *dbentry;
 	BufferAccessStrategy bstrategy;
-	ScanKeyData key;
 	TupleDesc	pg_class_desc;
 	int			effective_multixact_freeze_max_age;
 	bool		did_vacuum = false;
 	bool		found_concurrent_worker = false;
-	bool		updated = false;
 	int			i;
 
 	/*
@@ -2056,19 +2055,17 @@ do_autovacuum(void)
 	/*
 	 * Scan pg_class to determine which tables to vacuum.
 	 *
-	 * We do this in three passes: First we let pgstat collector know about
+	 * We do this in two passes: First we let pgstat collector know about
 	 * the partitioned table ancestors of all partitions that have recently
 	 * acquired rows for analyze.  This informs the second pass about the
-	 * total number of tuple count in partitioning hierarchies.
+	 * total number of tuple count in partitioning hierarchies.  In this scan
+	 * we also collect the association of main tables to toast tables.
 	 *
 	 * On the second pass, we collect the list of plain relations,
-	 * materialized views and partitioned tables.  On the third one we collect
-	 * TOAST tables.
-	 *
-	 * The reason for doing the third pass is that during it we want to use
-	 * the main relation's pg_class.reloptions entry if the TOAST table does
-	 * not have any, and we cannot obtain it unless we know beforehand what's
-	 * the main table OID.
+	 * materialized views, partitioned tables.  Also do TOAST tables,
+	 * using the association collected during the first scan (we want to
+	 * apply the main table's reloptions entry in case the TOAST table
+	 * doesn't have any.)
 	 *
 	 * We need to check TOAST tables separately because in cases with short,
 	 * wide tables there might be proportionally much more activity in the
@@ -2079,43 +2076,90 @@ do_autovacuum(void)
 	/*
 	 * First pass: before collecting the list of tables to vacuum, let stat
 	 * collector know about partitioned-table ancestors of each partition.
+	 * Also capture the TOAST-to-main-table association.
 	 */
 	while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
 	{
 		Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
-		Oid			relid = classForm->oid;
-		PgStat_StatTabEntry *tabentry;
 
-		/* Only consider permanent leaf partitions */
-		if (!classForm->relispartition ||
-			classForm->relkind == RELKIND_PARTITIONED_TABLE ||
-			classForm->relpersistence == RELPERSISTENCE_TEMP)
+		/* Ignore all temp tables here */
+		if (classForm->relpersistence == RELPERSISTENCE_TEMP)
 			continue;
 
 		/*
-		 * No need to do this for partitions that haven't acquired any rows.
+		 * Remember TOAST associations for the second pass.  Note: we must do
+		 * this whether or not the table is going to be vacuumed, because we
+		 * don't automatically vacuum toast tables along the parent table.
 		 */
-		tabentry = pgstat_fetch_stat_tabentry(relid);
-		if (tabentry &&
-			tabentry->changes_since_analyze -
-			tabentry->changes_since_analyze_reported > 0)
+		if ((classForm->relkind == RELKIND_RELATION ||
+			 classForm->relkind == RELKIND_MATVIEW) &&
+			OidIsValid(classForm->reltoastrelid))
 		{
-			pgstat_report_anl_ancestors(relid);
-			updated = true;
+			av_relation *hentry;
+			bool		found;
+
+			hentry = hash_search(table_toast_map,
+								 &classForm->reltoastrelid,
+								 HASH_ENTER, &found);
+			if (!found)
+			{
+				AutoVacOpts *relopts;
+
+				/* hash_search already filled in the key */
+				relopts = extract_autovac_opts(tuple, pg_class_desc);
+				if (relopts)
+				{
+					memcpy(&hentry->ar_reloptions, relopts,
+						   sizeof(AutoVacOpts));
+					hentry->ar_hasrelopts = true;
+				}
+				else
+					hentry->ar_hasrelopts = false;
+			}
+		}
+
+		/* For the below, only consider leaf partitions. */
+		if (classForm->relispartition &&
+			classForm->relkind != RELKIND_PARTITIONED_TABLE)
+		{
+			PgStat_StatTabEntry *tabentry;
+
+			tabentry = pgstat_fetch_stat_tabentry(classForm->oid);
+			if (tabentry &&
+				tabentry->changes_since_analyze -
+				tabentry->changes_since_analyze_reported > 0)
+			{
+				report_ancestors = lappend_oid(report_ancestors, classForm->oid);
+			}
 		}
 	}
 
-	/* Acquire fresh stats for the next passes, if needed */
-	if (updated)
+	/*
+	 * Send the partition-ancestor report to the collector, then acquire fresh
+	 * stats for what comes next.
+	 */
+	if (report_ancestors != NIL)
 	{
+		partition_analyze_report_ancestors(report_ancestors);
+		list_free(report_ancestors);
+
+		/*
+		 * XXX some clever wait goes here, so that collector has time to digest
+		 * the above updates.  I have no better ideas than just sleeping.  We
+		 * hope this is correct because the next backend_read_statsfile will
+		 * only succeed if the file read is as new as the current timestamp.
+		 * This hopefully gives sufficient time for the messages we just sent
+		 * to be processed.
+		 */
+		pg_usleep(1000 * 500);	/* PGSTAT_STAT_INTERVAL */
+
 		autovac_refresh_stats();
 		dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
 		shared = pgstat_fetch_stat_dbentry(InvalidOid);
 	}
 
 	/*
-	 * On the second pass, we collect main tables to vacuum, and also the main
-	 * table relid to TOAST relid mapping.
+	 * On the second pass we collect tables to vacuum.
 	 */
 	while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
 	{
@@ -2127,13 +2171,46 @@ do_autovacuum(void)
 		bool		doanalyze;
 		bool		wraparound;
 
+		relid = classForm->oid;
+
+		if (classForm->relkind == RELKIND_TOASTVALUE &&
+			classForm->relpersistence != RELPERSISTENCE_TEMP)
+		{
+			/*
+			 * fetch reloptions -- if this toast table does not have them, try the
+			 * main rel
+			 */
+			relopts = extract_autovac_opts(tuple, pg_class_desc);
+			if (relopts == NULL)
+			{
+				av_relation *hentry;
+				bool		found;
+
+				hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+				if (found && hentry->ar_hasrelopts)
+					relopts = &hentry->ar_reloptions;
+			}
+
+			/* Fetch the pgstat entry for this table */
+			tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+												 shared, dbentry);
+
+			relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+									  effective_multixact_freeze_max_age,
+									  &dovacuum, &doanalyze, &wraparound);
+
+			/* ignore analyze for toast tables */
+			if (dovacuum)
+				table_oids = lappend_oid(table_oids, relid);
+
+			continue;
+		}
+
 		if (classForm->relkind != RELKIND_RELATION &&
 			classForm->relkind != RELKIND_MATVIEW &&
 			classForm->relkind != RELKIND_PARTITIONED_TABLE)
 			continue;
 
-		relid = classForm->oid;
-
 		/*
 		 * Check if it is a temp table (presumably, of some other backend's).
 		 * We cannot safely process other backends' temp tables.
@@ -2172,89 +2249,6 @@ do_autovacuum(void)
 		/* Relations that need work are added to table_oids */
 		if (dovacuum || doanalyze)
 			table_oids = lappend_oid(table_oids, relid);
-
-		/*
-		 * Remember TOAST associations for the second pass.  Note: we must do
-		 * this whether or not the table is going to be vacuumed, because we
-		 * don't automatically vacuum toast tables along the parent table.
-		 */
-		if (OidIsValid(classForm->reltoastrelid))
-		{
-			av_relation *hentry;
-			bool		found;
-
-			hentry = hash_search(table_toast_map,
-								 &classForm->reltoastrelid,
-								 HASH_ENTER, &found);
-
-			if (!found)
-			{
-				/* hash_search already filled in the key */
-				hentry->ar_relid = relid;
-				hentry->ar_hasrelopts = false;
-				if (relopts != NULL)
-				{
-					hentry->ar_hasrelopts = true;
-					memcpy(&hentry->ar_reloptions, relopts,
-						   sizeof(AutoVacOpts));
-				}
-			}
-		}
-	}
-
-	table_endscan(relScan);
-
-	/* third pass: check TOAST tables */
-	ScanKeyInit(&key,
-				Anum_pg_class_relkind,
-				BTEqualStrategyNumber, F_CHAREQ,
-				CharGetDatum(RELKIND_TOASTVALUE));
-
-	relScan = table_beginscan_catalog(classRel, 1, &key);
-	while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
-	{
-		Form_pg_class classForm = (Form_pg_class) GETSTRUCT(tuple);
-		PgStat_StatTabEntry *tabentry;
-		Oid			relid;
-		AutoVacOpts *relopts = NULL;
-		bool		dovacuum;
-		bool		doanalyze;
-		bool		wraparound;
-
-		/*
-		 * We cannot safely process other backends' temp tables, so skip 'em.
-		 */
-		if (classForm->relpersistence == RELPERSISTENCE_TEMP)
-			continue;
-
-		relid = classForm->oid;
-
-		/*
-		 * fetch reloptions -- if this toast table does not have them, try the
-		 * main rel
-		 */
-		relopts = extract_autovac_opts(tuple, pg_class_desc);
-		if (relopts == NULL)
-		{
-			av_relation *hentry;
-			bool		found;
-
-			hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
-			if (found && hentry->ar_hasrelopts)
-				relopts = &hentry->ar_reloptions;
-		}
-
-		/* Fetch the pgstat entry for this table */
-		tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
-											 shared, dbentry);
-
-		relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
-								  effective_multixact_freeze_max_age,
-								  &dovacuum, &doanalyze, &wraparound);
-
-		/* ignore analyze for toast tables */
-		if (dovacuum)
-			table_oids = lappend_oid(table_oids, relid);
 	}
 
 	table_endscan(relScan);
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 1b54ef74eb..6af6b7ea45 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1679,42 +1679,42 @@ pgstat_report_analyze(Relation rel,
 /*
  * pgstat_report_anl_ancestors
  *
- *	Send list of partitioned table ancestors of the given partition to the
- *	collector.  The collector is in charge of propagating the analyze tuple
- *	counts from the partition to its ancestors.  This is necessary so that
+ *	Send a message to report the ancestor of each given partition.
+ *	The collector is in charge of propagating the analyze tuple
+ *	counts from the partition to its ancestor.  This is necessary so that
  *	other processes can decide whether to analyze the partitioned tables.
  */
 void
-pgstat_report_anl_ancestors(Oid relid)
+pgstat_report_anl_ancestors(List *partitions, List *ancestors)
 {
 	PgStat_MsgAnlAncestors msg;
-	List	   *ancestors;
-	ListCell   *lc;
+	ListCell   *lc1,
+			   *lc2;
 
 	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANL_ANCESTORS);
 	msg.m_databaseid = MyDatabaseId;
-	msg.m_tableoid = relid;
 	msg.m_nancestors = 0;
 
-	ancestors = get_partition_ancestors(relid);
-	foreach(lc, ancestors)
+	forboth(lc1, partitions, lc2, ancestors)
 	{
-		Oid			ancestor = lfirst_oid(lc);
+		msg.m_ancestors[msg.m_nancestors].m_partition_id = lfirst_oid(lc1);
+		msg.m_ancestors[msg.m_nancestors].m_ancestor_id = lfirst_oid(lc2);
+		msg.m_nancestors++;
 
-		msg.m_ancestors[msg.m_nancestors] = ancestor;
-		if (++msg.m_nancestors >= PGSTAT_NUM_ANCESTORENTRIES)
+		if (msg.m_nancestors >= PGSTAT_NUM_ANCESTORENTRIES)
 		{
 			pgstat_send(&msg, offsetof(PgStat_MsgAnlAncestors, m_ancestors[0]) +
-						msg.m_nancestors * sizeof(Oid));
+						msg.m_nancestors * sizeof(PgStat_AnlAncestor));
 			msg.m_nancestors = 0;
 		}
 	}
 
 	if (msg.m_nancestors > 0)
+	{
 		pgstat_send(&msg, offsetof(PgStat_MsgAnlAncestors, m_ancestors[0]) +
-					msg.m_nancestors * sizeof(Oid));
-
-	list_free(ancestors);
+					msg.m_nancestors * sizeof(PgStat_AnlAncestor));
+		msg.m_nancestors = 0;
+	}
 }
 
 /* --------
@@ -5403,28 +5403,36 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
 	}
 }
 
+/* ------------
+ * pgstat_recv_anl_ancestors
+ *
+ *	Process an ANALYZE ANCESTORS message
+ * ------------
+ */
 static void
 pgstat_recv_anl_ancestors(PgStat_MsgAnlAncestors *msg, int len)
 {
 	PgStat_StatDBEntry *dbentry;
-	PgStat_StatTabEntry *tabentry;
-
-	dbentry = pgstat_get_db_entry(msg->m_databaseid, true);
-
-	tabentry = pgstat_get_tab_entry(dbentry, msg->m_tableoid, true);
 
+	dbentry = pgstat_get_db_entry(msg->m_databaseid, false);
+	if (dbentry == NULL)
+		return;
 	for (int i = 0; i < msg->m_nancestors; i++)
 	{
-		Oid			ancestor_relid = msg->m_ancestors[i];
+		PgStat_StatTabEntry *tabentry;
 		PgStat_StatTabEntry *ancestor;
 
-		ancestor = pgstat_get_tab_entry(dbentry, ancestor_relid, true);
+		tabentry = pgstat_get_tab_entry(dbentry, msg->m_ancestors[i].m_partition_id, false);
+		if (tabentry == NULL)
+			continue;
+		ancestor = pgstat_get_tab_entry(dbentry, msg->m_ancestors[i].m_ancestor_id, true);
+		if (ancestor == NULL)
+			continue;
+
 		ancestor->changes_since_analyze +=
 			tabentry->changes_since_analyze - tabentry->changes_since_analyze_reported;
+		tabentry->changes_since_analyze_reported = tabentry->changes_since_analyze;
 	}
-
-	tabentry->changes_since_analyze_reported = tabentry->changes_since_analyze;
-
 }
 
 /* ----------
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index c8c7bc1d99..6b16f92f22 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -22,6 +22,7 @@
 extern Oid	get_partition_parent(Oid relid, bool even_if_detached);
 extern List *get_partition_ancestors(Oid relid);
 extern Oid	index_get_partition(Relation partition, Oid indexId);
+extern void partition_analyze_report_ancestors(List *partitions);
 extern List *map_partition_varattnos(List *expr, int fromrel_varno,
 									 Relation to_rel, Relation from_rel);
 extern bool has_partition_attrs(Relation rel, Bitmapset *attnums,
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2068a68a5f..bbca2d7755 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -433,22 +433,27 @@ typedef struct PgStat_MsgAnalyze
 
 /* ----------
  * PgStat_MsgAnlAncestors		Sent by the backend or autovacuum daemon
- *								to inform partitioned tables that are
- *								ancestors of a partition, to propagate
+ *								to inform partitioned table that's
+ *								top-most ancestor of a partition, to propagate
  *								analyze counters
  * ----------
  */
-#define PGSTAT_NUM_ANCESTORENTRIES    \
-	((PGSTAT_MSG_PAYLOAD - sizeof(Oid) - sizeof(Oid) - sizeof(int))	\
-	 / sizeof(Oid))
+#define PGSTAT_NUM_ANCESTORENTRIES	\
+	((PGSTAT_MSG_PAYLOAD - sizeof(Oid) - sizeof(int)) \
+	 / sizeof(PgStat_AnlAncestor))
+
+typedef struct PgStat_AnlAncestor
+{
+	Oid			m_partition_id;
+	Oid			m_ancestor_id;
+} PgStat_AnlAncestor;
 
 typedef struct PgStat_MsgAnlAncestors
 {
 	PgStat_MsgHdr m_hdr;
 	Oid			m_databaseid;
-	Oid			m_tableoid;
 	int			m_nancestors;
-	Oid			m_ancestors[PGSTAT_NUM_ANCESTORENTRIES];
+	PgStat_AnlAncestor m_ancestors[PGSTAT_NUM_ANCESTORENTRIES];
 } PgStat_MsgAnlAncestors;
 
 /* ----------
@@ -1038,7 +1043,7 @@ extern void pgstat_report_vacuum(Oid tableoid, bool shared,
 extern void pgstat_report_analyze(Relation rel,
 								  PgStat_Counter livetuples, PgStat_Counter deadtuples,
 								  bool resetcounter);
-extern void pgstat_report_anl_ancestors(Oid relid);
+extern void pgstat_report_anl_ancestors(List *partitions, List *ancestors);
 
 extern void pgstat_report_recovery_conflict(int reason);
 extern void pgstat_report_deadlock(void);
-- 
2.20.1