pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedAutovacuum on partitioned table
28+ messages / 6 participants
[nested] [flat]
* Autovacuum on partitioned table
@ 2019-12-02 09:02 yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2019-12-02 09:02 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Greg Stark <stark@mit.edu>
Hello,
Greg reported in [1] before, autovacuum ignores partitioned tables.
That is, even if individual partitions’ statistics are updated, its parent's
statistics are not updated. This is TODO for declarative partitioning.
As Amit mentioned in [2], a way to make parent's statistics from
partitions' statistics without scanning the partitions would be nice,
but it will need a lot of modifications. So I tried to fix that using the
current analyze method.
The summary of the attached patch is as follows:
* If the relation is a partitioned table, check its children if they need
vacuum or analyze. Children need to do that are added to
a table list for autovacuuum. At least one child is added to the list,
the partitioned table is also added to the list. Then, autovacuum
runs on all the tables in the list.
* If the partitioned table has foreign partitions, ignore them.
When the parent has children don't need vacuum/analyze or foreign
partitions, parent's stats are updated scanning the current data of all
children, so old stats and new are mixed within the partition tree.
Is that suitable? Any thoughts?
[1] https://www.postgresql.org/message-id/CAM-w4HMQKC8hw7nB9TW3OV%2BhkB5OUcPtvr_U_EiSOjByoa-e4Q%40mail.g...
[2] https://www.postgresql.org/message-id/CA%2BHiwqEeZQ-H2OVbHZ%3Dn2RNNPF84Hygi1HC-MDwC-VnBjpA1%3DQ%40ma...
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v1_autovacuum_on_partitioned_table.patch (8.6K, ../../CAKkQ508_PwVgwJyBY=0Lmkz90j8CmWNPUxgHvCUwGhMrouz6UA@mail.gmail.com/2-v1_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c1dd8168ca..d4ce99a9f5 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2036,11 +2037,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2063,7 +2064,12 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ continue;
+
+ /* Collect partitioned tables, not partitions. So skip them. */
+ if (classForm->relispartition)
continue;
relid = classForm->oid;
@@ -2092,19 +2098,76 @@ do_autovacuum(void)
continue;
}
- /* Fetch reloptions and the pgstat entry for this table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ if (classForm->relkind == RELKIND_RELATION ||
+ classForm->relkind == RELKIND_MATVIEW)
+ {
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
+
+ /* Check if it needs vacuum or analyze */
+ relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* Relations that need work are added to table_oids */
+ if (dovacuum || doanalyze)
+ table_oids = lappend_oid(table_oids, relid);
+ }
+ else
+ {
+ /*
+ * If the relation is a partitioned table, we check if its children
+ * need vacuum or analyze. All children excluding foreign partitions
+ * need to do that are added to the table_oids list. At least one
+ * child is added the list, the partitioned table become an object
+ * for autovacuum.
+ */
+ List *tableOIDs;
+ ListCell *lc;
+ List *child_oids = NIL;
- /* Check if it needs vacuum or analyze */
- relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
- /* Relations that need work are added to table_oids */
- if (dovacuum || doanalyze)
- table_oids = lappend_oid(table_oids, relid);
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(childtuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(childOID,
+ childclassForm->relisshared,
+ shared, dbentry);
+
+ relation_needs_vacanalyze(childOID, relopts, childclassForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ if (dovacuum || doanalyze)
+ child_oids = lappend_oid(child_oids, childOID);
+ }
+
+ if (child_oids)
+ {
+ table_oids = lappend_oid(table_oids, relid);
+ table_oids = list_concat(table_oids, child_oids);
+ }
+ }
/*
* Remember TOAST associations for the second pass. Note: we must do
@@ -2725,6 +2788,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2798,33 +2862,79 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
return NULL;
classForm = (Form_pg_class) GETSTRUCT(classTup);
- /*
- * Get the applicable reloptions. If it is a TOAST table, try to get the
- * main table reloptions if the toast table itself doesn't have.
- */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- if (classForm->relkind == RELKIND_TOASTVALUE &&
- avopts == NULL && table_toast_map != NULL)
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
{
- av_relation *hentry;
- bool found;
+ /*
+ * Get the applicable reloptions. If it is a TOAST table, try to get the
+ * main table reloptions if the toast table itself doesn't have.
+ */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ if (classForm->relkind == RELKIND_TOASTVALUE &&
+ avopts == NULL && table_toast_map != NULL)
+ {
+ av_relation *hentry;
+ bool found;
+
+ hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+ if (found && hentry->ar_hasrelopts)
+ avopts = &hentry->ar_reloptions;
+ }
+
+ /* fetch the pgstat table entry */
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
+
+ relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
- if (found && hentry->ar_hasrelopts)
- avopts = &hentry->ar_reloptions;
+ /* ignore ANALYZE for toast tables */
+ if (classForm->relkind == RELKIND_TOASTVALUE)
+ doanalyze = false;
}
+ else
+ {
+ /*
+ * If the relation is partitioned and doesn't have any foreign tables
+ * we check its children again.
+ */
+ List *tableOIDs;
+ ListCell *lc;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
- /* fetch the pgstat table entry */
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ /* Fetch reloptions and the pgstat entry */
+ avopts = extract_autovac_opts(childtuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(childOID, childclassForm->relisshared,
+ shared, dbentry);
- relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ relation_needs_vacanalyze(childOID, avopts, childclassForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- /* ignore ANALYZE for toast tables */
- if (classForm->relkind == RELKIND_TOASTVALUE)
- doanalyze = false;
+ /* Its parents need vacuum or analyze */
+ if (dovacuum || doanalyze)
+ break;
+ }
+ }
/* OK, it needs something done */
if (doanalyze || dovacuum)
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2019-12-02 09:19 Laurenz Albe <laurenz.albe@cybertec.at>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Laurenz Albe @ 2019-12-02 09:19 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Greg Stark <stark@mit.edu>
On Mon, 2019-12-02 at 18:02 +0900, yuzuko wrote:
> Greg reported in [1] before, autovacuum ignores partitioned tables.
> That is, even if individual partitions’ statistics are updated, its parent's
> statistics are not updated. This is TODO for declarative partitioning.
> As Amit mentioned in [2], a way to make parent's statistics from
> partitions' statistics without scanning the partitions would be nice,
> but it will need a lot of modifications. So I tried to fix that using the
> current analyze method.
>
> The summary of the attached patch is as follows:
> * If the relation is a partitioned table, check its children if they need
> vacuum or analyze. Children need to do that are added to
> a table list for autovacuuum. At least one child is added to the list,
> the partitioned table is also added to the list. Then, autovacuum
> runs on all the tables in the list.
That means that all partitions are vacuumed if only one of them needs it,
right? This will result in way more vacuuming than necessary.
Wouldn't it be an option to update the partitioned table's statistics
whenever one of the partitions is vacuumed?
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2019-12-02 09:42 yuzuko <yuzukohosoya@gmail.com>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2019-12-02 09:42 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hi Laurenz,
Thanks for the comments.
On Mon, Dec 2, 2019 at 6:19 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> On Mon, 2019-12-02 at 18:02 +0900, yuzuko wrote:
> > Greg reported in [1] before, autovacuum ignores partitioned tables.
> > That is, even if individual partitions’ statistics are updated, its parent's
> > statistics are not updated. This is TODO for declarative partitioning.
> > As Amit mentioned in [2], a way to make parent's statistics from
> > partitions' statistics without scanning the partitions would be nice,
> > but it will need a lot of modifications. So I tried to fix that using the
> > current analyze method.
> >
> > The summary of the attached patch is as follows:
> > * If the relation is a partitioned table, check its children if they need
> > vacuum or analyze. Children need to do that are added to
> > a table list for autovacuuum. At least one child is added to the list,
> > the partitioned table is also added to the list. Then, autovacuum
> > runs on all the tables in the list.
>
> That means that all partitions are vacuumed if only one of them needs it,
> right? This will result in way more vacuuming than necessary.
>
Autovacuum runs only partitions need vacuum/analyze, so unnecessary
partitions stats are not updated. However, to make parent's stats,
all children are scanned. It might be a waste of time.
> Wouldn't it be an option to update the partitioned table's statistics
> whenever one of the partitions is vacuumed?
>
> Yours,
> Laurenz Albe
>
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2019-12-27 03:37 yuzuko <yuzukohosoya@gmail.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2019-12-27 03:37 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hi,
As Laurenz commented in this thread, I tried adding option
to update parent's statistics during Autovacuum. To do that,
I propose supporting 'autovacuum_enabled' option already
exists on partitioned tables.
In the attached patch, you can use 'autovacuum_enabled' option
on partitioned table as usual, that is, a default value of this option
is true. So if you don't need autovacuum on a partitioned table,
you have to specify the option:
CREATE TABLE p(i int) partition by range(i) with (autovacuum_enabled=0);
I'm not sure but I wonder if a suitable value as a default of
'autovacuum_enabled' for partitioned tables might be false.
Because autovacuum on *partitioned tables* requires scanning
all children to make partitioned tables' statistics.
But if the default value varies according to the relation,
is it confusing? Any thoughts?
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v2_autovacuum_on_partitioned_table.patch (11.2K, ../../CAKkQ50_=gqT9DbgOVD677qUQY7mSepsq-Vet=73cKgTxvUBNvQ@mail.gmail.com/2-v2_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index e9a31aa257..8d80efd445 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1308,8 +1308,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 48377ace24..d640aa4bda 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -1586,13 +1586,19 @@ build_reloptions(Datum reloptions, bool validate,
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+ static const relopt_parse_elt tab[] = {
+ {"autovacuum_enabled", RELOPT_TYPE_BOOL,
+ offsetof(PartitionedTableOptions, autovacuum_enabled)}
+ };
+
/*
* There are no options for partitioned tables yet, but this is able to do
* some validation.
*/
return (bytea *) build_reloptions(reloptions, validate,
RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ sizeof(PartitionedTableOptions),
+ tab, lengthof(tab));
}
/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c1dd8168ca..7bd4950d90 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2036,11 +2037,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2063,7 +2064,12 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ continue;
+
+ /* Collect partitioned tables, not partitions. So skip them. */
+ if (classForm->relispartition)
continue;
relid = classForm->oid;
@@ -2092,19 +2098,85 @@ do_autovacuum(void)
continue;
}
- /* Fetch reloptions and the pgstat entry for this table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ if (classForm->relkind == RELKIND_RELATION ||
+ classForm->relkind == RELKIND_MATVIEW)
+ {
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
+
+ /* Check if it needs vacuum or analyze */
+ relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* Relations that need work are added to table_oids */
+ if (dovacuum || doanalyze)
+ table_oids = lappend_oid(table_oids, relid);
+ }
+ else
+ {
+ /*
+ * If the relation is a partitioned table, we check if its children
+ * need vacuum or analyze. All children excluding foreign partitions
+ * need to do that are added to the table_oids list. At least one
+ * child is added the list, the partitioned table become an object
+ * for autovacuum.
+ */
+ List *tableOIDs;
+ ListCell *lc;
+ List *child_oids = NIL;
+ bool av_enabled;
- /* Check if it needs vacuum or analyze */
- relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ /*
+ * Fetch reloptions and check whether partitioned table needs
+ * autovacuum or not.
+ */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ av_enabled = (relopts ? relopts->enabled : true);
- /* Relations that need work are added to table_oids */
- if (dovacuum || doanalyze)
- table_oids = lappend_oid(table_oids, relid);
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(childtuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(childOID,
+ childclassForm->relisshared,
+ shared, dbentry);
+
+ relation_needs_vacanalyze(childOID, relopts, childclassForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ if (dovacuum || doanalyze)
+ child_oids = lappend_oid(child_oids, childOID);
+ }
+
+ if (child_oids)
+ {
+ if (av_enabled)
+ table_oids = lappend_oid(table_oids, relid);
+ table_oids = list_concat(table_oids, child_oids);
+ }
+ }
/*
* Remember TOAST associations for the second pass. Note: we must do
@@ -2725,6 +2797,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2798,33 +2871,79 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
return NULL;
classForm = (Form_pg_class) GETSTRUCT(classTup);
- /*
- * Get the applicable reloptions. If it is a TOAST table, try to get the
- * main table reloptions if the toast table itself doesn't have.
- */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- if (classForm->relkind == RELKIND_TOASTVALUE &&
- avopts == NULL && table_toast_map != NULL)
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
{
- av_relation *hentry;
- bool found;
+ /*
+ * Get the applicable reloptions. If it is a TOAST table, try to get the
+ * main table reloptions if the toast table itself doesn't have.
+ */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ if (classForm->relkind == RELKIND_TOASTVALUE &&
+ avopts == NULL && table_toast_map != NULL)
+ {
+ av_relation *hentry;
+ bool found;
+
+ hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+ if (found && hentry->ar_hasrelopts)
+ avopts = &hentry->ar_reloptions;
+ }
+
+ /* fetch the pgstat table entry */
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
- hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
- if (found && hentry->ar_hasrelopts)
- avopts = &hentry->ar_reloptions;
+ relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* ignore ANALYZE for toast tables */
+ if (classForm->relkind == RELKIND_TOASTVALUE)
+ doanalyze = false;
}
+ else
+ {
+ /*
+ * If the relation is partitioned and doesn't have any foreign tables
+ * we check its children again.
+ */
+ List *tableOIDs;
+ ListCell *lc;
- /* fetch the pgstat table entry */
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
- relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
- /* ignore ANALYZE for toast tables */
- if (classForm->relkind == RELKIND_TOASTVALUE)
- doanalyze = false;
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Fetch reloptions and the pgstat entry */
+ avopts = extract_autovac_opts(childtuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(childOID, childclassForm->relisshared,
+ shared, dbentry);
+
+ relation_needs_vacanalyze(childOID, avopts, childclassForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* Its parents need vacuum or analyze */
+ if (dovacuum || doanalyze)
+ break;
+ }
+ }
/* OK, it needs something done */
if (doanalyze || dovacuum)
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 31d8a1a10e..d007d7feab 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -326,6 +326,16 @@ typedef struct StdRdOptions
((relation)->rd_options ? \
((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+/*
+ * PartitionedTableOptions
+ * Contents of rd_options for partitioned tables
+ */
+typedef struct PartitionedTableOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ bool autovacuum_enabled;
+} PartitionedTableOptions;
+
/* ViewOptions->check_option values */
typedef enum ViewOptCheckOption
{
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2019-12-27 05:01 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Masahiko Sawada @ 2019-12-27 05:01 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Fri, 27 Dec 2019 at 12:37, yuzuko <yuzukohosoya@gmail.com> wrote:
>
> Hi,
>
> As Laurenz commented in this thread, I tried adding option
> to update parent's statistics during Autovacuum. To do that,
> I propose supporting 'autovacuum_enabled' option already
> exists on partitioned tables.
>
> In the attached patch, you can use 'autovacuum_enabled' option
> on partitioned table as usual, that is, a default value of this option
> is true. So if you don't need autovacuum on a partitioned table,
> you have to specify the option:
> CREATE TABLE p(i int) partition by range(i) with (autovacuum_enabled=0);
>
> I'm not sure but I wonder if a suitable value as a default of
> 'autovacuum_enabled' for partitioned tables might be false.
> Because autovacuum on *partitioned tables* requires scanning
> all children to make partitioned tables' statistics.
> But if the default value varies according to the relation,
> is it confusing? Any thoughts?
I don't look at the patch deeply yet but your patch seems to attempt
to vacuum on partitioned table. IIUC partitioned tables don't need to
be vacuumed and its all child tables are vacuumed instead if we pass
the partitioned table to vacuum() function. But autovacuum on child
tables is normally triggered since their statistics are updated.
I think it's a good idea to have that option but I think that doing
autovacuum on the parent table every time when autovacuum is triggered
on one of its child tables is very high cost especially when there are
a lot of child tables. Instead I thought it's more straight forward if
we compare the summation of the statistics of child tables (e.g.
n_live_tuples, n_dead_tuples etc) to vacuum thresholds when we
consider the needs of autovacuum on the parent table. What do you
think?
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-01-28 08:52 Amit Langote <amitlangote09@gmail.com>
parent: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-01-28 08:52 UTC (permalink / raw)
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; +Cc: yuzuko <yuzukohosoya@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hello,
On Fri, Dec 27, 2019 at 2:02 PM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
> On Fri, 27 Dec 2019 at 12:37, yuzuko <yuzukohosoya@gmail.com> wrote:
> > As Laurenz commented in this thread, I tried adding option
> > to update parent's statistics during Autovacuum. To do that,
> > I propose supporting 'autovacuum_enabled' option already
> > exists on partitioned tables.
> >
> > In the attached patch, you can use 'autovacuum_enabled' option
> > on partitioned table as usual, that is, a default value of this option
> > is true. So if you don't need autovacuum on a partitioned table,
> > you have to specify the option:
> > CREATE TABLE p(i int) partition by range(i) with (autovacuum_enabled=0);
> >
> > I'm not sure but I wonder if a suitable value as a default of
> > 'autovacuum_enabled' for partitioned tables might be false.
> > Because autovacuum on *partitioned tables* requires scanning
> > all children to make partitioned tables' statistics.
> > But if the default value varies according to the relation,
> > is it confusing? Any thoughts?
>
> I don't look at the patch deeply yet but your patch seems to attempt
> to vacuum on partitioned table. IIUC partitioned tables don't need to
> be vacuumed and its all child tables are vacuumed instead if we pass
> the partitioned table to vacuum() function. But autovacuum on child
> tables is normally triggered since their statistics are updated.
>
> I think it's a good idea to have that option but I think that doing
> autovacuum on the parent table every time when autovacuum is triggered
> on one of its child tables is very high cost especially when there are
> a lot of child tables. Instead I thought it's more straight forward if
> we compare the summation of the statistics of child tables (e.g.
> n_live_tuples, n_dead_tuples etc) to vacuum thresholds when we
> consider the needs of autovacuum on the parent table. What do you
> think?
There's this old email where Tom outlines a few ideas about triggering
auto-analyze on inheritance trees:
https://www.postgresql.org/message-id/4823.1262132964%40sss.pgh.pa.us
If I'm reading that correctly, the idea is to track only
changes_since_analyze and none of the finer-grained stats like
live/dead tuples for inheritance parents (partitioned tables) using
some new pgstat infrastrcture, an idea that Hosoya-san also seems to
be considering per an off-list discussion. Besides the complexity of
getting that infrastructure in place, an important question is whether
the current system of applying threshold and scale factor to
changes_since_analyze should be used as-is for inheritance parents
(partitioned tables), because if users set those parameters similarly
to for regular tables, autovacuum might analyze partitioned tables
more than necessary. We'll either need a different formula, or some
commentary in the documentation about how partitioned tables might
need different setting, or maybe both.
By the way, maybe I'm misunderstanding what Sawada-san wrote above,
but the only missing piece seems to be a way to trigger an *analyze*
on the parent tables -- to collect optimizer statistics for the
inheritance trees -- not vacuum, for which the existing system seems
enough.
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-01-28 11:27 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Masahiko Sawada @ 2020-01-28 11:27 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: yuzuko <yuzukohosoya@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Tue, 28 Jan 2020 at 17:52, Amit Langote <amitlangote09@gmail.com> wrote:
>
> Hello,
>
> On Fri, Dec 27, 2019 at 2:02 PM Masahiko Sawada
> <masahiko.sawada@2ndquadrant.com> wrote:
> > On Fri, 27 Dec 2019 at 12:37, yuzuko <yuzukohosoya@gmail.com> wrote:
> > > As Laurenz commented in this thread, I tried adding option
> > > to update parent's statistics during Autovacuum. To do that,
> > > I propose supporting 'autovacuum_enabled' option already
> > > exists on partitioned tables.
> > >
> > > In the attached patch, you can use 'autovacuum_enabled' option
> > > on partitioned table as usual, that is, a default value of this option
> > > is true. So if you don't need autovacuum on a partitioned table,
> > > you have to specify the option:
> > > CREATE TABLE p(i int) partition by range(i) with (autovacuum_enabled=0);
> > >
> > > I'm not sure but I wonder if a suitable value as a default of
> > > 'autovacuum_enabled' for partitioned tables might be false.
> > > Because autovacuum on *partitioned tables* requires scanning
> > > all children to make partitioned tables' statistics.
> > > But if the default value varies according to the relation,
> > > is it confusing? Any thoughts?
> >
> > I don't look at the patch deeply yet but your patch seems to attempt
> > to vacuum on partitioned table. IIUC partitioned tables don't need to
> > be vacuumed and its all child tables are vacuumed instead if we pass
> > the partitioned table to vacuum() function. But autovacuum on child
> > tables is normally triggered since their statistics are updated.
> >
> > I think it's a good idea to have that option but I think that doing
> > autovacuum on the parent table every time when autovacuum is triggered
> > on one of its child tables is very high cost especially when there are
> > a lot of child tables. Instead I thought it's more straight forward if
> > we compare the summation of the statistics of child tables (e.g.
> > n_live_tuples, n_dead_tuples etc) to vacuum thresholds when we
> > consider the needs of autovacuum on the parent table. What do you
> > think?
>
> There's this old email where Tom outlines a few ideas about triggering
> auto-analyze on inheritance trees:
>
> https://www.postgresql.org/message-id/4823.1262132964%40sss.pgh.pa.us
>
> If I'm reading that correctly, the idea is to track only
> changes_since_analyze and none of the finer-grained stats like
> live/dead tuples for inheritance parents (partitioned tables) using
> some new pgstat infrastrcture, an idea that Hosoya-san also seems to
> be considering per an off-list discussion. Besides the complexity of
> getting that infrastructure in place, an important question is whether
> the current system of applying threshold and scale factor to
> changes_since_analyze should be used as-is for inheritance parents
> (partitioned tables), because if users set those parameters similarly
> to for regular tables, autovacuum might analyze partitioned tables
> more than necessary.
How are you going to track changes_since_analyze of partitioned table?
It's just an idea but we can accumulate changes_since_analyze of
partitioned table by adding child tables's value after analyzing each
child table. And compare the partitioned tables value to the threshold
that is computed by (autovacuum_analyze_threshold + total rows
including all child tables * autovacuum_analyze_scale_factor).
> By the way, maybe I'm misunderstanding what Sawada-san wrote above,
> but the only missing piece seems to be a way to trigger an *analyze*
> on the parent tables -- to collect optimizer statistics for the
> inheritance trees -- not vacuum, for which the existing system seems
> enough.
Right. We need only autoanalyze on partitioned tables.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-01-29 02:29 yuzuko <yuzukohosoya@gmail.com>
parent: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2020-01-29 02:29 UTC (permalink / raw)
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hello,
> Besides the complexity of
> getting that infrastructure in place, an important question is whether
> the current system of applying threshold and scale factor to
> changes_since_analyze should be used as-is for inheritance parents
> (partitioned tables), because if users set those parameters similarly
> to for regular tables, autovacuum might analyze partitioned tables
> more than necessary. We'll either need a different formula, or some
> commentary in the documentation about how partitioned tables might
> need different setting, or maybe both.
>
I'm not sure but I think we need new autovacuum parameters for
partitioned tables (autovacuum, autovacuum_analyze_threshold,
autovacuum_analyze_scale_factor) because whether it's necessary
to run autovacuum on partitioned tables will depend on users.
What do you think?
> How are you going to track changes_since_analyze of partitioned table?
> It's just an idea but we can accumulate changes_since_analyze of
> partitioned table by adding child tables's value after analyzing each
> child table. And compare the partitioned tables value to the threshold
> that is computed by (autovacuum_analyze_threshold + total rows
> including all child tables * autovacuum_analyze_scale_factor).
>
The idea Sawada-san mentioned is similar to mine. Also, for tracking
changes_since_analyze, we have to make partitioned table's statistics.
To do that, we can invent a new PgStat_StatPartitionedTabEntry based
on PgStat_StatTabEntry. Through talking with Amit, I think the new structure
needs the following members:
tableid
changes_since_analyze
analyze_timestamp
analyze_count
autovac_analyze_timestamp
autovac_analyze_count
Vacuum doesn't run on partitioned tables, so I think members related to
(auto) vacuum need not be contained in the structure.
I'm still writing a patch. I'll send it this week.
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-01-29 08:56 Amit Langote <amitlangote09@gmail.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 2 replies; 28+ messages in thread
From: Amit Langote @ 2020-01-29 08:56 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Wed, Jan 29, 2020 at 11:29 AM yuzuko <yuzukohosoya@gmail.com> wrote:
> > Besides the complexity of
> > getting that infrastructure in place, an important question is whether
> > the current system of applying threshold and scale factor to
> > changes_since_analyze should be used as-is for inheritance parents
> > (partitioned tables), because if users set those parameters similarly
> > to for regular tables, autovacuum might analyze partitioned tables
> > more than necessary. We'll either need a different formula, or some
> > commentary in the documentation about how partitioned tables might
> > need different setting, or maybe both.
> >
> I'm not sure but I think we need new autovacuum parameters for
> partitioned tables (autovacuum, autovacuum_analyze_threshold,
> autovacuum_analyze_scale_factor) because whether it's necessary
> to run autovacuum on partitioned tables will depend on users.
> What do you think?
Yes, we will need to first support those parameters on partitioned
tables. Currently, you get:
create table p (a int) partition by list (a) with
(autovacuum_analyze_scale_factor=0);
ERROR: unrecognized parameter "autovacuum_analyze_scale_factor"
> > How are you going to track changes_since_analyze of partitioned table?
> > It's just an idea but we can accumulate changes_since_analyze of
> > partitioned table by adding child tables's value after analyzing each
> > child table. And compare the partitioned tables value to the threshold
> > that is computed by (autovacuum_analyze_threshold + total rows
> > including all child tables * autovacuum_analyze_scale_factor).
> >
> The idea Sawada-san mentioned is similar to mine.
So if I understand this idea correctly, a partitioned table's analyze
will only be triggered when partitions are analyzed. That is,
inserts, updates, deletes of tuples in partitions will be tracked by
pgstat, which in turn is used by autovacuum to trigger analyze on
partitions. Then, partitions changes_since_analyze is added into the
parent's changes_since_analyze, which in turn *may* trigger analyze
parent. I said "may", because it would take multiple partition
analyzes to accumulate enough changes to trigger one on the parent.
Am I getting that right?
> Also, for tracking
> changes_since_analyze, we have to make partitioned table's statistics.
> To do that, we can invent a new PgStat_StatPartitionedTabEntry based
> on PgStat_StatTabEntry. Through talking with Amit, I think the new structure
> needs the following members:
>
> tableid
> changes_since_analyze
> analyze_timestamp
> analyze_count
> autovac_analyze_timestamp
> autovac_analyze_count
>
> Vacuum doesn't run on partitioned tables, so I think members related to
> (auto) vacuum need not be contained in the structure.
On second thought, maybe we don't need a new PgStat_ struct. We can
just use what's used for regular tables and leave the fields that
don't make sense for partitioned tables set to 0, such as those that
track the counts of scans, tuples, etc. That means we don't have to
mess with interfaces of existing functions, like this one:
static void relation_needs_vacanalyze(Oid relid,
AutoVacOpts *relopts,
Form_pg_class classForm,
PgStat_StatTabEntry *tabentry, ...
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-01-29 11:38 Michael Paquier <michael@paquier.xyz>
parent: Amit Langote <amitlangote09@gmail.com>
1 sibling, 0 replies; 28+ messages in thread
From: Michael Paquier @ 2020-01-29 11:38 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: yuzuko <yuzukohosoya@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>
On Wed, Jan 29, 2020 at 05:56:40PM +0900, Amit Langote wrote:
> Yes, we will need to first support those parameters on partitioned
> tables. Currently, you get:
>
> create table p (a int) partition by list (a) with
> (autovacuum_analyze_scale_factor=0);
> ERROR: unrecognized parameter "autovacuum_analyze_scale_factor"
Worth the note: partitioned tables support zero reloptions as of now,
but there is the facility in place to allow that (see
RELOPT_KIND_PARTITIONED and partitioned_table_reloptions).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../20200129113852.GI145179@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-02 03:52 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
parent: Amit Langote <amitlangote09@gmail.com>
1 sibling, 1 reply; 28+ messages in thread
From: Masahiko Sawada @ 2020-02-02 03:52 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: yuzuko <yuzukohosoya@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Wed, 29 Jan 2020 at 17:56, Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Wed, Jan 29, 2020 at 11:29 AM yuzuko <yuzukohosoya@gmail.com> wrote:
> > > Besides the complexity of
> > > getting that infrastructure in place, an important question is whether
> > > the current system of applying threshold and scale factor to
> > > changes_since_analyze should be used as-is for inheritance parents
> > > (partitioned tables), because if users set those parameters similarly
> > > to for regular tables, autovacuum might analyze partitioned tables
> > > more than necessary. We'll either need a different formula, or some
> > > commentary in the documentation about how partitioned tables might
> > > need different setting, or maybe both.
> > >
> > I'm not sure but I think we need new autovacuum parameters for
> > partitioned tables (autovacuum, autovacuum_analyze_threshold,
> > autovacuum_analyze_scale_factor) because whether it's necessary
> > to run autovacuum on partitioned tables will depend on users.
> > What do you think?
>
> Yes, we will need to first support those parameters on partitioned
> tables. Currently, you get:
>
> create table p (a int) partition by list (a) with
> (autovacuum_analyze_scale_factor=0);
> ERROR: unrecognized parameter "autovacuum_analyze_scale_factor"
>
> > > How are you going to track changes_since_analyze of partitioned table?
> > > It's just an idea but we can accumulate changes_since_analyze of
> > > partitioned table by adding child tables's value after analyzing each
> > > child table. And compare the partitioned tables value to the threshold
> > > that is computed by (autovacuum_analyze_threshold + total rows
> > > including all child tables * autovacuum_analyze_scale_factor).
> > >
> > The idea Sawada-san mentioned is similar to mine.
>
> So if I understand this idea correctly, a partitioned table's analyze
> will only be triggered when partitions are analyzed. That is,
> inserts, updates, deletes of tuples in partitions will be tracked by
> pgstat, which in turn is used by autovacuum to trigger analyze on
> partitions. Then, partitions changes_since_analyze is added into the
> parent's changes_since_analyze, which in turn *may* trigger analyze
> parent. I said "may", because it would take multiple partition
> analyzes to accumulate enough changes to trigger one on the parent.
> Am I getting that right?
Yeah that is what I meant. In addition, adding partition's
changes_since_analyze to its parent needs to be done recursively as
the parent table could also be a partitioned table.
>
> > Also, for tracking
> > changes_since_analyze, we have to make partitioned table's statistics.
> > To do that, we can invent a new PgStat_StatPartitionedTabEntry based
> > on PgStat_StatTabEntry. Through talking with Amit, I think the new structure
> > needs the following members:
> >
> > tableid
> > changes_since_analyze
> > analyze_timestamp
> > analyze_count
> > autovac_analyze_timestamp
> > autovac_analyze_count
> >
> > Vacuum doesn't run on partitioned tables, so I think members related to
> > (auto) vacuum need not be contained in the structure.
>
> On second thought, maybe we don't need a new PgStat_ struct. We can
> just use what's used for regular tables and leave the fields that
> don't make sense for partitioned tables set to 0, such as those that
> track the counts of scans, tuples, etc. That means we don't have to
> mess with interfaces of existing functions, like this one:
>
> static void relation_needs_vacanalyze(Oid relid,
> AutoVacOpts *relopts,
> Form_pg_class classForm,
> PgStat_StatTabEntry *tabentry, ...
+1
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-03 02:36 Amit Langote <amitlangote09@gmail.com>
parent: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-03 02:36 UTC (permalink / raw)
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; +Cc: yuzuko <yuzukohosoya@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Sun, Feb 2, 2020 at 12:53 PM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
> On Wed, 29 Jan 2020 at 17:56, Amit Langote <amitlangote09@gmail.com> wrote:
> > On Wed, Jan 29, 2020 at 11:29 AM yuzuko <yuzukohosoya@gmail.com> wrote:
> > > > How are you going to track changes_since_analyze of partitioned table?
> > > > It's just an idea but we can accumulate changes_since_analyze of
> > > > partitioned table by adding child tables's value after analyzing each
> > > > child table. And compare the partitioned tables value to the threshold
> > > > that is computed by (autovacuum_analyze_threshold + total rows
> > > > including all child tables * autovacuum_analyze_scale_factor).
> > > >
> > > The idea Sawada-san mentioned is similar to mine.
> >
> > So if I understand this idea correctly, a partitioned table's analyze
> > will only be triggered when partitions are analyzed. That is,
> > inserts, updates, deletes of tuples in partitions will be tracked by
> > pgstat, which in turn is used by autovacuum to trigger analyze on
> > partitions. Then, partitions changes_since_analyze is added into the
> > parent's changes_since_analyze, which in turn *may* trigger analyze
> > parent. I said "may", because it would take multiple partition
> > analyzes to accumulate enough changes to trigger one on the parent.
> > Am I getting that right?
>
> Yeah that is what I meant. In addition, adding partition's
> changes_since_analyze to its parent needs to be done recursively as
> the parent table could also be a partitioned table.
That's a good point. So, changes_since_analyze increments are
essentially propagated from leaf partitions to all the way up to the
root table, including any intermediate partitioned tables. We'll need
to consider whether we should propagate only one level at a time (from
bottom of the tree) or update all parents up to the root, every time a
leaf partition is analyzed. If we we do the latter, that might end up
triggering analyze on all the parents at the same time causing
repeated scanning of the same child tables in close intervals,
although setting analyze threshold and scale factor of the parent
tables of respective levels wisely can help avoid any negative impact
of that.
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-20 06:33 yuzuko <yuzukohosoya@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2020-02-20 06:33 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hello,
I'm sorry for the delay.
Attach the latest patch based on discussion in this thread.
> > Yeah that is what I meant. In addition, adding partition's
> > changes_since_analyze to its parent needs to be done recursively as
> > the parent table could also be a partitioned table.
>
> That's a good point. So, changes_since_analyze increments are
> essentially propagated from leaf partitions to all the way up to the
> root table, including any intermediate partitioned tables. We'll need
> to consider whether we should propagate only one level at a time (from
> bottom of the tree) or update all parents up to the root, every time a
> leaf partition is analyzed.
For multi-level partitioning, all parents' changes_since_analyze will be
updated whenever analyzing a leaf partition in this patch.
Could you please check the patch again?
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v3_autovacuum_on_partitioned_table.patch (22.1K, ../../CAKkQ50_=siZssjZdsLbdE-4MiRdEAwKT4PJhsjR=ePGWb0ejew@mail.gmail.com/2-v3_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 4a2b6f0dae..88c635f82f 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1310,8 +1310,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 79430d2b7b..20183a96a4 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -227,7 +227,7 @@ static relopt_int intRelOpts[] =
{
"autovacuum_analyze_threshold",
"Minimum number of tuple inserts, updates or deletes prior to analyze",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0, INT_MAX
@@ -379,7 +379,7 @@ static relopt_real realRelOpts[] =
{
"autovacuum_analyze_scale_factor",
"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0.0, 100.0
@@ -1586,13 +1586,12 @@ build_reloptions(Datum reloptions, bool validate,
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+
/*
- * There are no options for partitioned tables yet, but this is able to do
- * some validation.
+ * autovacuum_enabled, autovacuum_analyze_threshold and
+ * autovacuum_analyze_scale_factor are supported for partitioned tables.
*/
- return (bytea *) build_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ return default_reloptions(reloptions, validate, RELOPT_KIND_PARTITIONED);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f681aafcf9..161abb6450 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -584,7 +584,7 @@ CREATE VIEW pg_stat_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_xact_all_tables AS
@@ -604,7 +604,7 @@ CREATE VIEW pg_stat_xact_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_sys_tables AS
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c4420ddd7f..df3d93ea5d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -655,15 +655,14 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+ (va_cols == NIL));
/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
if (!(params->options & VACOPT_VACUUM))
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6d1f28c327..7d0a5ce30d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2031,11 +2032,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2058,7 +2059,8 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
relid = classForm->oid;
@@ -2087,19 +2089,103 @@ do_autovacuum(void)
continue;
}
- /* Fetch reloptions and the pgstat entry for this table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ if (classForm->relkind == RELKIND_RELATION ||
+ classForm->relkind == RELKIND_MATVIEW)
+ {
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
+
+ /* Check if it needs vacuum or analyze */
+ relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* Relations that need work are added to table_oids */
+ if (dovacuum || doanalyze)
+ table_oids = lappend_oid(table_oids, relid);
+ }
+ else
+ {
+ /*
+ * If the relation is a partitioned table, we check it using reltuples
+ * added up childrens' and changes_since_analyze tracked by stats collector.
+ * We check only auto analyze because partitioned tables don't need to vacuum.
+ */
+ List *tableOIDs;
+ ListCell *lc;
+ bool av_enabled;
+ int anl_base_thresh;
+ float4 all_reltuples = 0,
+ anl_scale_factor,
+ anlthresh,
+ reltuples,
+ anltuples;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
- /* Check if it needs vacuum or analyze */
- relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
- /* Relations that need work are added to table_oids */
- if (dovacuum || doanalyze)
- table_oids = lappend_oid(table_oids, relid);
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ all_reltuples += childclassForm->reltuples;
+ elog(NOTICE, "[parent:%s] child%s has %.0f tuples", NameStr(classForm->relname),NameStr(childclassForm->relname), childclassForm->reltuples);
+ }
+
+
+ /* Fetch reloptions and the pgstat entry for the partitioned table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid,
+ classForm->relisshared,
+ shared, dbentry);
+
+ /* Check if it needs auto analyze */
+ av_enabled = (relopts ? relopts->enabled : true);
+
+ if (av_enabled)
+ {
+ anl_scale_factor = (relopts && relopts->analyze_scale_factor >= 0)
+ ? relopts->analyze_scale_factor
+ : autovacuum_anl_scale;
+
+ anl_base_thresh = (relopts && relopts->analyze_threshold >= 0)
+ ? relopts->analyze_threshold
+ : autovacuum_anl_thresh;
+
+ elog(NOTICE, "[parent:%s] has %.0f tuples", NameStr(classForm->relname), all_reltuples);
+ if (PointerIsValid(tabentry))
+ {
+ reltuples = all_reltuples;
+ anltuples = tabentry->changes_since_analyze;
+ anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
+
+ elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
+ NameStr(classForm->relname),
+ anltuples, anlthresh);
+
+ /* Determine if this table needs analyze. */
+ doanalyze = (anltuples > anlthresh);
+ }
+ if (doanalyze)
+ table_oids = lappend_oid(table_oids, relid);
+ }
+ }
/*
* Remember TOAST associations for the second pass. Note: we must do
@@ -2720,6 +2806,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2793,33 +2880,105 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
return NULL;
classForm = (Form_pg_class) GETSTRUCT(classTup);
- /*
- * Get the applicable reloptions. If it is a TOAST table, try to get the
- * main table reloptions if the toast table itself doesn't have.
- */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- if (classForm->relkind == RELKIND_TOASTVALUE &&
- avopts == NULL && table_toast_map != NULL)
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
{
- av_relation *hentry;
- bool found;
+ /*
+ * Get the applicable reloptions. If it is a TOAST table, try to get the
+ * main table reloptions if the toast table itself doesn't have.
+ */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ if (classForm->relkind == RELKIND_TOASTVALUE &&
+ avopts == NULL && table_toast_map != NULL)
+ {
+ av_relation *hentry;
+ bool found;
- hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
- if (found && hentry->ar_hasrelopts)
- avopts = &hentry->ar_reloptions;
+ hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+ if (found && hentry->ar_hasrelopts)
+ avopts = &hentry->ar_reloptions;
+ }
+
+ /* fetch the pgstat table entry */
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
+
+ relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
+
+ /* ignore ANALYZE for toast tables */
+ if (classForm->relkind == RELKIND_TOASTVALUE)
+ doanalyze = false;
}
+ else
+ {
+ List *tableOIDs;
+ ListCell *lc;
+ bool av_enabled;
+ int anl_base_thresh;
+ float4 all_reltuples,
+ anl_scale_factor,
+ anlthresh,
+ reltuples,
+ anltuples;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, tableOIDs)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclassForm;
- /* fetch the pgstat table entry */
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
- relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ /* Skip foreign partitions */
+ if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for the partitioned table */
+ all_reltuples += childclassForm->reltuples;
+ }
- /* ignore ANALYZE for toast tables */
- if (classForm->relkind == RELKIND_TOASTVALUE)
- doanalyze = false;
+ /* Fetch reloptions and the pgstat entry for the partitioned table */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid,
+ classForm->relisshared,
+ shared, dbentry);
+
+ /* Check if it needs auto analyze */
+ av_enabled = (avopts ? avopts->enabled : true);
+
+ if (av_enabled)
+ {
+ anl_scale_factor = (avopts && avopts->analyze_scale_factor >= 0)
+ ? avopts->analyze_scale_factor
+ : autovacuum_anl_scale;
+
+ anl_base_thresh = (avopts && avopts->analyze_threshold >= 0)
+ ? avopts->analyze_threshold
+ : autovacuum_anl_thresh;
+
+ if (PointerIsValid(tabentry))
+ {
+ reltuples = all_reltuples;
+ anltuples = tabentry->changes_since_analyze;
+ anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
+
+ elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
+ NameStr(classForm->relname),
+ anltuples, anlthresh);
+
+ doanalyze = (anltuples > anlthresh);
+ }
+ }
+ }
/* OK, it needs something done */
if (doanalyze || dovacuum)
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 59dc4f31ab..1933da145a 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/partition.h"
#include "catalog/pg_database.h"
#include "catalog/pg_proc.h"
#include "common/ip.h"
@@ -67,6 +68,7 @@
#include "utils/ps_status.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/timestamp.h"
/* ----------
@@ -322,6 +324,7 @@ static void pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, in
static void pgstat_recv_autovac(PgStat_MsgAutovacStart *msg, int len);
static void pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len);
static void pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len);
+static void pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *msg, int len);
static void pgstat_recv_archiver(PgStat_MsgArchiver *msg, int len);
static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
@@ -1463,6 +1466,32 @@ pgstat_report_analyze(Relation rel,
deadtuples = Max(deadtuples, 0);
}
+ /*
+ * If the table is a leaf partition, tell the stats collector its parent's
+ * changes_since_analyze for auto analyze
+ */
+ if (rel->rd_rel->relispartition &&
+ !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
+ {
+ Oid parentoid;
+ Relation parentrel;
+ PgStat_StatDBEntry *dbentry;
+ PgStat_StatTabEntry *tabentry;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Fetch the pgstat for this table */
+ dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
+ tabentry = pgstat_get_tab_entry(dbentry, RelationGetRelid(rel), true);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, tabentry->changes_since_analyze);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE);
msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
msg.m_tableoid = RelationGetRelid(rel);
@@ -1475,6 +1504,49 @@ pgstat_report_analyze(Relation rel,
}
/* --------
+ * pgstat_report_partanalyze() -
+ *
+ * Tell the collector about the parent table of which partition just analyzed.
+ *
+ * Caller must provide a child's changes_since_analyze as a parents.
+ * --------
+ */
+void
+pgstat_report_partanalyze(Relation rel, PgStat_Counter changes_tuples)
+{
+ PgStat_MsgPartAnalyze msg;
+
+ if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+ return;
+
+ /*
+ * If the partitioned table is also a partition, tell the stats collector
+ * its parent's changes_since_analyze for auto analyze
+ */
+ if (rel->rd_rel->relispartition)
+ {
+ Oid parentoid;
+ Relation parentrel;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, changes_tuples);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
+ pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_PARTANALYZE);
+ msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
+ msg.m_tableoid = RelationGetRelid(rel);
+ msg.m_changes_tuples = changes_tuples;
+ pgstat_send(&msg, sizeof(msg));
+}
+
+
+/* --------
* pgstat_report_recovery_conflict() -
*
* Tell the collector about a Hot Standby recovery conflict.
@@ -1749,6 +1821,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!(relkind == RELKIND_RELATION ||
relkind == RELKIND_MATVIEW ||
+ relkind == RELKIND_PARTITIONED_TABLE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_SEQUENCE))
@@ -4592,6 +4665,10 @@ PgstatCollectorMain(int argc, char *argv[])
pgstat_recv_analyze(&msg.msg_analyze, len);
break;
+ case PGSTAT_MTYPE_PARTANALYZE:
+ pgstat_recv_partanalyze(&msg.msg_partanalyze, len);
+ break;
+
case PGSTAT_MTYPE_ARCHIVER:
pgstat_recv_archiver(&msg.msg_archiver, len);
break;
@@ -6239,6 +6316,18 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
}
}
+static void
+pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *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);
+
+ tabentry->changes_since_analyze += msg->m_changes_tuples;
+}
/* ----------
* pgstat_recv_archiver() -
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 3a65a51696..590885d7e8 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -57,6 +57,7 @@ typedef enum StatMsgType
PGSTAT_MTYPE_AUTOVAC_START,
PGSTAT_MTYPE_VACUUM,
PGSTAT_MTYPE_ANALYZE,
+ PGSTAT_MTYPE_PARTANALYZE,
PGSTAT_MTYPE_ARCHIVER,
PGSTAT_MTYPE_BGWRITER,
PGSTAT_MTYPE_FUNCSTAT,
@@ -389,6 +390,18 @@ typedef struct PgStat_MsgAnalyze
PgStat_Counter m_dead_tuples;
} PgStat_MsgAnalyze;
+/* ----------
+ * PgStat_MsgPartAnalyze Sent by the backend or autovacuum daemon
+ * after ANALYZE for partitioned tables
+ * ----------
+ */
+typedef struct PgStat_MsgPartAnalyze
+{
+ PgStat_MsgHdr m_hdr;
+ Oid m_databaseid;
+ Oid m_tableoid;
+ PgStat_Counter m_changes_tuples;
+} PgStat_MsgPartAnalyze;
/* ----------
* PgStat_MsgArchiver Sent by the archiver to update statistics.
@@ -562,6 +575,7 @@ typedef union PgStat_Msg
PgStat_MsgAutovacStart msg_autovacuum_start;
PgStat_MsgVacuum msg_vacuum;
PgStat_MsgAnalyze msg_analyze;
+ PgStat_MsgPartAnalyze msg_partanalyze;
PgStat_MsgArchiver msg_archiver;
PgStat_MsgBgWriter msg_bgwriter;
PgStat_MsgFuncstat msg_funcstat;
@@ -1267,7 +1281,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_partanalyze(Relation rel, PgStat_Counter changes_tuples);
extern void pgstat_report_recovery_conflict(int reason);
extern void pgstat_report_deadlock(void);
extern void pgstat_report_checksum_failures_in_db(Oid dboid, int failurecount);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 634f8256f7..7e9f6de9cb 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1789,7 +1789,7 @@ pg_stat_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_archiver| SELECT s.archived_count,
s.last_archived_wal,
@@ -2117,7 +2117,7 @@ pg_stat_xact_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_xact_sys_tables| SELECT pg_stat_xact_all_tables.relid,
pg_stat_xact_all_tables.schemaname,
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-20 07:50 Amit Langote <amitlangote09@gmail.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-20 07:50 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hosoya-san,
On Thu, Feb 20, 2020 at 3:34 PM yuzuko <yuzukohosoya@gmail.com> wrote:
> Attach the latest patch based on discussion in this thread.
>
> > > Yeah that is what I meant. In addition, adding partition's
> > > changes_since_analyze to its parent needs to be done recursively as
> > > the parent table could also be a partitioned table.
> >
> > That's a good point. So, changes_since_analyze increments are
> > essentially propagated from leaf partitions to all the way up to the
> > root table, including any intermediate partitioned tables. We'll need
> > to consider whether we should propagate only one level at a time (from
> > bottom of the tree) or update all parents up to the root, every time a
> > leaf partition is analyzed.
>
> For multi-level partitioning, all parents' changes_since_analyze will be
> updated whenever analyzing a leaf partition in this patch.
> Could you please check the patch again?
Thank you for the new patch.
I built and confirmed that the patch works.
Here are some comments:
* White-space noise in the diff (space used where tab is expected);
please check with git diff --check and fix.
* Names changes_tuples, m_changes_tuples should be changed_tuples and
m_changed_tuples, respectively?
* Did you intend to make it so that we now report *all* inherited
stats to the stats collector, not just those for partitioned tables?
IOW, do did you intend the new feature to also cover traditional
inheritance parents? I am talking about the following diff:
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+ (va_cols == NIL));
* I may be missing something, but why doesn't do_autovacuum() fetch a
partitioned table's entry from pgstat instead of fetching that for
individual children and adding? That is, why do we need to do the
following:
+ /*
+ * If the relation is a partitioned table, we check it
using reltuples
+ * added up childrens' and changes_since_analyze tracked
by stats collector.
More later...
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-20 08:32 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-20 08:32 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Thu, Feb 20, 2020 at 4:50 PM Amit Langote <amitlangote09@gmail.com> wrote:
> * I may be missing something, but why doesn't do_autovacuum() fetch a
> partitioned table's entry from pgstat instead of fetching that for
> individual children and adding? That is, why do we need to do the
> following:
>
> + /*
> + * If the relation is a partitioned table, we check it
> using reltuples
> + * added up childrens' and changes_since_analyze tracked
> by stats collector.
Oh, it's only adding up children's pg_class.reltuple, not pgstat
stats. We need to do that because a partitioned table's
pg_class.reltuples is always 0 and correctly so. Sorry for not
reading the patch properly.
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-20 09:29 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-20 09:29 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Thu, Feb 20, 2020 at 5:32 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Thu, Feb 20, 2020 at 4:50 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > * I may be missing something, but why doesn't do_autovacuum() fetch a
> > partitioned table's entry from pgstat instead of fetching that for
> > individual children and adding? That is, why do we need to do the
> > following:
> >
> > + /*
> > + * If the relation is a partitioned table, we check it
> > using reltuples
> > + * added up childrens' and changes_since_analyze tracked
> > by stats collector.
>
> Oh, it's only adding up children's pg_class.reltuple, not pgstat
> stats. We need to do that because a partitioned table's
> pg_class.reltuples is always 0 and correctly so. Sorry for not
> reading the patch properly.
Having read the relevant diffs again, I think this could be done
without duplicating code too much. You seem to have added the same
logic in two places: do_autovacuum() and table_recheck_autovac().
More importantly, part of the logic of relation_needs_vacanalyze() is
duplicated in both of the aforementioned places, which I think is
unnecessary and undesirable if you consider maintainability. I think
we could just add the logic to compute reltuples for partitioned
tables at the beginning of relation_needs_vacanalyze() and be done. I
have attached a delta patch to show what I mean. Please check and
tell what you think.
Thanks,
Amit
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7d0a5ce30d..ca6996e448 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2089,103 +2089,19 @@ do_autovacuum(void)
continue;
}
- if (classForm->relkind == RELKIND_RELATION ||
- classForm->relkind == RELKIND_MATVIEW)
- {
- /* Fetch reloptions and the pgstat entry for this table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs vacuum or analyze */
- relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
-
- /* Relations that need work are added to table_oids */
- if (dovacuum || doanalyze)
- table_oids = lappend_oid(table_oids, relid);
- }
- else
- {
- /*
- * If the relation is a partitioned table, we check it using reltuples
- * added up childrens' and changes_since_analyze tracked by stats collector.
- * We check only auto analyze because partitioned tables don't need to vacuum.
- */
- List *tableOIDs;
- ListCell *lc;
- bool av_enabled;
- int anl_base_thresh;
- float4 all_reltuples = 0,
- anl_scale_factor,
- anlthresh,
- reltuples,
- anltuples;
-
- /* Find all members of inheritance set taking AccessShareLock */
- tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
-
- foreach(lc, tableOIDs)
- {
- Oid childOID = lfirst_oid(lc);
- HeapTuple childtuple;
- Form_pg_class childclassForm;
-
- /* Ignore the parent table */
- if (childOID == relid)
- continue;
-
- childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
- childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
-
- /* Skip foreign partitions */
- if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
- continue;
-
- /* Sum up the child's reltuples for its parent table */
- all_reltuples += childclassForm->reltuples;
- elog(NOTICE, "[parent:%s] child%s has %.0f tuples", NameStr(classForm->relname),NameStr(childclassForm->relname), childclassForm->reltuples);
- }
-
-
- /* Fetch reloptions and the pgstat entry for the partitioned table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid,
- classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs auto analyze */
- av_enabled = (relopts ? relopts->enabled : true);
-
- if (av_enabled)
- {
- anl_scale_factor = (relopts && relopts->analyze_scale_factor >= 0)
- ? relopts->analyze_scale_factor
- : autovacuum_anl_scale;
-
- anl_base_thresh = (relopts && relopts->analyze_threshold >= 0)
- ? relopts->analyze_threshold
- : autovacuum_anl_thresh;
-
- elog(NOTICE, "[parent:%s] has %.0f tuples", NameStr(classForm->relname), all_reltuples);
- if (PointerIsValid(tabentry))
- {
- reltuples = all_reltuples;
- anltuples = tabentry->changes_since_analyze;
- anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
- elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
- NameStr(classForm->relname),
- anltuples, anlthresh);
+ /* Check if it needs vacuum or analyze */
+ relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- /* Determine if this table needs analyze. */
- doanalyze = (anltuples > anlthresh);
- }
- if (doanalyze)
- table_oids = lappend_oid(table_oids, relid);
- }
- }
+ /* 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
@@ -2880,105 +2796,33 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
return NULL;
classForm = (Form_pg_class) GETSTRUCT(classTup);
- if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ /*
+ * Get the applicable reloptions. If it is a TOAST table, try to get the
+ * main table reloptions if the toast table itself doesn't have.
+ */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ if (classForm->relkind == RELKIND_TOASTVALUE &&
+ avopts == NULL && table_toast_map != NULL)
{
- /*
- * Get the applicable reloptions. If it is a TOAST table, try to get the
- * main table reloptions if the toast table itself doesn't have.
- */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- if (classForm->relkind == RELKIND_TOASTVALUE &&
- avopts == NULL && table_toast_map != NULL)
- {
- av_relation *hentry;
- bool found;
-
- hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
- if (found && hentry->ar_hasrelopts)
- avopts = &hentry->ar_reloptions;
- }
-
- /* fetch the pgstat table entry */
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
-
- relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ av_relation *hentry;
+ bool found;
- /* ignore ANALYZE for toast tables */
- if (classForm->relkind == RELKIND_TOASTVALUE)
- doanalyze = false;
+ hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+ if (found && hentry->ar_hasrelopts)
+ avopts = &hentry->ar_reloptions;
}
- else
- {
- List *tableOIDs;
- ListCell *lc;
- bool av_enabled;
- int anl_base_thresh;
- float4 all_reltuples,
- anl_scale_factor,
- anlthresh,
- reltuples,
- anltuples;
- /* Find all members of inheritance set taking AccessShareLock */
- tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+ /* fetch the pgstat table entry */
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
- foreach(lc, tableOIDs)
- {
- Oid childOID = lfirst_oid(lc);
- HeapTuple childtuple;
- Form_pg_class childclassForm;
+ relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- /* Ignore the parent table */
- if (childOID == relid)
- continue;
-
- childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
- childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
-
- /* Skip foreign partitions */
- if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
- continue;
-
- /* Sum up the child's reltuples for the partitioned table */
- all_reltuples += childclassForm->reltuples;
- }
-
- /* Fetch reloptions and the pgstat entry for the partitioned table */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid,
- classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs auto analyze */
- av_enabled = (avopts ? avopts->enabled : true);
-
- if (av_enabled)
- {
- anl_scale_factor = (avopts && avopts->analyze_scale_factor >= 0)
- ? avopts->analyze_scale_factor
- : autovacuum_anl_scale;
-
- anl_base_thresh = (avopts && avopts->analyze_threshold >= 0)
- ? avopts->analyze_threshold
- : autovacuum_anl_thresh;
-
- if (PointerIsValid(tabentry))
- {
- reltuples = all_reltuples;
- anltuples = tabentry->changes_since_analyze;
- anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
-
- elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
- NameStr(classForm->relname),
- anltuples, anlthresh);
-
- doanalyze = (anltuples > anlthresh);
- }
- }
- }
+ /* ignore ANALYZE for toast tables */
+ if (classForm->relkind == RELKIND_TOASTVALUE)
+ doanalyze = false;
/* OK, it needs something done */
if (doanalyze || dovacuum)
@@ -3148,6 +2992,42 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(classForm != NULL);
AssertArg(OidIsValid(relid));
+ /*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
/*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
Attachments:
[text/plain] v3_amit_delta.patch (9.3K, ../../CA+HiwqG52Wy_Xds6X4UdbwK93S4FCD8Y0qWKN0V=NSyD=_Kk3A@mail.gmail.com/2-v3_amit_delta.patch)
download | inline diff:
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7d0a5ce30d..ca6996e448 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2089,103 +2089,19 @@ do_autovacuum(void)
continue;
}
- if (classForm->relkind == RELKIND_RELATION ||
- classForm->relkind == RELKIND_MATVIEW)
- {
- /* Fetch reloptions and the pgstat entry for this table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs vacuum or analyze */
- relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
-
- /* Relations that need work are added to table_oids */
- if (dovacuum || doanalyze)
- table_oids = lappend_oid(table_oids, relid);
- }
- else
- {
- /*
- * If the relation is a partitioned table, we check it using reltuples
- * added up childrens' and changes_since_analyze tracked by stats collector.
- * We check only auto analyze because partitioned tables don't need to vacuum.
- */
- List *tableOIDs;
- ListCell *lc;
- bool av_enabled;
- int anl_base_thresh;
- float4 all_reltuples = 0,
- anl_scale_factor,
- anlthresh,
- reltuples,
- anltuples;
-
- /* Find all members of inheritance set taking AccessShareLock */
- tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
-
- foreach(lc, tableOIDs)
- {
- Oid childOID = lfirst_oid(lc);
- HeapTuple childtuple;
- Form_pg_class childclassForm;
-
- /* Ignore the parent table */
- if (childOID == relid)
- continue;
-
- childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
- childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
-
- /* Skip foreign partitions */
- if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
- continue;
-
- /* Sum up the child's reltuples for its parent table */
- all_reltuples += childclassForm->reltuples;
- elog(NOTICE, "[parent:%s] child%s has %.0f tuples", NameStr(classForm->relname),NameStr(childclassForm->relname), childclassForm->reltuples);
- }
-
-
- /* Fetch reloptions and the pgstat entry for the partitioned table */
- relopts = extract_autovac_opts(tuple, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid,
- classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs auto analyze */
- av_enabled = (relopts ? relopts->enabled : true);
-
- if (av_enabled)
- {
- anl_scale_factor = (relopts && relopts->analyze_scale_factor >= 0)
- ? relopts->analyze_scale_factor
- : autovacuum_anl_scale;
-
- anl_base_thresh = (relopts && relopts->analyze_threshold >= 0)
- ? relopts->analyze_threshold
- : autovacuum_anl_thresh;
-
- elog(NOTICE, "[parent:%s] has %.0f tuples", NameStr(classForm->relname), all_reltuples);
- if (PointerIsValid(tabentry))
- {
- reltuples = all_reltuples;
- anltuples = tabentry->changes_since_analyze;
- anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
+ /* Fetch reloptions and the pgstat entry for this table */
+ relopts = extract_autovac_opts(tuple, pg_class_desc);
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
- elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
- NameStr(classForm->relname),
- anltuples, anlthresh);
+ /* Check if it needs vacuum or analyze */
+ relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- /* Determine if this table needs analyze. */
- doanalyze = (anltuples > anlthresh);
- }
- if (doanalyze)
- table_oids = lappend_oid(table_oids, relid);
- }
- }
+ /* 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
@@ -2880,105 +2796,33 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
return NULL;
classForm = (Form_pg_class) GETSTRUCT(classTup);
- if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ /*
+ * Get the applicable reloptions. If it is a TOAST table, try to get the
+ * main table reloptions if the toast table itself doesn't have.
+ */
+ avopts = extract_autovac_opts(classTup, pg_class_desc);
+ if (classForm->relkind == RELKIND_TOASTVALUE &&
+ avopts == NULL && table_toast_map != NULL)
{
- /*
- * Get the applicable reloptions. If it is a TOAST table, try to get the
- * main table reloptions if the toast table itself doesn't have.
- */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- if (classForm->relkind == RELKIND_TOASTVALUE &&
- avopts == NULL && table_toast_map != NULL)
- {
- av_relation *hentry;
- bool found;
-
- hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
- if (found && hentry->ar_hasrelopts)
- avopts = &hentry->ar_reloptions;
- }
-
- /* fetch the pgstat table entry */
- tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
- shared, dbentry);
-
- relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
- effective_multixact_freeze_max_age,
- &dovacuum, &doanalyze, &wraparound);
+ av_relation *hentry;
+ bool found;
- /* ignore ANALYZE for toast tables */
- if (classForm->relkind == RELKIND_TOASTVALUE)
- doanalyze = false;
+ hentry = hash_search(table_toast_map, &relid, HASH_FIND, &found);
+ if (found && hentry->ar_hasrelopts)
+ avopts = &hentry->ar_reloptions;
}
- else
- {
- List *tableOIDs;
- ListCell *lc;
- bool av_enabled;
- int anl_base_thresh;
- float4 all_reltuples,
- anl_scale_factor,
- anlthresh,
- reltuples,
- anltuples;
- /* Find all members of inheritance set taking AccessShareLock */
- tableOIDs = find_all_inheritors(relid, AccessShareLock, NULL);
+ /* fetch the pgstat table entry */
+ tabentry = get_pgstat_tabentry_relid(relid, classForm->relisshared,
+ shared, dbentry);
- foreach(lc, tableOIDs)
- {
- Oid childOID = lfirst_oid(lc);
- HeapTuple childtuple;
- Form_pg_class childclassForm;
+ relation_needs_vacanalyze(relid, avopts, classForm, tabentry,
+ effective_multixact_freeze_max_age,
+ &dovacuum, &doanalyze, &wraparound);
- /* Ignore the parent table */
- if (childOID == relid)
- continue;
-
- childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
- childclassForm = (Form_pg_class) GETSTRUCT(childtuple);
-
- /* Skip foreign partitions */
- if (childclassForm->relkind == RELKIND_FOREIGN_TABLE)
- continue;
-
- /* Sum up the child's reltuples for the partitioned table */
- all_reltuples += childclassForm->reltuples;
- }
-
- /* Fetch reloptions and the pgstat entry for the partitioned table */
- avopts = extract_autovac_opts(classTup, pg_class_desc);
- tabentry = get_pgstat_tabentry_relid(relid,
- classForm->relisshared,
- shared, dbentry);
-
- /* Check if it needs auto analyze */
- av_enabled = (avopts ? avopts->enabled : true);
-
- if (av_enabled)
- {
- anl_scale_factor = (avopts && avopts->analyze_scale_factor >= 0)
- ? avopts->analyze_scale_factor
- : autovacuum_anl_scale;
-
- anl_base_thresh = (avopts && avopts->analyze_threshold >= 0)
- ? avopts->analyze_threshold
- : autovacuum_anl_thresh;
-
- if (PointerIsValid(tabentry))
- {
- reltuples = all_reltuples;
- anltuples = tabentry->changes_since_analyze;
- anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples;
-
- elog(DEBUG3, "%s: anl: %.0f (threshold %.0f)",
- NameStr(classForm->relname),
- anltuples, anlthresh);
-
- doanalyze = (anltuples > anlthresh);
- }
- }
- }
+ /* ignore ANALYZE for toast tables */
+ if (classForm->relkind == RELKIND_TOASTVALUE)
+ doanalyze = false;
/* OK, it needs something done */
if (doanalyze || dovacuum)
@@ -3148,6 +2992,42 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(classForm != NULL);
AssertArg(OidIsValid(relid));
+ /*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
/*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-21 06:14 yuzuko <yuzukohosoya@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2020-02-21 06:14 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hello Amit-san,
Thanks for your comments.
> * White-space noise in the diff (space used where tab is expected);
> please check with git diff --check and fix.
Fixed it.
> * Names changes_tuples, m_changes_tuples should be changed_tuples and
> m_changed_tuples, respectively?
Yes, I modified it.
> * Did you intend to make it so that we now report *all* inherited
> stats to the stats collector, not just those for partitioned tables?
> IOW, do did you intend the new feature to also cover traditional
> inheritance parents? I am talking about the following diff:
>
I modified as follows to apply this feature to only declaretive partitioning.
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+ (va_cols == NIL));
> Having read the relevant diffs again, I think this could be done
> without duplicating code too much. You seem to have added the same
> logic in two places: do_autovacuum() and table_recheck_autovac().
> More importantly, part of the logic of relation_needs_vacanalyze() is
> duplicated in both of the aforementioned places, which I think is
> unnecessary and undesirable if you consider maintainability. I think
> we could just add the logic to compute reltuples for partitioned
> tables at the beginning of relation_needs_vacanalyze() and be done.
>
Yes, indeed. Partitioned tables don't need to vacuum so I added new
checking process for partitioned tables outside relation_needs_vacanalyze().
However, partitioned tables' tabentry->n_dead_tuples are always 0 so
dovacuum is always false. So I think that checking both auto vacuum
and analyze for partitioned tables doesn't matter. I merged v3_amit_delta.patch
into the new patch and found minor bug, partitioned table's reltuples is
overwritten with it's classForm->reltuples, so I fixed it.
Also, I think partitioned tables' changes_since_analyze should be reported
only when Autovacuum process. So I fixed it too.
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v4_autovacuum_on_partitioned_table.patch (15.8K, ../../CAKkQ509MqcJ7eG3DJD7vNMVTrSBV3=88V-KrV4zT0GTw1HfwTA@mail.gmail.com/2-v4_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 4a2b6f0dae..88c635f82f 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1310,8 +1310,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 79430d2b7b..20183a96a4 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -227,7 +227,7 @@ static relopt_int intRelOpts[] =
{
"autovacuum_analyze_threshold",
"Minimum number of tuple inserts, updates or deletes prior to analyze",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0, INT_MAX
@@ -379,7 +379,7 @@ static relopt_real realRelOpts[] =
{
"autovacuum_analyze_scale_factor",
"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0.0, 100.0
@@ -1586,13 +1586,12 @@ build_reloptions(Datum reloptions, bool validate,
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+
/*
- * There are no options for partitioned tables yet, but this is able to do
- * some validation.
+ * autovacuum_enabled, autovacuum_analyze_threshold and
+ * autovacuum_analyze_scale_factor are supported for partitioned tables.
*/
- return (bytea *) build_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ return default_reloptions(reloptions, validate, RELOPT_KIND_PARTITIONED);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f681aafcf9..161abb6450 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -584,7 +584,7 @@ CREATE VIEW pg_stat_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_xact_all_tables AS
@@ -604,7 +604,7 @@ CREATE VIEW pg_stat_xact_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_sys_tables AS
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c4420ddd7f..bbfdf75dbe 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -655,15 +655,15 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ 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 */
if (!(params->options & VACOPT_VACUUM))
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6d1f28c327..aabb1903de 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2031,11 +2032,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2058,7 +2059,8 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
relid = classForm->oid;
@@ -2720,6 +2722,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2990,6 +2993,42 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(OidIsValid(relid));
/*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
+ /*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
* table), or the autovacuum GUC variables.
@@ -3056,7 +3095,8 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ reltuples = classForm->reltuples;
vactuples = tabentry->n_dead_tuples;
anltuples = tabentry->changes_since_analyze;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 59dc4f31ab..ced2599050 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/partition.h"
#include "catalog/pg_database.h"
#include "catalog/pg_proc.h"
#include "common/ip.h"
@@ -67,6 +68,7 @@
#include "utils/ps_status.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/timestamp.h"
/* ----------
@@ -322,6 +324,7 @@ static void pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, in
static void pgstat_recv_autovac(PgStat_MsgAutovacStart *msg, int len);
static void pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len);
static void pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len);
+static void pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *msg, int len);
static void pgstat_recv_archiver(PgStat_MsgArchiver *msg, int len);
static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
@@ -1463,6 +1466,33 @@ pgstat_report_analyze(Relation rel,
deadtuples = Max(deadtuples, 0);
}
+ /*
+ * If the table is a leaf partition, tell the stats collector its parent's
+ * changes_since_analyze for auto analyze
+ */
+ if (IsAutoVacuumWorkerProcess() &&
+ rel->rd_rel->relispartition &&
+ !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
+ {
+ Oid parentoid;
+ Relation parentrel;
+ PgStat_StatDBEntry *dbentry;
+ PgStat_StatTabEntry *tabentry;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Fetch the pgstat for this table */
+ dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
+ tabentry = pgstat_get_tab_entry(dbentry, RelationGetRelid(rel), true);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, tabentry->changes_since_analyze);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE);
msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
msg.m_tableoid = RelationGetRelid(rel);
@@ -1475,6 +1505,49 @@ pgstat_report_analyze(Relation rel,
}
/* --------
+ * pgstat_report_partanalyze() -
+ *
+ * Tell the collector about the parent table of which partition just analyzed.
+ *
+ * Caller must provide a child's changes_since_analyze as a parents.
+ * --------
+ */
+void
+pgstat_report_partanalyze(Relation rel, PgStat_Counter changed_tuples)
+{
+ PgStat_MsgPartAnalyze msg;
+
+ if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+ return;
+
+ /*
+ * If the partitioned table is also a partition, tell the stats collector
+ * its parent's changes_since_analyze for auto analyze
+ */
+ if (rel->rd_rel->relispartition)
+ {
+ Oid parentoid;
+ Relation parentrel;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, changed_tuples);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
+ pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_PARTANALYZE);
+ msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
+ msg.m_tableoid = RelationGetRelid(rel);
+ msg.m_changed_tuples = changed_tuples;
+ pgstat_send(&msg, sizeof(msg));
+}
+
+
+/* --------
* pgstat_report_recovery_conflict() -
*
* Tell the collector about a Hot Standby recovery conflict.
@@ -1749,6 +1822,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!(relkind == RELKIND_RELATION ||
relkind == RELKIND_MATVIEW ||
+ relkind == RELKIND_PARTITIONED_TABLE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_SEQUENCE))
@@ -4592,6 +4666,10 @@ PgstatCollectorMain(int argc, char *argv[])
pgstat_recv_analyze(&msg.msg_analyze, len);
break;
+ case PGSTAT_MTYPE_PARTANALYZE:
+ pgstat_recv_partanalyze(&msg.msg_partanalyze, len);
+ break;
+
case PGSTAT_MTYPE_ARCHIVER:
pgstat_recv_archiver(&msg.msg_archiver, len);
break;
@@ -6239,6 +6317,18 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
}
}
+static void
+pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *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);
+
+ tabentry->changes_since_analyze += msg->m_changed_tuples;
+}
/* ----------
* pgstat_recv_archiver() -
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 3a65a51696..9bb872b171 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -57,6 +57,7 @@ typedef enum StatMsgType
PGSTAT_MTYPE_AUTOVAC_START,
PGSTAT_MTYPE_VACUUM,
PGSTAT_MTYPE_ANALYZE,
+ PGSTAT_MTYPE_PARTANALYZE,
PGSTAT_MTYPE_ARCHIVER,
PGSTAT_MTYPE_BGWRITER,
PGSTAT_MTYPE_FUNCSTAT,
@@ -389,6 +390,18 @@ typedef struct PgStat_MsgAnalyze
PgStat_Counter m_dead_tuples;
} PgStat_MsgAnalyze;
+/* ----------
+ * PgStat_MsgPartAnalyze Sent by the backend or autovacuum daemon
+ * after ANALYZE for partitioned tables
+ * ----------
+ */
+typedef struct PgStat_MsgPartAnalyze
+{
+ PgStat_MsgHdr m_hdr;
+ Oid m_databaseid;
+ Oid m_tableoid;
+ PgStat_Counter m_changed_tuples;
+} PgStat_MsgPartAnalyze;
/* ----------
* PgStat_MsgArchiver Sent by the archiver to update statistics.
@@ -562,6 +575,7 @@ typedef union PgStat_Msg
PgStat_MsgAutovacStart msg_autovacuum_start;
PgStat_MsgVacuum msg_vacuum;
PgStat_MsgAnalyze msg_analyze;
+ PgStat_MsgPartAnalyze msg_partanalyze;
PgStat_MsgArchiver msg_archiver;
PgStat_MsgBgWriter msg_bgwriter;
PgStat_MsgFuncstat msg_funcstat;
@@ -1267,7 +1281,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_partanalyze(Relation rel, PgStat_Counter changed_tuples);
extern void pgstat_report_recovery_conflict(int reason);
extern void pgstat_report_deadlock(void);
extern void pgstat_report_checksum_failures_in_db(Oid dboid, int failurecount);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 634f8256f7..7e9f6de9cb 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1789,7 +1789,7 @@ pg_stat_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_archiver| SELECT s.archived_count,
s.last_archived_wal,
@@ -2117,7 +2117,7 @@ pg_stat_xact_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_xact_sys_tables| SELECT pg_stat_xact_all_tables.relid,
pg_stat_xact_all_tables.schemaname,
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-21 07:47 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Masahiko Sawada @ 2020-02-21 07:47 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Fri, 21 Feb 2020 at 15:14, yuzuko <yuzukohosoya@gmail.com> wrote:
>
> Hello Amit-san,
>
> Thanks for your comments.
>
> > * White-space noise in the diff (space used where tab is expected);
> > please check with git diff --check and fix.
> Fixed it.
>
> > * Names changes_tuples, m_changes_tuples should be changed_tuples and
> > m_changed_tuples, respectively?
> Yes, I modified it.
>
> > * Did you intend to make it so that we now report *all* inherited
> > stats to the stats collector, not just those for partitioned tables?
> > IOW, do did you intend the new feature to also cover traditional
> > inheritance parents? I am talking about the following diff:
> >
> I modified as follows to apply this feature to only declaretive partitioning.
>
> - if (!inh)
> - pgstat_report_analyze(onerel, totalrows, totaldeadrows,
> - (va_cols == NIL));
> + if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
> + pgstat_report_analyze(onerel, totalrows, totaldeadrows,
> + (va_cols == NIL));
>
>
> > Having read the relevant diffs again, I think this could be done
> > without duplicating code too much. You seem to have added the same
> > logic in two places: do_autovacuum() and table_recheck_autovac().
> > More importantly, part of the logic of relation_needs_vacanalyze() is
> > duplicated in both of the aforementioned places, which I think is
> > unnecessary and undesirable if you consider maintainability. I think
> > we could just add the logic to compute reltuples for partitioned
> > tables at the beginning of relation_needs_vacanalyze() and be done.
> >
> Yes, indeed. Partitioned tables don't need to vacuum so I added new
> checking process for partitioned tables outside relation_needs_vacanalyze().
> However, partitioned tables' tabentry->n_dead_tuples are always 0 so
> dovacuum is always false. So I think that checking both auto vacuum
> and analyze for partitioned tables doesn't matter. I merged v3_amit_delta.patch
> into the new patch and found minor bug, partitioned table's reltuples is
> overwritten with it's classForm->reltuples, so I fixed it.
>
> Also, I think partitioned tables' changes_since_analyze should be reported
> only when Autovacuum process. So I fixed it too.
Thank you for updating the patch. I tested v4 patch.
After analyze or autoanalyze on partitioned table n_live_tup and
n_dead_tup are updated. However, TRUNCATE and VACUUM on the
partitioned table don't change these values until invoking analyze or
autoanalyze whereas in normal tables these values are reset or
changed. For example, with your patch:
* Before
relname | n_live_tup | n_dead_tup | n_mod_since_analyze
---------+------------+------------+---------------------
c1 | 11 | 0 | 0
c2 | 11 | 0 | 0
c3 | 11 | 0 | 0
c4 | 11 | 0 | 0
c5 | 11 | 0 | 0
parent | 55 | 0 | 0
(6 rows)
* After 'TRUNCATE parent'
relname | n_live_tup | n_dead_tup | n_mod_since_analyze
---------+------------+------------+---------------------
c1 | 0 | 0 | 0
c2 | 0 | 0 | 0
c3 | 0 | 0 | 0
c4 | 0 | 0 | 0
c5 | 0 | 0 | 0
parent | 55 | 0 | 0
(6 rows)
* Before
relname | n_live_tup | n_dead_tup | n_mod_since_analyze
---------+------------+------------+---------------------
c1 | 0 | 11 | 0
c2 | 0 | 11 | 0
c3 | 0 | 11 | 0
c4 | 0 | 11 | 0
c5 | 0 | 11 | 0
parent | 0 | 55 | 0
(6 rows)
* After 'VACUUM parent'
relname | n_live_tup | n_dead_tup | n_mod_since_analyze
---------+------------+------------+---------------------
c1 | 0 | 0 | 0
c2 | 0 | 0 | 0
c3 | 0 | 0 | 0
c4 | 0 | 0 | 0
c5 | 0 | 0 | 0
parent | 0 | 55 | 0
(6 rows)
We can make it work correctly but I think perhaps we can skip updating
statistics values of partitioned tables other than n_mod_since_analyze
as the first step. Because if we support also n_live_tup and
n_dead_tup, user might get confused that other statistics values such
as seq_scan, seq_tup_read however are not supported.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-21 08:35 Amit Langote <amitlangote09@gmail.com>
parent: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-21 08:35 UTC (permalink / raw)
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; +Cc: yuzuko <yuzukohosoya@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Fri, Feb 21, 2020 at 4:47 PM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
> Thank you for updating the patch. I tested v4 patch.
>
> After analyze or autoanalyze on partitioned table n_live_tup and
> n_dead_tup are updated. However, TRUNCATE and VACUUM on the
> partitioned table don't change these values until invoking analyze or
> autoanalyze whereas in normal tables these values are reset or
> changed. For example, with your patch:
>
> * Before
> relname | n_live_tup | n_dead_tup | n_mod_since_analyze
> ---------+------------+------------+---------------------
> c1 | 11 | 0 | 0
> c2 | 11 | 0 | 0
> c3 | 11 | 0 | 0
> c4 | 11 | 0 | 0
> c5 | 11 | 0 | 0
> parent | 55 | 0 | 0
> (6 rows)
>
> * After 'TRUNCATE parent'
> relname | n_live_tup | n_dead_tup | n_mod_since_analyze
> ---------+------------+------------+---------------------
> c1 | 0 | 0 | 0
> c2 | 0 | 0 | 0
> c3 | 0 | 0 | 0
> c4 | 0 | 0 | 0
> c5 | 0 | 0 | 0
> parent | 55 | 0 | 0
> (6 rows)
>
> * Before
> relname | n_live_tup | n_dead_tup | n_mod_since_analyze
> ---------+------------+------------+---------------------
> c1 | 0 | 11 | 0
> c2 | 0 | 11 | 0
> c3 | 0 | 11 | 0
> c4 | 0 | 11 | 0
> c5 | 0 | 11 | 0
> parent | 0 | 55 | 0
> (6 rows)
>
> * After 'VACUUM parent'
> relname | n_live_tup | n_dead_tup | n_mod_since_analyze
> ---------+------------+------------+---------------------
> c1 | 0 | 0 | 0
> c2 | 0 | 0 | 0
> c3 | 0 | 0 | 0
> c4 | 0 | 0 | 0
> c5 | 0 | 0 | 0
> parent | 0 | 55 | 0
> (6 rows)
>
> We can make it work correctly but I think perhaps we can skip updating
> statistics values of partitioned tables other than n_mod_since_analyze
> as the first step. Because if we support also n_live_tup and
> n_dead_tup, user might get confused that other statistics values such
> as seq_scan, seq_tup_read however are not supported.
+1, that makes sense.
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-26 02:32 yuzuko <yuzukohosoya@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 3 replies; 28+ messages in thread
From: yuzuko @ 2020-02-26 02:32 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hi,
Thanks for reviewing the patch.
> > We can make it work correctly but I think perhaps we can skip updating
> > statistics values of partitioned tables other than n_mod_since_analyze
> > as the first step. Because if we support also n_live_tup and
> > n_dead_tup, user might get confused that other statistics values such
> > as seq_scan, seq_tup_read however are not supported.
>
> +1, that makes sense.
>
Yes, Indeed. I modified it not to update statistics other than
n_mod_since_analyze.
Attach the v5 patch. In this patch, pgstat_report_analyze() always reports 0 as
msg.m_live_tuples and m_dead_tuples when the relation is partitioned.
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v5_autovacuum_on_partitioned_table.patch (16.2K, ../../CAKkQ509Gn8bOEZNmcudBYj=g97qG4PcNC2pbGxC4Ci_pPtQrEQ@mail.gmail.com/2-v5_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 4a2b6f0dae..88c635f82f 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1310,8 +1310,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 79430d2b7b..20183a96a4 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -227,7 +227,7 @@ static relopt_int intRelOpts[] =
{
"autovacuum_analyze_threshold",
"Minimum number of tuple inserts, updates or deletes prior to analyze",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0, INT_MAX
@@ -379,7 +379,7 @@ static relopt_real realRelOpts[] =
{
"autovacuum_analyze_scale_factor",
"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0.0, 100.0
@@ -1586,13 +1586,12 @@ build_reloptions(Datum reloptions, bool validate,
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+
/*
- * There are no options for partitioned tables yet, but this is able to do
- * some validation.
+ * autovacuum_enabled, autovacuum_analyze_threshold and
+ * autovacuum_analyze_scale_factor are supported for partitioned tables.
*/
- return (bytea *) build_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ return default_reloptions(reloptions, validate, RELOPT_KIND_PARTITIONED);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f681aafcf9..161abb6450 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -584,7 +584,7 @@ CREATE VIEW pg_stat_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_xact_all_tables AS
@@ -604,7 +604,7 @@ CREATE VIEW pg_stat_xact_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_sys_tables AS
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c4420ddd7f..bbfdf75dbe 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -655,15 +655,15 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ 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 */
if (!(params->options & VACOPT_VACUUM))
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6d1f28c327..aabb1903de 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2031,11 +2032,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2058,7 +2059,8 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
relid = classForm->oid;
@@ -2720,6 +2722,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2990,6 +2993,42 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(OidIsValid(relid));
/*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ /* Ignore the parent table */
+ if (childOID == relid)
+ continue;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
+ /*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
* table), or the autovacuum GUC variables.
@@ -3056,7 +3095,8 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ reltuples = classForm->reltuples;
vactuples = tabentry->n_dead_tuples;
anltuples = tabentry->changes_since_analyze;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 59dc4f31ab..3071487c47 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/partition.h"
#include "catalog/pg_database.h"
#include "catalog/pg_proc.h"
#include "common/ip.h"
@@ -67,6 +68,7 @@
#include "utils/ps_status.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/timestamp.h"
/* ----------
@@ -322,6 +324,7 @@ static void pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, in
static void pgstat_recv_autovac(PgStat_MsgAutovacStart *msg, int len);
static void pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len);
static void pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len);
+static void pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *msg, int len);
static void pgstat_recv_archiver(PgStat_MsgArchiver *msg, int len);
static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
@@ -1463,17 +1466,91 @@ pgstat_report_analyze(Relation rel,
deadtuples = Max(deadtuples, 0);
}
+ /*
+ * If the table is a leaf partition, tell the stats collector its parent's
+ * changes_since_analyze for auto analyze
+ */
+ if (IsAutoVacuumWorkerProcess() &&
+ rel->rd_rel->relispartition &&
+ !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
+ {
+ Oid parentoid;
+ Relation parentrel;
+ PgStat_StatDBEntry *dbentry;
+ PgStat_StatTabEntry *tabentry;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Fetch the pgstat for this table */
+ dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
+ tabentry = pgstat_get_tab_entry(dbentry, RelationGetRelid(rel), true);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, tabentry->changes_since_analyze);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE);
msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
msg.m_tableoid = RelationGetRelid(rel);
msg.m_autovacuum = IsAutoVacuumWorkerProcess();
msg.m_resetcounter = resetcounter;
msg.m_analyzetime = GetCurrentTimestamp();
- msg.m_live_tuples = livetuples;
- msg.m_dead_tuples = deadtuples;
+ msg.m_live_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : livetuples;
+ msg.m_dead_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : deadtuples;
+ pgstat_send(&msg, sizeof(msg));
+}
+
+/* --------
+ * pgstat_report_partanalyze() -
+ *
+ * Tell the collector about the parent table of which partition just analyzed.
+ *
+ * Caller must provide a child's changes_since_analyze as a parents.
+ * --------
+ */
+void
+pgstat_report_partanalyze(Relation rel, PgStat_Counter changed_tuples)
+{
+ PgStat_MsgPartAnalyze msg;
+
+ if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+ return;
+
+ /*
+ * If the partitioned table is also a partition, tell the stats collector
+ * its parent's changes_since_analyze for auto analyze
+ */
+ if (rel->rd_rel->relispartition)
+ {
+ Oid parentoid;
+ Relation parentrel;
+
+ /* Get its parent table's Oid and relation */
+ parentoid = get_partition_parent(RelationGetRelid(rel));
+ parentrel = table_open(parentoid, AccessShareLock);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partanalyze(parentrel, changed_tuples);
+
+ table_close(parentrel, AccessShareLock);
+ }
+
+ pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_PARTANALYZE);
+ msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
+ msg.m_tableoid = RelationGetRelid(rel);
+ msg.m_changed_tuples = changed_tuples;
pgstat_send(&msg, sizeof(msg));
}
+
/* --------
* pgstat_report_recovery_conflict() -
*
@@ -1749,6 +1826,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!(relkind == RELKIND_RELATION ||
relkind == RELKIND_MATVIEW ||
+ relkind == RELKIND_PARTITIONED_TABLE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_SEQUENCE))
@@ -4592,6 +4670,10 @@ PgstatCollectorMain(int argc, char *argv[])
pgstat_recv_analyze(&msg.msg_analyze, len);
break;
+ case PGSTAT_MTYPE_PARTANALYZE:
+ pgstat_recv_partanalyze(&msg.msg_partanalyze, len);
+ break;
+
case PGSTAT_MTYPE_ARCHIVER:
pgstat_recv_archiver(&msg.msg_archiver, len);
break;
@@ -6239,6 +6321,18 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
}
}
+static void
+pgstat_recv_partanalyze(PgStat_MsgPartAnalyze *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);
+
+ tabentry->changes_since_analyze += msg->m_changed_tuples;
+}
/* ----------
* pgstat_recv_archiver() -
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 3a65a51696..9bb872b171 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -57,6 +57,7 @@ typedef enum StatMsgType
PGSTAT_MTYPE_AUTOVAC_START,
PGSTAT_MTYPE_VACUUM,
PGSTAT_MTYPE_ANALYZE,
+ PGSTAT_MTYPE_PARTANALYZE,
PGSTAT_MTYPE_ARCHIVER,
PGSTAT_MTYPE_BGWRITER,
PGSTAT_MTYPE_FUNCSTAT,
@@ -389,6 +390,18 @@ typedef struct PgStat_MsgAnalyze
PgStat_Counter m_dead_tuples;
} PgStat_MsgAnalyze;
+/* ----------
+ * PgStat_MsgPartAnalyze Sent by the backend or autovacuum daemon
+ * after ANALYZE for partitioned tables
+ * ----------
+ */
+typedef struct PgStat_MsgPartAnalyze
+{
+ PgStat_MsgHdr m_hdr;
+ Oid m_databaseid;
+ Oid m_tableoid;
+ PgStat_Counter m_changed_tuples;
+} PgStat_MsgPartAnalyze;
/* ----------
* PgStat_MsgArchiver Sent by the archiver to update statistics.
@@ -562,6 +575,7 @@ typedef union PgStat_Msg
PgStat_MsgAutovacStart msg_autovacuum_start;
PgStat_MsgVacuum msg_vacuum;
PgStat_MsgAnalyze msg_analyze;
+ PgStat_MsgPartAnalyze msg_partanalyze;
PgStat_MsgArchiver msg_archiver;
PgStat_MsgBgWriter msg_bgwriter;
PgStat_MsgFuncstat msg_funcstat;
@@ -1267,7 +1281,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_partanalyze(Relation rel, PgStat_Counter changed_tuples);
extern void pgstat_report_recovery_conflict(int reason);
extern void pgstat_report_deadlock(void);
extern void pgstat_report_checksum_failures_in_db(Oid dboid, int failurecount);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 634f8256f7..7e9f6de9cb 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1789,7 +1789,7 @@ pg_stat_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_archiver| SELECT s.archived_count,
s.last_archived_wal,
@@ -2117,7 +2117,7 @@ pg_stat_xact_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_xact_sys_tables| SELECT pg_stat_xact_all_tables.relid,
pg_stat_xact_all_tables.schemaname,
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-28 02:02 Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
parent: yuzuko <yuzukohosoya@gmail.com>
2 siblings, 0 replies; 28+ messages in thread
From: Masahiko Sawada @ 2020-02-28 02:02 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
On Wed, 26 Feb 2020 at 11:33, yuzuko <yuzukohosoya@gmail.com> wrote:
>
> Hi,
>
> Thanks for reviewing the patch.
>
> > > We can make it work correctly but I think perhaps we can skip updating
> > > statistics values of partitioned tables other than n_mod_since_analyze
> > > as the first step. Because if we support also n_live_tup and
> > > n_dead_tup, user might get confused that other statistics values such
> > > as seq_scan, seq_tup_read however are not supported.
> >
> > +1, that makes sense.
> >
> Yes, Indeed. I modified it not to update statistics other than
> n_mod_since_analyze.
> Attach the v5 patch. In this patch, pgstat_report_analyze() always reports 0 as
> msg.m_live_tuples and m_dead_tuples when the relation is partitioned.
>
Thank you for updating the patch. I'll look at it. I'd recommend to
register this patch to the next commit fest so at not to forget.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-28 02:25 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: yuzuko <yuzukohosoya@gmail.com>
2 siblings, 1 reply; 28+ messages in thread
From: Alvaro Herrera @ 2020-02-28 02:25 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: 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>
Hello Yuzuko,
> + * Report ANALYZE to the stats collector, too. If the table is a
> + * partition, report changes_since_analyze of its parent because
> + * autovacuum process for partitioned tables needs it. Reset the
> + * changes_since_analyze counter only if we analyzed all columns;
> + * otherwise, there is still work for auto-analyze to do.
> */
> - if (!inh)
> - pgstat_report_analyze(onerel, totalrows, totaldeadrows,
> - (va_cols == NIL));
> + if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
> + pgstat_report_analyze(onerel, totalrows, totaldeadrows,
> + (va_cols == NIL));
Hmm, I think the comment has a bug: it says "report ... of its parent"
but the report is of the same rel. (The pgstat_report_analyze line is
mis-indented also).
> /*
> + * If the relation is a partitioned table, we must add up children's
> + * reltuples.
> + */
> + if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
> + {
> + List *children;
> + ListCell *lc;
> +
> + reltuples = 0;
> +
> + /* Find all members of inheritance set taking AccessShareLock */
> + children = find_all_inheritors(relid, AccessShareLock, NULL);
> +
> + foreach(lc, children)
> + {
> + Oid childOID = lfirst_oid(lc);
> + HeapTuple childtuple;
> + Form_pg_class childclass;
> +
> + /* Ignore the parent table */
> + if (childOID == relid)
> + continue;
I think this loop counts partitioned partitions multiple times, because
we add up reltuples for all levels, no? (If I'm wrong, that is, if
a partitioned rel does not have reltuples, then why skip the parent?)
> + /*
> + * If the table is a leaf partition, tell the stats collector its parent's
> + * changes_since_analyze for auto analyze
> + */
> + if (IsAutoVacuumWorkerProcess() &&
> + rel->rd_rel->relispartition &&
> + !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
I'm not sure I understand why we do this only on autovac. Why not all
analyzes?
> + {
> + Oid parentoid;
> + Relation parentrel;
> + PgStat_StatDBEntry *dbentry;
> + PgStat_StatTabEntry *tabentry;
> +
> + /* Get its parent table's Oid and relation */
> + parentoid = get_partition_parent(RelationGetRelid(rel));
> + parentrel = table_open(parentoid, AccessShareLock);
Climbing up the partitioning hierarchy acquiring locks on ancestor
relations opens up for deadlocks. It's better to avoid that. (As a
test, you could try what happens if you lock the topmost relation with
access-exclusive and leave a transaction open, then have autoanalyze
run). At the same time, I wonder if it's sensible to move one level up
here, and also have pgstat_report_partanalyze move more levels up.
> + * pgstat_report_partanalyze() -
> + *
> + * Tell the collector about the parent table of which partition just analyzed.
> + *
> + * Caller must provide a child's changes_since_analyze as a parents.
I'm not sure what the last line is trying to say.
Thanks,
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-28 02:31 Amit Langote <amitlangote09@gmail.com>
parent: yuzuko <yuzukohosoya@gmail.com>
2 siblings, 0 replies; 28+ messages in thread
From: Amit Langote @ 2020-02-28 02:31 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>; Laurenz Albe <laurenz.albe@cybertec.at>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Greg Stark <stark@mit.edu>
Hosoya-san,
Thanks for the new patch.
On Wed, Feb 26, 2020 at 11:33 AM yuzuko <yuzukohosoya@gmail.com> wrote:
> Attach the v5 patch. In this patch, pgstat_report_analyze() always reports 0 as
> msg.m_live_tuples and m_dead_tuples when the relation is partitioned.
Some comments:
+ * PgStat_MsgPartAnalyze Sent by the backend or autovacuum daemon
+ * after ANALYZE for partitioned tables
Looking at the way this message is used, it does not seem to be an
"analyze" message and also it's not sent "after ANALYZE of partitioned
tables", but really after ANALYZE of leaf partitions. Analyze (for
both partitioned tables and leaf partitions) is reported as a
PgStat_MsgAnalyze message as before. It seems that
PgStat_MsgPartAnalyze is only sent to update a leaf partition's
parent's (and recursively any grandparents') changes_since_analyze
counters, so maybe we should find a different name for it. Maybe,
PgStat_MsgPartChanges and accordingly the message type enum value.
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
The new comment says "partitions", which we typically use to refer to
a child table, but this comment really talks about parent tables. Old
comment says we don't report "inherited stats", presumably because
stats collector lacks the infrastructure to distinguish a table's
inherited stats and own stats, at least in the case of traditional
inheritance. With this patch, we are making an exception for
partitioned tables, because we are also teaching the stats collector
to maintain at least changes_since_analyze for them that accumulates
counts of changed tuples from partitions.
It seems Alvaro already reported some of the other issues I had with
the patch, such as why partanalyze messages are only sent from a
autovacuum worker.
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-02-28 03:57 Amit Langote <amitlangote09@gmail.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 28+ messages in thread
From: Amit Langote @ 2020-02-28 03:57 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: yuzuko <yuzukohosoya@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>
On Fri, Feb 28, 2020 at 11:25 AM Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> > /*
> > + * If the relation is a partitioned table, we must add up children's
> > + * reltuples.
> > + */
> > + if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
> > + {
> > + List *children;
> > + ListCell *lc;
> > +
> > + reltuples = 0;
> > +
> > + /* Find all members of inheritance set taking AccessShareLock */
> > + children = find_all_inheritors(relid, AccessShareLock, NULL);
> > +
> > + foreach(lc, children)
> > + {
> > + Oid childOID = lfirst_oid(lc);
> > + HeapTuple childtuple;
> > + Form_pg_class childclass;
> > +
> > + /* Ignore the parent table */
> > + if (childOID == relid)
> > + continue;
>
> I think this loop counts partitioned partitions multiple times, because
> we add up reltuples for all levels, no? (If I'm wrong, that is, if
> a partitioned rel does not have reltuples, then why skip the parent?)
+1, no need to skip partitioned tables here a their reltuples is always 0.
> > + /*
> > + * If the table is a leaf partition, tell the stats collector its parent's
> > + * changes_since_analyze for auto analyze
Maybe write:
For a leaf partition, add its current changes_since_analyze into its
ancestors' counts. This must be done before sending the ANALYZE
message as it resets the partition's changes_since_analyze counter.
> > + */
> > + if (IsAutoVacuumWorkerProcess() &&
> > + rel->rd_rel->relispartition &&
> > + !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
>
> I'm not sure I understand why we do this only on autovac. Why not all
> analyzes?
+1. If there is a reason, it should at least be documented in the
comment above.
> > + {
> > + Oid parentoid;
> > + Relation parentrel;
> > + PgStat_StatDBEntry *dbentry;
> > + PgStat_StatTabEntry *tabentry;
> > +
> > + /* Get its parent table's Oid and relation */
> > + parentoid = get_partition_parent(RelationGetRelid(rel));
> > + parentrel = table_open(parentoid, AccessShareLock);
>
> Climbing up the partitioning hierarchy acquiring locks on ancestor
> relations opens up for deadlocks. It's better to avoid that. (As a
> test, you could try what happens if you lock the topmost relation with
> access-exclusive and leave a transaction open, then have autoanalyze
> run). At the same time, I wonder if it's sensible to move one level up
> here, and also have pgstat_report_partanalyze move more levels up.
Maybe fetch all ancestors here and process from the top. But as we'd
have locked the leaf partition long before we got here, maybe we
should lock ancestors even before we start analyzing the leaf
partition? AccessShareLock should be enough on the ancestors because
we're not actually analyzing them.
(It appears get_partition_ancestors() returns a list where the root
parent is the last element, so need to be careful with that.)
Thanks,
Amit
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-03-17 03:22 yuzuko <yuzukohosoya@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2020-03-17 03:22 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.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>
Hello,
Thank you for reviewing.
> > > + */
> > > + if (IsAutoVacuumWorkerProcess() &&
> > > + rel->rd_rel->relispartition &&
> > > + !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
> >
> > I'm not sure I understand why we do this only on autovac. Why not all
> > analyzes?
>
> +1. If there is a reason, it should at least be documented in the
> comment above.
>
When we analyze partitioned table by ANALYZE command,
all inheritors including partitioned table are analyzed
at the same time. In this case, if we call pgstat_report_partanalyze,
partitioned table's changes_since_analyze is updated
according to the number of analyzed tuples of partitions
as follows. But I think it should be 0.
\d+ p
Partitioned table "public.p"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
i | integer | | | | plain | |
Partition key: RANGE (i)
Partitions: p_1 FOR VALUES FROM (0) TO (100),
p_2 FOR VALUES FROM (100) TO (200)
insert into p select * from generate_series(0,199);
INSERT 0 200
(before analyze)
-[ RECORD 1 ]-------+------------------
relname | p
n_mod_since_analyze | 0
-[ RECORD 2 ]-------+------------------
relname | p_1
n_mod_since_analyze | 100
-[ RECORD 3 ]-------+------------------
relname | p_2
n_mod_since_analyze | 100
(after analyze)
-[ RECORD 1 ]-------+------------------
relname | p
n_mod_since_analyze | 200
-[ RECORD 2 ]-------+------------------
relname | p_1
n_mod_since_analyze | 0
-[ RECORD 3 ]-------+------------------
relname | p_2
n_mod_since_analyze | 0
I think if we analyze partition tree in order from leaf partitions
to root table, this problem can be fixed.
What do you think about it?
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-03-18 01:06 yuzuko <yuzukohosoya@gmail.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: yuzuko @ 2020-03-18 01:06 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Alvaro Herrera <alvherre@2ndquadrant.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>
Hello,
> > > > + */
> > > > + if (IsAutoVacuumWorkerProcess() &&
> > > > + rel->rd_rel->relispartition &&
> > > > + !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
> > >
> > > I'm not sure I understand why we do this only on autovac. Why not all
> > > analyzes?
> >
> > +1. If there is a reason, it should at least be documented in the
> > comment above.
> >
> When we analyze partitioned table by ANALYZE command,
> all inheritors including partitioned table are analyzed
> at the same time. In this case, if we call pgstat_report_partanalyze,
> partitioned table's changes_since_analyze is updated
> according to the number of analyzed tuples of partitions
> as follows. But I think it should be 0.
>
> \d+ p
> Partitioned table "public.p"
> Column | Type | Collation | Nullable | Default | Storage | Stats
> target | Description
> --------+---------+-----------+----------+---------+---------+--------------+-------------
> i | integer | | | | plain | |
> Partition key: RANGE (i)
> Partitions: p_1 FOR VALUES FROM (0) TO (100),
> p_2 FOR VALUES FROM (100) TO (200)
>
> insert into p select * from generate_series(0,199);
> INSERT 0 200
>
> (before analyze)
> -[ RECORD 1 ]-------+------------------
> relname | p
> n_mod_since_analyze | 0
> -[ RECORD 2 ]-------+------------------
> relname | p_1
> n_mod_since_analyze | 100
> -[ RECORD 3 ]-------+------------------
> relname | p_2
> n_mod_since_analyze | 100
>
> (after analyze)
> -[ RECORD 1 ]-------+------------------
> relname | p
> n_mod_since_analyze | 200
> -[ RECORD 2 ]-------+------------------
> relname | p_1
> n_mod_since_analyze | 0
> -[ RECORD 3 ]-------+------------------
> relname | p_2
> n_mod_since_analyze | 0
>
>
> I think if we analyze partition tree in order from leaf partitions
> to root table, this problem can be fixed.
> What do you think about it?
>
Attach the new patch fixes the above problem. Also, This patch
includes modifications accoring to all comments Alvaro and Amit
mentioned before in this thread.
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v6_autovacuum_on_partitioned_table.patch (20.6K, ../../CAKkQ508nu-YVDmoDef9riW5ApoOB0RG51S1ADuFG4_yvqRnSOQ@mail.gmail.com/2-v6_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 4a2b6f0dae..88c635f82f 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1310,8 +1310,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 79430d2b7b..20183a96a4 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -227,7 +227,7 @@ static relopt_int intRelOpts[] =
{
"autovacuum_analyze_threshold",
"Minimum number of tuple inserts, updates or deletes prior to analyze",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0, INT_MAX
@@ -379,7 +379,7 @@ static relopt_real realRelOpts[] =
{
"autovacuum_analyze_scale_factor",
"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0.0, 100.0
@@ -1586,13 +1586,12 @@ build_reloptions(Datum reloptions, bool validate,
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+
/*
- * There are no options for partitioned tables yet, but this is able to do
- * some validation.
+ * autovacuum_enabled, autovacuum_analyze_threshold and
+ * autovacuum_analyze_scale_factor are supported for partitioned tables.
*/
- return (bytea *) build_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ return default_reloptions(reloptions, validate, RELOPT_KIND_PARTITIONED);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f681aafcf9..161abb6450 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -584,7 +584,7 @@ CREATE VIEW pg_stat_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_xact_all_tables AS
@@ -604,7 +604,7 @@ CREATE VIEW pg_stat_xact_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_sys_tables AS
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c4420ddd7f..b6cbdf3471 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -655,13 +655,14 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do. Also,
+ * if the table is a leaf partition, we add its current
+ * changes_since_analyze into its ancestors' counts because
+ * autovacuum process for partitioned tables needs it.
*/
- if (!inh)
+ if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
pgstat_report_analyze(onerel, totalrows, totaldeadrows,
(va_cols == NIL));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index d625d17bf4..546c35652d 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -828,9 +828,9 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
* later.
*/
oldcontext = MemoryContextSwitchTo(vac_context);
- vacrels = lappend(vacrels, makeVacuumRelation(NULL,
- part_oid,
- vrel->va_cols));
+ vacrels = lcons(makeVacuumRelation(NULL, part_oid, vrel->va_cols),
+ vacrels);
+
MemoryContextSwitchTo(oldcontext);
}
}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 6d1f28c327..eed391f3cd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2031,11 +2032,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2058,7 +2059,8 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
relid = classForm->oid;
@@ -2720,6 +2722,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2990,6 +2993,38 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(OidIsValid(relid));
/*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
+ /*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
* table), or the autovacuum GUC variables.
@@ -3056,7 +3091,8 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ reltuples = classForm->reltuples;
vactuples = tabentry->n_dead_tuples;
anltuples = tabentry->changes_since_analyze;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 59dc4f31ab..d0c9e14403 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/partition.h"
#include "catalog/pg_database.h"
#include "catalog/pg_proc.h"
#include "common/ip.h"
@@ -67,6 +68,7 @@
#include "utils/ps_status.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/timestamp.h"
/* ----------
@@ -322,6 +324,7 @@ static void pgstat_recv_resetsinglecounter(PgStat_MsgResetsinglecounter *msg, in
static void pgstat_recv_autovac(PgStat_MsgAutovacStart *msg, int len);
static void pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len);
static void pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len);
+static void pgstat_recv_partchanges(PgStat_MsgPartChanges *msg, int len);
static void pgstat_recv_archiver(PgStat_MsgArchiver *msg, int len);
static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len);
static void pgstat_recv_funcstat(PgStat_MsgFuncstat *msg, int len);
@@ -1463,18 +1466,76 @@ pgstat_report_analyze(Relation rel,
deadtuples = Max(deadtuples, 0);
}
+ /*
+ * For a leaf partition, add its current changes_since_analyze
+ * into its ancestors' counts. This must be done before sending
+ * the ANALYZE message as it resets the partition's changes_since_analze
+ * counter.
+ */
+ if (rel->rd_rel->relispartition &&
+ !(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))
+
+ {
+ List *ancestors;
+ ListCell *lc;
+ PgStat_StatDBEntry *dbentry;
+ PgStat_StatTabEntry *tabentry;
+
+ /* Fetch the pgstat for this table */
+ dbentry = pgstat_fetch_stat_dbentry(MyDatabaseId);
+ tabentry = pgstat_get_tab_entry(dbentry, RelationGetRelid(rel), true);
+
+ /* Get its all ancestors */
+ ancestors = get_partition_ancestors(RelationGetRelid(rel));
+ foreach(lc, ancestors)
+ {
+ Oid parentOid = lfirst_oid(lc);
+ Relation parentrel = table_open(parentOid, AccessShareLock);
+
+ /* Report changes_since_analyze to the stats collector */
+ pgstat_report_partchanges(parentrel, tabentry->changes_since_analyze);
+
+ table_close(parentrel, AccessShareLock);
+ }
+ }
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ANALYZE);
msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
msg.m_tableoid = RelationGetRelid(rel);
msg.m_autovacuum = IsAutoVacuumWorkerProcess();
msg.m_resetcounter = resetcounter;
msg.m_analyzetime = GetCurrentTimestamp();
- msg.m_live_tuples = livetuples;
- msg.m_dead_tuples = deadtuples;
+ msg.m_live_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : livetuples;
+ msg.m_dead_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : deadtuples;
pgstat_send(&msg, sizeof(msg));
}
/* --------
+ * pgstat_report_partchanges() -
+ *
+ * Tell the collector about the parent table of which partition just analyzed.
+ * --------
+ */
+void
+pgstat_report_partchanges(Relation rel, PgStat_Counter changed_tuples)
+{
+ PgStat_MsgPartChanges msg;
+
+ if (pgStatSock == PGINVALID_SOCKET || !pgstat_track_counts)
+ return;
+
+ pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_PARTCHANGES);
+ msg.m_databaseid = rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId;
+ msg.m_tableoid = RelationGetRelid(rel);
+ msg.m_changed_tuples = changed_tuples;
+ pgstat_send(&msg, sizeof(msg));
+}
+
+
+/* --------
* pgstat_report_recovery_conflict() -
*
* Tell the collector about a Hot Standby recovery conflict.
@@ -1749,6 +1810,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!(relkind == RELKIND_RELATION ||
relkind == RELKIND_MATVIEW ||
+ relkind == RELKIND_PARTITIONED_TABLE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_SEQUENCE))
@@ -4592,6 +4654,10 @@ PgstatCollectorMain(int argc, char *argv[])
pgstat_recv_analyze(&msg.msg_analyze, len);
break;
+ case PGSTAT_MTYPE_PARTCHANGES:
+ pgstat_recv_partchanges(&msg.msg_partchanges, len);
+ break;
+
case PGSTAT_MTYPE_ARCHIVER:
pgstat_recv_archiver(&msg.msg_archiver, len);
break;
@@ -6239,6 +6305,18 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
}
}
+static void
+pgstat_recv_partchanges(PgStat_MsgPartChanges *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);
+
+ tabentry->changes_since_analyze += msg->m_changed_tuples;
+}
/* ----------
* pgstat_recv_archiver() -
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 3a65a51696..b979ed32d4 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -57,6 +57,7 @@ typedef enum StatMsgType
PGSTAT_MTYPE_AUTOVAC_START,
PGSTAT_MTYPE_VACUUM,
PGSTAT_MTYPE_ANALYZE,
+ PGSTAT_MTYPE_PARTCHANGES,
PGSTAT_MTYPE_ARCHIVER,
PGSTAT_MTYPE_BGWRITER,
PGSTAT_MTYPE_FUNCSTAT,
@@ -389,6 +390,18 @@ typedef struct PgStat_MsgAnalyze
PgStat_Counter m_dead_tuples;
} PgStat_MsgAnalyze;
+/* ----------
+ * PgStat_MsgPartChanges Sent by the autovacuum daemon
+ * after ANALYZE of leaf partitions
+ * ----------
+ */
+typedef struct PgStat_MsgPartChanges
+{
+ PgStat_MsgHdr m_hdr;
+ Oid m_databaseid;
+ Oid m_tableoid;
+ PgStat_Counter m_changed_tuples;
+} PgStat_MsgPartChanges;
/* ----------
* PgStat_MsgArchiver Sent by the archiver to update statistics.
@@ -562,6 +575,7 @@ typedef union PgStat_Msg
PgStat_MsgAutovacStart msg_autovacuum_start;
PgStat_MsgVacuum msg_vacuum;
PgStat_MsgAnalyze msg_analyze;
+ PgStat_MsgPartChanges msg_partchanges;
PgStat_MsgArchiver msg_archiver;
PgStat_MsgBgWriter msg_bgwriter;
PgStat_MsgFuncstat msg_funcstat;
@@ -1267,7 +1281,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_partchanges(Relation rel, PgStat_Counter changed_tuples);
extern void pgstat_report_recovery_conflict(int reason);
extern void pgstat_report_deadlock(void);
extern void pgstat_report_checksum_failures_in_db(Oid dboid, int failurecount);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 634f8256f7..7e9f6de9cb 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1789,7 +1789,7 @@ pg_stat_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_archiver| SELECT s.archived_count,
s.last_archived_wal,
@@ -2117,7 +2117,7 @@ pg_stat_xact_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_xact_sys_tables| SELECT pg_stat_xact_all_tables.relid,
pg_stat_xact_all_tables.schemaname,
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 0cfe28e63f..abf5be31ca 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -192,9 +192,9 @@ VACUUM (FULL) vacparted;
VACUUM (FREEZE) vacparted;
-- check behavior with duplicate column mentions
VACUUM ANALYZE vacparted(a,b,a);
-ERROR: column "a" of relation "vacparted" appears more than once
+ERROR: column "a" of relation "vacparted1" appears more than once
ANALYZE vacparted(a,b,b);
-ERROR: column "b" of relation "vacparted" appears more than once
+ERROR: column "b" of relation "vacparted1" appears more than once
-- multiple tables specified
VACUUM vaccluster, vactst;
VACUUM vacparted, does_not_exist;
@@ -213,7 +213,7 @@ ANALYZE vacparted (b), vactst;
ANALYZE vactst, does_not_exist, vacparted;
ERROR: relation "does_not_exist" does not exist
ANALYZE vactst (i), vacparted (does_not_exist);
-ERROR: column "does_not_exist" of relation "vacparted" does not exist
+ERROR: column "does_not_exist" of relation "vacparted1" does not exist
ANALYZE vactst, vactst;
BEGIN; -- ANALYZE behaves differently inside a transaction block
ANALYZE vactst, vactst;
@@ -287,24 +287,24 @@ WARNING: skipping "pg_authid" --- only superuser can vacuum it
-- independently.
VACUUM vacowned_parted;
WARNING: skipping "vacowned_parted" --- only table or database owner can vacuum it
-WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
+WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM vacowned_part2;
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
ANALYZE vacowned_parted;
WARNING: skipping "vacowned_parted" --- only table or database owner can analyze it
-WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
WARNING: skipping "vacowned_part2" --- only table or database owner can analyze it
+WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
ANALYZE vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
ANALYZE vacowned_part2;
WARNING: skipping "vacowned_part2" --- only table or database owner can analyze it
VACUUM (ANALYZE) vacowned_parted;
WARNING: skipping "vacowned_parted" --- only table or database owner can vacuum it
-WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
+WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM (ANALYZE) vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM (ANALYZE) vacowned_part2;
@@ -357,22 +357,22 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;
SET ROLE regress_vacuum;
VACUUM vacowned_parted;
-WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
+WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM vacowned_part2;
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
ANALYZE vacowned_parted;
-WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
WARNING: skipping "vacowned_part2" --- only table or database owner can analyze it
+WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
ANALYZE vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can analyze it
ANALYZE vacowned_part2;
WARNING: skipping "vacowned_part2" --- only table or database owner can analyze it
VACUUM (ANALYZE) vacowned_parted;
-WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
WARNING: skipping "vacowned_part2" --- only table or database owner can vacuum it
+WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM (ANALYZE) vacowned_part1;
WARNING: skipping "vacowned_part1" --- only table or database owner can vacuum it
VACUUM (ANALYZE) vacowned_part2;
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-03-18 01:49 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: yuzuko <yuzukohosoya@gmail.com>
0 siblings, 1 reply; 28+ messages in thread
From: Alvaro Herrera @ 2020-03-18 01:49 UTC (permalink / raw)
To: yuzuko <yuzukohosoya@gmail.com>; +Cc: 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>
On 2020-Mar-18, yuzuko wrote:
> > I think if we analyze partition tree in order from leaf partitions
> > to root table, this problem can be fixed.
> > What do you think about it?
>
> Attach the new patch fixes the above problem.
Thanks for the new version.
I'm confused about some error messages in the regression test when a
column is mentioned twice, that changed from mentioning the table named
in the vacuum command, to mentioning the first partition. Is that
because you changed an lappend() to lcons()? I think you do this so
that the counters accumulate for the topmost parent that will be
processed at the end. I'm not sure I like that too much ... I think
that needs more thought.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: Autovacuum on partitioned table
@ 2020-04-07 02:34 yuzuko <yuzukohosoya@gmail.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 28+ messages in thread
From: yuzuko @ 2020-04-07 02:34 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: 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>
Hi Alvaro,
Thank you for your comments.
> I'm confused about some error messages in the regression test when a
> column is mentioned twice, that changed from mentioning the table named
> in the vacuum command, to mentioning the first partition. Is that
> because you changed an lappend() to lcons()? I think you do this so
> that the counters accumulate for the topmost parent that will be
> processed at the end. I'm not sure I like that too much ... I think
> that needs more thought.
>
I couldn't come up with a solution that counts changes_since_analyze
precisely when analyzing partitioned trees by ANALYZE command based on
this approach (update all ancestor's changes_since_analyze according to the
number of analyzed tuples of leaf partitions).
So I tried another approach to run autovacuum on partitioned tables.
In this approach, all ancestors' changed_tuples are updated when commiting
transactions (at AtEOXact_PgStat) according to the number of inserted/updated/
deleted tuples of leaf partitions.
Attach the latest patch. What do you think?
--
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center
Attachments:
[application/octet-stream] v7_autovacuum_on_partitioned_table.patch (15.1K, ../../CAKkQ50_2HWfJJ7NYfmGvyNE9DYTdrANkG53RtXudVAwddh5ACg@mail.gmail.com/2-v7_autovacuum_on_partitioned_table.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 155866c7c8..046a397ac3 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1322,8 +1322,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
If a table parameter value is set and the
equivalent <literal>toast.</literal> parameter is not, the TOAST table
will use the table's parameter value.
- Specifying these parameters for partitioned tables is not supported,
- but you may specify them for individual leaf partitions.
</para>
<variablelist>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8ccc228a8c..35bc2e5bdb 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -108,7 +108,7 @@ static relopt_bool boolRelOpts[] =
{
"autovacuum_enabled",
"Enables autovacuum in this relation",
- RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+ RELOPT_KIND_HEAP | RELOPT_KIND_TOAST | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
true
@@ -246,7 +246,7 @@ static relopt_int intRelOpts[] =
{
"autovacuum_analyze_threshold",
"Minimum number of tuple inserts, updates or deletes prior to analyze",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0, INT_MAX
@@ -420,7 +420,7 @@ static relopt_real realRelOpts[] =
{
"autovacuum_analyze_scale_factor",
"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
- RELOPT_KIND_HEAP,
+ RELOPT_KIND_HEAP | RELOPT_KIND_PARTITIONED,
ShareUpdateExclusiveLock
},
-1, 0.0, 100.0
@@ -1961,13 +1961,12 @@ build_local_reloptions(local_relopts *relopts, Datum options, bool validate)
bytea *
partitioned_table_reloptions(Datum reloptions, bool validate)
{
+
/*
- * There are no options for partitioned tables yet, but this is able to do
- * some validation.
+ * autovacuum_enabled, autovacuum_analyze_threshold and
+ * autovacuum_analyze_scale_factor are supported for partitioned tables.
*/
- return (bytea *) build_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED,
- 0, NULL, 0);
+ return default_reloptions(reloptions, validate, RELOPT_KIND_PARTITIONED);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 813ea8bfc3..ffda2f45bf 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -585,7 +585,7 @@ CREATE VIEW pg_stat_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_xact_all_tables AS
@@ -605,7 +605,7 @@ CREATE VIEW pg_stat_xact_all_tables AS
FROM pg_class C LEFT JOIN
pg_index I ON C.oid = I.indrelid
LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
- WHERE C.relkind IN ('r', 't', 'm')
+ WHERE C.relkind IN ('r', 't', 'm', 'p')
GROUP BY C.oid, N.nspname, C.relname;
CREATE VIEW pg_stat_sys_tables AS
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 924ef37c81..de7f1c3bb1 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -655,13 +655,11 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, 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.
+ * Report ANALYZE to the stats collector, too. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
+ if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
pgstat_report_analyze(onerel, totalrows, totaldeadrows,
(va_cols == NIL));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 3a89f8fe1e..8b3cf85389 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -831,6 +831,7 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
vacrels = lappend(vacrels, makeVacuumRelation(NULL,
part_oid,
vrel->va_cols));
+
MemoryContextSwitchTo(oldcontext);
}
}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7e97ffab27..47905380fb 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -75,6 +75,7 @@
#include "catalog/dependency.h"
#include "catalog/namespace.h"
#include "catalog/pg_database.h"
+#include "catalog/pg_inherits.h"
#include "commands/dbcommands.h"
#include "commands/vacuum.h"
#include "lib/ilist.h"
@@ -2033,11 +2034,11 @@ do_autovacuum(void)
* Scan pg_class to determine which tables to vacuum.
*
* We do this in two passes: on the first one we collect the list of plain
- * relations and materialized views, and on the second one we collect
- * TOAST tables. The reason for doing the second 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.
+ * relations, materialized views and partitioned tables, and on the second
+ * one we collect TOAST tables. The reason for doing the second 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.
*
* We need to check TOAST tables separately because in cases with short,
* wide tables there might be proportionally much more activity in the
@@ -2060,7 +2061,8 @@ do_autovacuum(void)
bool wraparound;
if (classForm->relkind != RELKIND_RELATION &&
- classForm->relkind != RELKIND_MATVIEW)
+ classForm->relkind != RELKIND_MATVIEW &&
+ classForm->relkind != RELKIND_PARTITIONED_TABLE)
continue;
relid = classForm->oid;
@@ -2723,6 +2725,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
+ ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_PARTITIONED_TABLE ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
relopts = extractRelOptions(tup, pg_class_desc, NULL);
@@ -2997,6 +3000,38 @@ relation_needs_vacanalyze(Oid relid,
AssertArg(OidIsValid(relid));
/*
+ * If the relation is a partitioned table, we must add up children's
+ * reltuples.
+ */
+ if (classForm->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *children;
+ ListCell *lc;
+
+ reltuples = 0;
+
+ /* Find all members of inheritance set taking AccessShareLock */
+ children = find_all_inheritors(relid, AccessShareLock, NULL);
+
+ foreach(lc, children)
+ {
+ Oid childOID = lfirst_oid(lc);
+ HeapTuple childtuple;
+ Form_pg_class childclass;
+
+ childtuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(childOID));
+ childclass = (Form_pg_class) GETSTRUCT(childtuple);
+
+ /* Skip foreign partitions */
+ if (childclass->relkind == RELKIND_FOREIGN_TABLE)
+ continue;
+
+ /* Sum up the child's reltuples for its parent table */
+ reltuples += childclass->reltuples;
+ }
+ }
+
+ /*
* Determine vacuum/analyze equation parameters. We have two possible
* sources: the passed reloptions (which could be a main table or a toast
* table), or the autovacuum GUC variables.
@@ -3072,7 +3107,8 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- reltuples = classForm->reltuples;
+ if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
+ reltuples = classForm->reltuples;
vactuples = tabentry->n_dead_tuples;
instuples = tabentry->inserts_since_vacuum;
anltuples = tabentry->changes_since_analyze;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9ebde47dea..1740b50352 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -38,6 +38,7 @@
#include "access/transam.h"
#include "access/twophase_rmgr.h"
#include "access/xact.h"
+#include "catalog/partition.h"
#include "catalog/pg_database.h"
#include "catalog/pg_proc.h"
#include "common/ip.h"
@@ -1523,8 +1524,12 @@ pgstat_report_analyze(Relation rel,
msg.m_autovacuum = IsAutoVacuumWorkerProcess();
msg.m_resetcounter = resetcounter;
msg.m_analyzetime = GetCurrentTimestamp();
- msg.m_live_tuples = livetuples;
- msg.m_dead_tuples = deadtuples;
+ msg.m_live_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : livetuples;
+ msg.m_dead_tuples = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ? 0 /* partitioned tables don't have any data, so it's 0 */
+ : deadtuples;
pgstat_send(&msg, sizeof(msg));
}
@@ -1803,6 +1808,7 @@ pgstat_initstats(Relation rel)
/* We only count stats for things that have storage */
if (!(relkind == RELKIND_RELATION ||
relkind == RELKIND_MATVIEW ||
+ relkind == RELKIND_PARTITIONED_TABLE ||
relkind == RELKIND_INDEX ||
relkind == RELKIND_TOASTVALUE ||
relkind == RELKIND_SEQUENCE))
@@ -2001,6 +2007,28 @@ pgstat_count_heap_insert(Relation rel, PgStat_Counter n)
/* We have to log the effect at the proper transactional level */
int nest_level = GetCurrentTransactionNestLevel();
+ /*
+ * If this relation is partitioned, we store all ancestors' oid
+ * to propagate its changed_tuples to their parents when this
+ * transaction is committed.
+ */
+ if (rel->rd_rel->relispartition && pgstat_info->ancestors == NULL)
+ {
+ List *ancestors;
+ ListCell *lc;
+ int i = 0;
+
+ ancestors = get_partition_ancestors(rel->rd_rel->oid);
+ pgstat_info->ancestors =
+ (Oid *) MemoryContextAllocZero(TopTransactionContext,
+ sizeof(Oid) * (ancestors->length + 1));
+ foreach(lc, ancestors)
+ {
+ pgstat_info->ancestors[i] = lfirst_oid(lc);
+ ++i;
+ }
+ }
+
if (pgstat_info->trans == NULL ||
pgstat_info->trans->nest_level != nest_level)
add_tabstat_xact_level(pgstat_info, nest_level);
@@ -2022,6 +2050,28 @@ pgstat_count_heap_update(Relation rel, bool hot)
/* We have to log the effect at the proper transactional level */
int nest_level = GetCurrentTransactionNestLevel();
+ /*
+ * If this relation is partitioned, we store all ancestors' oid
+ * to propagate its changed_tuples to their parents when this
+ * transaction is committed.
+ */
+ if (rel->rd_rel->relispartition && pgstat_info->ancestors == NULL)
+ {
+ List *ancestors;
+ ListCell *lc;
+ int i = 0;
+
+ ancestors = get_partition_ancestors(rel->rd_rel->oid);
+ pgstat_info->ancestors =
+ (Oid *) MemoryContextAllocZero(TopTransactionContext,
+ sizeof(Oid) * (ancestors->length + 1));
+ foreach(lc, ancestors)
+ {
+ pgstat_info->ancestors[i] = lfirst_oid(lc);
+ ++i;
+ }
+ }
+
if (pgstat_info->trans == NULL ||
pgstat_info->trans->nest_level != nest_level)
add_tabstat_xact_level(pgstat_info, nest_level);
@@ -2047,6 +2097,28 @@ pgstat_count_heap_delete(Relation rel)
/* We have to log the effect at the proper transactional level */
int nest_level = GetCurrentTransactionNestLevel();
+ /*
+ * If this relation is partitioned, we store all ancestors' oid
+ * to propagate its changed_tuples to their parents when this
+ * transaction is committed.
+ */
+ if (rel->rd_rel->relispartition && pgstat_info->ancestors == NULL)
+ {
+ List *ancestors;
+ ListCell *lc;
+ int i = 0;
+
+ ancestors = get_partition_ancestors(rel->rd_rel->oid);
+ pgstat_info->ancestors =
+ (Oid *) MemoryContextAllocZero(TopTransactionContext,
+ sizeof(Oid) * (ancestors->length + 1));
+ foreach(lc, ancestors)
+ {
+ pgstat_info->ancestors[i] = lfirst_oid(lc);
+ ++i;
+ }
+ }
+
if (pgstat_info->trans == NULL ||
pgstat_info->trans->nest_level != nest_level)
add_tabstat_xact_level(pgstat_info, nest_level);
@@ -2201,6 +2273,29 @@ AtEOXact_PgStat(bool isCommit, bool parallel)
tabstat->t_counts.t_changed_tuples +=
trans->tuples_inserted + trans->tuples_updated +
trans->tuples_deleted;
+
+ /*
+ * If this relation is partitioned, propagate its own
+ * changed_tuples to their all ancestors.
+ */
+ if (tabstat->ancestors != NULL)
+ {
+ int i = 0;
+
+ for(;;)
+ {
+ PgStat_TableStatus *entry;
+ Oid relid = tabstat->ancestors[i];
+
+ if(relid == InvalidOid)
+ break;
+
+ entry = get_tabstat_entry(relid, false);
+ entry->t_counts.t_changed_tuples +=
+ tabstat->t_counts.t_changed_tuples;
+ ++i;
+ }
+ }
}
else
{
@@ -6355,7 +6450,6 @@ pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len)
}
}
-
/* ----------
* pgstat_recv_archiver() -
*
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index b8041d9988..1165d4fb2c 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -157,6 +157,7 @@ typedef enum PgStat_Single_Reset_Type
typedef struct PgStat_TableStatus
{
Oid t_id; /* table's OID */
+ Oid *ancestors; /* all ancestors */
bool t_shared; /* is it a shared catalog? */
struct PgStat_TableXactStatus *trans; /* lowest subxact's counts */
PgStat_TableCounts t_counts; /* event counts to be sent */
@@ -404,7 +405,6 @@ typedef struct PgStat_MsgAnalyze
PgStat_Counter m_dead_tuples;
} PgStat_MsgAnalyze;
-
/* ----------
* PgStat_MsgArchiver Sent by the archiver to update statistics.
* ----------
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 6eec8ec568..c32745a0a2 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1790,7 +1790,7 @@ pg_stat_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_archiver| SELECT s.archived_count,
s.last_archived_wal,
@@ -2148,7 +2148,7 @@ pg_stat_xact_all_tables| SELECT c.oid AS relid,
FROM ((pg_class c
LEFT JOIN pg_index i ON ((c.oid = i.indrelid)))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
- WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char"]))
+ WHERE (c.relkind = ANY (ARRAY['r'::"char", 't'::"char", 'm'::"char", 'p'::"char"]))
GROUP BY c.oid, n.nspname, c.relname;
pg_stat_xact_sys_tables| SELECT pg_stat_xact_all_tables.relid,
pg_stat_xact_all_tables.schemaname,
^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2020-04-07 02:34 UTC | newest]
Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 09:02 Autovacuum on partitioned table yuzuko <yuzukohosoya@gmail.com>
2019-12-02 09:19 ` Laurenz Albe <laurenz.albe@cybertec.at>
2019-12-02 09:42 ` yuzuko <yuzukohosoya@gmail.com>
2019-12-27 03:37 ` yuzuko <yuzukohosoya@gmail.com>
2019-12-27 05:01 ` Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
2020-01-28 08:52 ` Amit Langote <amitlangote09@gmail.com>
2020-01-28 11:27 ` Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
2020-01-29 02:29 ` yuzuko <yuzukohosoya@gmail.com>
2020-01-29 08:56 ` Amit Langote <amitlangote09@gmail.com>
2020-01-29 11:38 ` Michael Paquier <michael@paquier.xyz>
2020-02-02 03:52 ` Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
2020-02-03 02:36 ` Amit Langote <amitlangote09@gmail.com>
2020-02-20 06:33 ` yuzuko <yuzukohosoya@gmail.com>
2020-02-20 07:50 ` Amit Langote <amitlangote09@gmail.com>
2020-02-20 08:32 ` Amit Langote <amitlangote09@gmail.com>
2020-02-20 09:29 ` Amit Langote <amitlangote09@gmail.com>
2020-02-21 06:14 ` yuzuko <yuzukohosoya@gmail.com>
2020-02-21 07:47 ` Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
2020-02-21 08:35 ` Amit Langote <amitlangote09@gmail.com>
2020-02-26 02:32 ` yuzuko <yuzukohosoya@gmail.com>
2020-02-28 02:02 ` Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
2020-02-28 02:25 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-02-28 03:57 ` Amit Langote <amitlangote09@gmail.com>
2020-03-17 03:22 ` yuzuko <yuzukohosoya@gmail.com>
2020-03-18 01:06 ` yuzuko <yuzukohosoya@gmail.com>
2020-03-18 01:49 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-04-07 02:34 ` yuzuko <yuzukohosoya@gmail.com>
2020-02-28 02:31 ` Amit Langote <amitlangote09@gmail.com>
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