From: Justin Pryzby <pryzby@telsasoft.com>
To: Álvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: yuzuko <yuzukohosoya@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: David Steele <david@pgmasters.net>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Greg Stark <stark@mit.edu>
Subject: Re: Autovacuum on partitioned table (autoanalyze)
Date: Tue, 17 Aug 2021 06:30:18 -0500
Message-ID: <20210817113018.GG10479@telsasoft.com> (raw)
In-Reply-To: <20210816222810.GE10479@telsasoft.com>
References: <202108161546.3zuouk76skr4@alvherre.pgsql>
<202108162142.crk2myigb2ad@alvherre.pgsql>
<20210816222810.GE10479@telsasoft.com>
On Mon, Aug 16, 2021 at 05:28:10PM -0500, Justin Pryzby wrote:
> On Mon, Aug 16, 2021 at 05:42:48PM -0400, Álvaro Herrera wrote:
> > On 2021-Aug-16, Álvaro Herrera wrote:
> >
> > > Here's the reversal patch for the 14 branch. (It applies cleanly to
> > > master, but the unused member of PgStat_StatTabEntry needs to be
> > > removed and catversion bumped).
> >
> > I have pushed this to both branches. (I did not remove the item from
> > the release notes in the 14 branch.)
>
> | I retained the addition of relkind 'p' to tables included by
> | pg_stat_user_tables, because reverting that would require a catversion
> | bump.
>
> Right now, on v15dev, it shows 0, which is misleading.
> Shouldn't it be null ?
>
> analyze_count | 0
>
> Note that having analyze_count and last_analyze would be an an independently
> useful change. Since parent tables aren't analyzed automatically, I have a
> script to periodically process them if they weren't processed recently. Right
> now, for partitioned tables, the best I could find is to check its partitions:
> | MIN(last_analyzed) FROM pg_stat_all_tables psat JOIN pg_inherits i ON psat.relid=i.inhrelid
>
> In 20200418050815.GE26953@telsasoft.com I wrote:
> |This patch includes partitioned tables in pg_stat_*_tables, which is great; I
> |complained awhile ago that they were missing [0]. It might be useful if that
> |part was split out into a separate 0001 patch (?).
> | [0] https://www.postgresql.org/message-id/20180601221428.GU5164%40telsasoft.com
I suggest the attached (which partially reverts the revert), to allow showing
correct data for analyze_count and last_analyzed.
Arguably these should be reported as null in v14 for partitioned tables, since
they're not "known to be zero", but rather "currently unpopulated".
n_mod_since_analyze | 0
n_ins_since_vacuum | 0
Justin
Attachments:
[text/x-diff] 0001-Report-last_analyze-and-analyze_count-of-partitioned.patch (4.7K, ../20210817113018.GG10479@telsasoft.com/2-0001-Report-last_analyze-and-analyze_count-of-partitioned.patch)
download | inline diff:
From 0d0e149727d89115803b4528e15f5b3c04bd816b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 16 Aug 2021 22:55:06 -0500
Subject: [PATCH] Report last_analyze and analyze_count of partitioned tables..
In v14, partitioned tables are included, but these fields are being reported as
zero, which is misleading.
---
src/backend/commands/analyze.c | 36 ++++++++++++++++++++++-----------
src/backend/postmaster/pgstat.c | 27 +++++++++++++++++--------
2 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.cindex 8d7b38d170..0050df08f6 100644--- a/src/backend/commands/analyze.c+++ b/src/backend/commands/analyze.c@@ -626,8 +626,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
PROGRESS_ANALYZE_PHASE_FINALIZE_ANALYZE);
/*
- * Update pages/tuples stats in pg_class, and report ANALYZE to the stats- * collector ... but not if we're doing inherited stats.+ * Update pages/tuples stats in pg_class ... but not if we're doing+ * inherited stats.
*
* We assume that VACUUM hasn't set pg_class.reltuples already, even
* during a VACUUM ANALYZE. Although VACUUM often updates pg_class,
@@ -668,20 +668,32 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
InvalidMultiXactId,
in_outer_xact);
}
-+ }+ else if (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ {
/*
- * Now report ANALYZE to the stats collector.- *- * We deliberately don't report to the stats collector when doing- * inherited stats, because the stats collector only tracks per-table- * stats.- *- * Reset the changes_since_analyze counter only if we analyzed all- * columns; otherwise, there is still work for auto-analyze to do.+ * Partitioned tables don't have storage, so we don't set any fields+ * in their pg_class entries except for reltuples, which is necessary+ * for auto-analyze to work properly, and relhasindex.
*/
+ vac_update_relstats(onerel, -1, totalrows,+ 0, hasindex, InvalidTransactionId,+ InvalidMultiXactId,+ in_outer_xact);+ }++ /*+ * Now report ANALYZE to the stats collector. For regular tables, we do+ * it only if not doing inherited stats. For partitioned tables, we only+ * do it for inherited stats. (We're never called for not-inherited stats+ * on partitioned tables anyway.)+ *+ * Reset the changes_since_analyze counter only if we analyzed all+ * columns; otherwise, there is still work for auto-analyze to do.+ */+ if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
pgstat_report_analyze(onerel, totalrows, totaldeadrows,
(va_cols == NIL));
- }
/*
* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup.
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.cindex a3c35bdf60..2a9673154b 100644--- a/src/backend/postmaster/pgstat.c+++ b/src/backend/postmaster/pgstat.c@@ -1632,21 +1632,31 @@ pgstat_report_analyze(Relation rel,
* be double-counted after commit. (This approach also ensures that the
* collector ends up with the right numbers if we abort instead of
* committing.)
+ *+ * For partitioned tables, we don't report live and dead tuples, because+ * such tables don't have any data.
*/
if (rel->pgstat_info != NULL)
{
PgStat_TableXactStatus *trans;
- for (trans = rel->pgstat_info->trans; trans; trans = trans->upper)+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ /* If this rel is partitioned, skip modifying */+ livetuples = deadtuples = 0;+ else
{
- livetuples -= trans->tuples_inserted - trans->tuples_deleted;- deadtuples -= trans->tuples_updated + trans->tuples_deleted;+ for (trans = rel->pgstat_info->trans; trans; trans = trans->upper)+ {+ livetuples -= trans->tuples_inserted - trans->tuples_deleted;+ deadtuples -= trans->tuples_updated + trans->tuples_deleted;+ }+ /* count stuff inserted by already-aborted subxacts, too */+ deadtuples -= rel->pgstat_info->t_counts.t_delta_dead_tuples;+ /* Since ANALYZE's counts are estimates, we could have underflowed */+ livetuples = Max(livetuples, 0);+ deadtuples = Max(deadtuples, 0);
}
- /* count stuff inserted by already-aborted subxacts, too */- deadtuples -= rel->pgstat_info->t_counts.t_delta_dead_tuples;- /* Since ANALYZE's counts are estimates, we could have underflowed */- livetuples = Max(livetuples, 0);- deadtuples = Max(deadtuples, 0);+
}
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE);
@@ -1999,6 +2009,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!RELKIND_HAS_STORAGE(relkind))
+// relkind != RELKIND_PARTITIONED_TABLE)
{
rel->pgstat_info = NULL;
return;
--
2.17.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, alvherre@alvh.no-ip.org, andres@anarazel.de, horiguchi.kyotaro@lab.ntt.co.jp, yuzukohosoya@gmail.com, tomas.vondra@enterprisedb.com, david@pgmasters.net, horikyota.ntt@gmail.com, daniel@yesql.se, amitlangote09@gmail.com, masahiko.sawada@2ndquadrant.com, laurenz.albe@cybertec.at, pgsql-hackers@lists.postgresql.org, stark@mit.edu
Subject: Re: Autovacuum on partitioned table (autoanalyze)
In-Reply-To: <20210817113018.GG10479@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox