Date: Tue, 10 Aug 2021 17:38:12 -0400
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Andres Freund <andres@anarazel.de>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>,
	yuzuko <yuzukohosoya@gmail.com>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	David Steele <david@pgmasters.net>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Daniel Gustafsson <daniel@yesql.se>,
	Amit Langote <amitlangote09@gmail.com>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
	Laurenz Albe <laurenz.albe@cybertec.at>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Greg Stark <stark@mit.edu>
Subject: Re: Autovacuum on partitioned table (autoanalyze)
Message-ID: <202108102138.rmo2pbphkkqs@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nelpyvwv3hukkuuh"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202108101327.6sutwnjffpid@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/202108102138.rmo2pbphkkqs%40alvherre.pgsql>
Precedence: bulk


--nelpyvwv3hukkuuh
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2021-Aug-10, Alvaro Herrera wrote:

> I bring a radical proposal that may be sufficient to close this
> particular hole.  What if we made partition only affected their
> top-level parents to become auto-analyzed, and not any intermediate
> ancestors?  Any intermediate partitioned partitions could be analyzed
> manually if the user wished, and perhaps some reloption could enable
> autovacuum to do it (with the caveat that it'd cause multiple sampling
> of partitions).  I don't yet have a clear picture on how to implement
> this, but I'll explore it while waiting for opinions on the idea.

So, with this patch (a quick and dirty job) we no longer sample all
partitions twice; we no longer propagate the tuple counts to p_0.
We don't have stats on p_0 anymore, only on p and on the individual
partitions.

I didn't move the new #include to a more decent place because
1. that stuff is going to move to partition.c as a new function,
including the new include;
2. that new function also needs to read the reloptions for p_0 to allow
the user to enable stat acquisition for p_0 with "alter table p_0 set
(autovacuum_enabled=1)";
3. need to avoid reporting ancestors of a partition repeatedly, which
forestalls the performance objection about reading reloptions too
frequently.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

--nelpyvwv3hukkuuh
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Propagate-counts-up-only-to-topmost-ancestor.patch"

From 064bc88bf94b6b4e1bfc16f0639e1500b17b9bf5 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] 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/postmaster/pgstat.c | 21 ++++++++++++++++-----
 src/include/pgstat.h            |  9 +++++++--
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 1b54ef74eb..a003966cc8 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1684,6 +1684,7 @@ pgstat_report_analyze(Relation rel,
  *	counts from the partition to its ancestors.  This is necessary so that
  *	other processes can decide whether to analyze the partitioned tables.
  */
+#include "utils/lsyscache.h"
 void
 pgstat_report_anl_ancestors(Oid relid)
 {
@@ -1700,19 +1701,25 @@ pgstat_report_anl_ancestors(Oid relid)
 	foreach(lc, ancestors)
 	{
 		Oid			ancestor = lfirst_oid(lc);
+		bool		ispartition;
 
-		msg.m_ancestors[msg.m_nancestors] = ancestor;
-		if (++msg.m_nancestors >= PGSTAT_NUM_ANCESTORENTRIES)
+		ispartition = get_rel_relispartition(ancestor);
+
+		msg.m_ancestors[msg.m_nancestors].m_ancestor_id = ancestor;
+		msg.m_ancestors[msg.m_nancestors].m_propagate_up = !ispartition;
+		msg.m_nancestors++;
+
+		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));
+					msg.m_nancestors * sizeof(PgStat_AnlAncestor));
 
 	list_free(ancestors);
 }
@@ -5415,9 +5422,13 @@ pgstat_recv_anl_ancestors(PgStat_MsgAnlAncestors *msg, int len)
 
 	for (int i = 0; i < msg->m_nancestors; i++)
 	{
-		Oid			ancestor_relid = msg->m_ancestors[i];
+		Oid			ancestor_relid;
 		PgStat_StatTabEntry *ancestor;
 
+		if (!msg->m_ancestors[i].m_propagate_up)
+			continue;
+
+		ancestor_relid = msg->m_ancestors[i].m_ancestor_id;
 		ancestor = pgstat_get_tab_entry(dbentry, ancestor_relid, true);
 		ancestor->changes_since_analyze +=
 			tabentry->changes_since_analyze - tabentry->changes_since_analyze_reported;
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2068a68a5f..46ef88e73b 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -438,9 +438,14 @@ typedef struct PgStat_MsgAnalyze
  *								analyze counters
  * ----------
  */
+typedef struct PgStat_AnlAncestor
+{
+	Oid			m_ancestor_id;
+	bool		m_propagate_up;
+} PgStat_AnlAncestor;
 #define PGSTAT_NUM_ANCESTORENTRIES    \
 	((PGSTAT_MSG_PAYLOAD - sizeof(Oid) - sizeof(Oid) - sizeof(int))	\
-	 / sizeof(Oid))
+	 / sizeof(PgStat_AnlAncestor))
 
 typedef struct PgStat_MsgAnlAncestors
 {
@@ -448,7 +453,7 @@ typedef struct PgStat_MsgAnlAncestors
 	Oid			m_databaseid;
 	Oid			m_tableoid;
 	int			m_nancestors;
-	Oid			m_ancestors[PGSTAT_NUM_ANCESTORENTRIES];
+	PgStat_AnlAncestor m_ancestors[PGSTAT_NUM_ANCESTORENTRIES];
 } PgStat_MsgAnlAncestors;
 
 /* ----------
-- 
2.20.1


--nelpyvwv3hukkuuh--