On Wed, Feb 01, 2023 at 07:24:48PM +0100, Matthias van de Meent wrote:
> On Wed, 1 Feb 2023 at 18:51, Ilya Gladyshev <ilya.v.gladys...@gmail.com>
> wrote:
> >
> > 1 февр. 2023 г., в 20:27, Matthias van de Meent
> > <boekewurm+postg...@gmail.com> написал(а):
> >
> >> In HEAD we set TOTAL to whatever number partitioned table we're
> >> currently processing has - regardless of whether we're the top level
> >> statement.
> >> With the patch we instead add the number of child relations to that
> >> count, for which REL_HAS_STORAGE(child) -- or at least, in the v3
> >> posted by Ilya. Approximately immediately after updating that count we
> >> recurse to the child relations, and that only returns once it is done
> >> creating the indexes, so both TOTAL and DONE go up as we process more
> >> partitions in the hierarchy.
> >
> >
> > The TOTAL in the patch is set only when processing the top-level parent and
> > it is not updated when we recurse, so yes, it is constant. From v3:
>
> Ugh, I misread the patch, more specifically count_leaf_partitions and
> the !OidIsValid(parentIndexId) condition changes.
>
> You are correct, sorry for the noise.
That suggests that the comments could've been more clear. I added a
comment suggested by Tomas and adjusted some others and wrote a commit
message. I even ran pgindent for about the 3rd time ever.
002 are my changes as a separate patch, which you could apply to your
local branch.
And 003/4 are assertions that I wrote to demonstrate the problem and the
verify the fixes, but not being proposed for commit.
--
Justin
>From 375961e18aaa7ed7b2ebee972ad07c7a38099ef4 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Tue, 31 Jan 2023 19:13:07 +0400
Subject: [PATCH 1/4] create index progress increment
---
doc/src/sgml/monitoring.sgml | 4 +-
src/backend/commands/indexcmds.c | 64 +++++++++++++++++--
src/backend/utils/activity/backend_progress.c | 25 ++++++++
src/include/utils/backend_progress.h | 1 +
4 files changed, 87 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 1756f1a4b67..a911900271c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6601,7 +6601,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the total number of partitions on which the index is to be created.
+ the total number of leaf partitions on which the index is to be created or attached.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
@@ -6612,7 +6612,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the number of partitions on which the index has been created.
+ the number of leaf partitions on which the index has been created or attached.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 16ec0b114e6..936b4e3c1db 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -130,6 +130,30 @@ typedef struct ReindexErrorInfo
char relkind;
} ReindexErrorInfo;
+
+/*
+ * Count the number of direct and indirect leaf partitions, excluding foreign
+ * tables.
+ */
+static int
+count_leaf_partitions(Oid relid)
+{
+ int nleaves = 0;
+ List *childs = find_all_inheritors(relid, NoLock, NULL);
+ ListCell *lc;
+
+ foreach(lc, childs)
+ {
+ Oid partrelid = lfirst_oid(lc);
+
+ if (RELKIND_HAS_STORAGE(get_rel_relkind(partrelid)))
+ nleaves++;
+ }
+
+ list_free(childs);
+ return nleaves;
+}
+
/*
* CheckIndexCompatible
* Determine whether an existing index definition is compatible with a
@@ -1219,8 +1243,14 @@ DefineIndex(Oid relationId,
Relation parentIndex;
TupleDesc parentDesc;
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
- nparts);
+ if (!OidIsValid(parentIndexId))
+ {
+ int total_parts;
+
+ total_parts = count_leaf_partitions(relationId);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
+ total_parts);
+ }
/* Make a local copy of partdesc->oids[], just for safety */
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
@@ -1250,6 +1280,7 @@ DefineIndex(Oid relationId,
{
Oid childRelid = part_oids[i];
Relation childrel;
+ char child_relkind;
Oid child_save_userid;
int child_save_sec_context;
int child_save_nestlevel;
@@ -1259,6 +1290,7 @@ DefineIndex(Oid relationId,
bool found = false;
childrel = table_open(childRelid, lockmode);
+ child_relkind = RelationGetForm(childrel)->relkind;
GetUserIdAndSecContext(&child_save_userid,
&child_save_sec_context);
@@ -1431,9 +1463,25 @@ DefineIndex(Oid relationId,
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
}
+ else
+ {
+ int attached_parts = 1;
+
+ if (RELKIND_HAS_PARTITIONS(child_relkind))
+ attached_parts = count_leaf_partitions(childRelid);
+
+ /*
+ * If the index was attached, we need to update progress
+ * here, in its parent. For a partitioned index, we need
+ * to mark all of its children that were included in
+ * PROGRESS_CREATEIDX_PARTITIONS_TOTAL as done. If the
+ * index was built by calling DefineIndex() recursively,
+ * the called function is responsible for updating the
+ * progress report for built indexes.
+ */
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, attached_parts);
+ }
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
free_attrmap(attmap);
}
@@ -1484,9 +1532,15 @@ DefineIndex(Oid relationId,
/* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
- /* If this is the top-level index, we're done. */
+ /*
+ * If this is the top-level index, we're done. When called recursively
+ * for child tables, the done partition counter is incremented now,
+ * rather than in the caller.
+ */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
return address;
}
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index d96af812b19..45f7e7144b4 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -58,6 +58,31 @@ pgstat_progress_update_param(int index, int64 val)
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
+/*-----------
+ * pgstat_progress_incr_param() -
+ *
+ * Increment index'th member in st_progress_param[] of own backend entry.
+ *-----------
+ */
+void pgstat_progress_incr_param(int index, int64 incr) {
+ volatile PgBackendStatus *beentry = MyBEEntry;
+ int64 val;
+
+ Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
+
+ if (!beentry || !pgstat_track_activities)
+ return;
+
+ /*
+ * Because current backend is the only process that writes to its own status,
+ * we don't need to do the looping to read the value.
+ */
+ val = beentry->st_progress_param[index];
+ val += incr;
+
+ pgstat_progress_update_param(index, val);
+}
+
/*-----------
* pgstat_progress_update_multi_param() -
*
diff --git a/src/include/utils/backend_progress.h b/src/include/utils/backend_progress.h
index 005e5d75ab6..a84752ade99 100644
--- a/src/include/utils/backend_progress.h
+++ b/src/include/utils/backend_progress.h
@@ -36,6 +36,7 @@ typedef enum ProgressCommandType
extern void pgstat_progress_start_command(ProgressCommandType cmdtype,
Oid relid);
extern void pgstat_progress_update_param(int index, int64 val);
+extern void pgstat_progress_incr_param(int index, int64 incr);
extern void pgstat_progress_update_multi_param(int nparam, const int *index,
const int64 *val);
extern void pgstat_progress_end_command(void);
--
2.25.1
>From 3d78449f1c3d2fef8e96f4874f13004325026305 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 1 Feb 2023 21:50:21 -0600
Subject: [PATCH 2/4] s!fix CREATE INDEX progress report with nested partitions
The progress reporting was added in v12 (ab0dfc961) but the original
patch didn't seem to consider the possibility of nested partitioning.
When called recursively, DefineIndex() would clobber the number of
completed partitions, and it was possible to end up with the TOTAL
counter greater than the DONE counter.
This clarifies/re-defines that the progress reporting counts both direct
and indirect children, but doesn't count intermediate partitioned tables:
- The TOTAL counter is set once at the start of the command.
- For indexes which are newly-built, the recursively-called
DefineIndex() increments the DONE counter.
- For pre-existing indexes which are ATTACHed rather than built,
DefineIndex() increments the DONE counter, and if the attached index is
partitioned, the counter is incremented to account for each of its leaf
partitions.
Author: Ilya Gladyshev
Reviewed-By: Justin Pryzby, Tomas Vondra, Dean Rasheed, Alvaro Herrera, Matthias van de Meent
Discussion: https://www.postgresql.org/message-id/flat/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel%40gmail.com
---
doc/src/sgml/monitoring.sgml | 10 +++++--
src/backend/commands/indexcmds.c | 30 +++++++++++--------
src/backend/utils/activity/backend_progress.c | 13 ++++----
3 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a911900271c..fa139dcece7 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6601,7 +6601,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the total number of leaf partitions on which the index is to be created or attached.
+ the total number of partitions on which the index is to be created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, but excludes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
@@ -6612,7 +6615,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the number of leaf partitions on which the index has been created or attached.
+ the number of partitions on which the index has been created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, but excludes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 936b4e3c1db..84c84c41acc 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -144,7 +144,7 @@ count_leaf_partitions(Oid relid)
foreach(lc, childs)
{
- Oid partrelid = lfirst_oid(lc);
+ Oid partrelid = lfirst_oid(lc);
if (RELKIND_HAS_STORAGE(get_rel_relkind(partrelid)))
nleaves++;
@@ -1243,9 +1243,13 @@ DefineIndex(Oid relationId,
Relation parentIndex;
TupleDesc parentDesc;
+ /*
+ * Set the total number of partitions at the start of the command,
+ * but don't change it when being called recursively.
+ */
if (!OidIsValid(parentIndexId))
{
- int total_parts;
+ int total_parts;
total_parts = count_leaf_partitions(relationId);
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
@@ -1465,19 +1469,21 @@ DefineIndex(Oid relationId,
}
else
{
- int attached_parts = 1;
+ int attached_parts;
- if (RELKIND_HAS_PARTITIONS(child_relkind))
- attached_parts = count_leaf_partitions(childRelid);
+ /*
+ * Avoid the overhead of counting partitions when that
+ * can't apply.
+ */
+ attached_parts = RELKIND_HAS_PARTITIONS(child_relkind) ?
+ count_leaf_partitions(childRelid) : 1;
/*
- * If the index was attached, we need to update progress
- * here, in its parent. For a partitioned index, we need
- * to mark all of its children that were included in
- * PROGRESS_CREATEIDX_PARTITIONS_TOTAL as done. If the
- * index was built by calling DefineIndex() recursively,
- * the called function is responsible for updating the
- * progress report for built indexes.
+ * If a pre-existing index was attached, the progress
+ * report is updated here. If the index was partitioned,
+ * all the children that were counted towards
+ * PROGRESS_CREATEIDX_PARTITIONS_TOTAL are counted as
+ * done.
*/
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, attached_parts);
}
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index 45f7e7144b4..2a9994b98fd 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -61,12 +61,14 @@ pgstat_progress_update_param(int index, int64 val)
/*-----------
* pgstat_progress_incr_param() -
*
- * Increment index'th member in st_progress_param[] of own backend entry.
+ * Increment index'th member in st_progress_param[] of the current backend.
*-----------
*/
-void pgstat_progress_incr_param(int index, int64 incr) {
+void
+pgstat_progress_incr_param(int index, int64 incr)
+{
volatile PgBackendStatus *beentry = MyBEEntry;
- int64 val;
+ int64 val;
Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
@@ -74,8 +76,9 @@ void pgstat_progress_incr_param(int index, int64 incr) {
return;
/*
- * Because current backend is the only process that writes to its own status,
- * we don't need to do the looping to read the value.
+ * Because no other process should write to this backend's own status, we
+ * can read its value from shared memory without needing to loop to ensure
+ * its consistency.
*/
val = beentry->st_progress_param[index];
val += incr;
--
2.25.1
>From f484d5c9b602f158e86f9507b662b7bbbb4a19b7 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 1 Feb 2023 10:23:53 -0600
Subject: [PATCH 3/4] assertions for progress reporting
---
src/backend/utils/activity/backend_progress.c | 83 +++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index 2a9994b98fd..d0566430fab 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -10,6 +10,7 @@
*/
#include "postgres.h"
+#include "commands/progress.h"
#include "port/atomics.h" /* for memory barriers */
#include "utils/backend_progress.h"
#include "utils/backend_status.h"
@@ -37,6 +38,84 @@ pgstat_progress_start_command(ProgressCommandType cmdtype, Oid relid)
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
+/*
+ * Check for consistency of progress data (current < total).
+ *
+ * Check during pgstat_progress_updates_*() rather than only from
+ * pgstat_progress_end_command() to catch issues with uninitialized/stale data
+ * from previous progress commands.
+ *
+ * If a command fails due to interrupt or error, the values may be less than
+ * the expected final value.
+ */
+static void
+pgstat_progress_asserts(void)
+{
+ volatile PgBackendStatus *beentry = MyBEEntry;
+ volatile int64 *a = beentry->st_progress_param;
+
+ switch (beentry->st_progress_command)
+ {
+ case PROGRESS_COMMAND_VACUUM:
+ Assert(a[PROGRESS_VACUUM_HEAP_BLKS_SCANNED] <=
+ a[PROGRESS_VACUUM_TOTAL_HEAP_BLKS]);
+ Assert(a[PROGRESS_VACUUM_HEAP_BLKS_VACUUMED] <=
+ a[PROGRESS_VACUUM_TOTAL_HEAP_BLKS]);
+ Assert(a[PROGRESS_VACUUM_NUM_DEAD_TUPLES] <=
+ a[PROGRESS_VACUUM_MAX_DEAD_TUPLES]);
+ break;
+
+ case PROGRESS_COMMAND_ANALYZE:
+ Assert(a[PROGRESS_ANALYZE_BLOCKS_DONE] <=
+ a[PROGRESS_ANALYZE_BLOCKS_TOTAL]);
+ Assert(a[PROGRESS_ANALYZE_EXT_STATS_COMPUTED] <=
+ a[PROGRESS_ANALYZE_EXT_STATS_TOTAL]);
+ Assert(a[PROGRESS_ANALYZE_CHILD_TABLES_DONE] <=
+ a[PROGRESS_ANALYZE_CHILD_TABLES_TOTAL]);
+ break;
+
+ case PROGRESS_COMMAND_CLUSTER:
+ Assert(a[PROGRESS_CLUSTER_HEAP_BLKS_SCANNED] <=
+ a[PROGRESS_CLUSTER_TOTAL_HEAP_BLKS]);
+ /* FALLTHROUGH */
+
+ case PROGRESS_COMMAND_CREATE_INDEX:
+ Assert(a[PROGRESS_CREATEIDX_TUPLES_DONE] <=
+ a[PROGRESS_CREATEIDX_TUPLES_TOTAL]);
+ Assert(a[PROGRESS_CREATEIDX_PARTITIONS_DONE] <=
+ a[PROGRESS_CREATEIDX_PARTITIONS_TOTAL]);
+ break;
+
+ case PROGRESS_COMMAND_BASEBACKUP:
+ /* progress reporting is optional for these */
+ if (a[PROGRESS_BASEBACKUP_BACKUP_TOTAL] >= 0)
+ {
+ Assert(a[PROGRESS_BASEBACKUP_BACKUP_STREAMED] <=
+ a[PROGRESS_BASEBACKUP_BACKUP_TOTAL]);
+ Assert(a[PROGRESS_BASEBACKUP_TBLSPC_STREAMED] <=
+ a[PROGRESS_BASEBACKUP_TBLSPC_TOTAL]);
+ }
+ break;
+
+ case PROGRESS_COMMAND_COPY:
+#if 0
+ //This currently fails file_fdw tests, since pgstat_prorgress evidently fails
+ // to support simultaneous copy commands, as happens during JOIN.
+ /* bytes progress is not available in all cases */
+ if (a[PROGRESS_COPY_BYTES_TOTAL] > 0)
+ //Assert(a[PROGRESS_COPY_BYTES_PROCESSED] <= a[PROGRESS_COPY_BYTES_TOTAL]);
+ if (a[PROGRESS_COPY_BYTES_PROCESSED] > a[PROGRESS_COPY_BYTES_TOTAL])
+ elog(WARNING, "PROGRESS_COPY_BYTES_PROCESSED %ld %ld",
+ a[PROGRESS_COPY_BYTES_PROCESSED],
+ a[PROGRESS_COPY_BYTES_TOTAL]);
+#endif
+ break;
+
+ case PROGRESS_COMMAND_INVALID:
+ break; /* Do nothing */
+ }
+}
+
/*-----------
* pgstat_progress_update_param() -
*
@@ -56,6 +135,8 @@ pgstat_progress_update_param(int index, int64 val)
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
beentry->st_progress_param[index] = val;
PGSTAT_END_WRITE_ACTIVITY(beentry);
+
+ pgstat_progress_asserts();
}
/*-----------
@@ -113,6 +194,8 @@ pgstat_progress_update_multi_param(int nparam, const int *index,
}
PGSTAT_END_WRITE_ACTIVITY(beentry);
+
+ pgstat_progress_asserts();
}
/*-----------
--
2.25.1
>From ee27366bf3f7f5fd6a9884462476b4240cdd5c9a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 21 Jan 2023 21:41:04 -0600
Subject: [PATCH 4/4] f! also assert that progress values don't go backwards
and the total is constant
See also:
https://www.postgresql.org/message-id/CA%2BTgmoYSvEP3weQaCPGf6%2BDXLy2__JbJUYtoXyWP%3DqHcyGbihA%40mail.gmail.com
---
src/backend/access/heap/vacuumlazy.c | 37 +++++++++
src/backend/commands/analyze.c | 10 ++-
src/backend/storage/lmgr/lmgr.c | 24 +++---
src/backend/utils/activity/backend_progress.c | 77 +++++++++++++++++++
4 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8f14cf85f38..22661f6a292 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1042,6 +1042,13 @@ lazy_scan_heap(LVRelState *vacrel)
/* Forget the LP_DEAD items that we just vacuumed */
dead_items->num_items = 0;
+ {
+ int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES};
+ const int64 progress_vals[] = {0};
+
+ pgstat_progress_update_multi_param(1, progress_inds, progress_vals);
+ }
+
/*
* Periodically perform FSM vacuuming to make newly-freed
@@ -2199,6 +2206,13 @@ lazy_vacuum(LVRelState *vacrel)
{
Assert(!vacrel->do_index_cleanup);
vacrel->dead_items->num_items = 0;
+ {
+ int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES};
+ const int64 progress_vals[] = {0};
+
+ pgstat_progress_update_multi_param(1, progress_inds, progress_vals);
+ }
+
return;
}
@@ -2301,6 +2315,13 @@ lazy_vacuum(LVRelState *vacrel)
* vacuum)
*/
vacrel->dead_items->num_items = 0;
+
+ {
+ int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES};
+ const int64 progress_vals[] = {0};
+
+ pgstat_progress_update_multi_param(1, progress_inds, progress_vals);
+ }
}
/*
@@ -2414,12 +2435,23 @@ lazy_vacuum_heap_rel(LVRelState *vacrel)
BlockNumber vacuumed_pages = 0;
Buffer vmbuffer = InvalidBuffer;
LVSavedErrInfo saved_err_info;
+#if 0
+ int const progress_inds[] = {
+ PROGRESS_VACUUM_PHASE,
+ PROGRESS_VACUUM_NUM_DEAD_TUPLES,
+ };
+ const int64 progress_vals[] = {
+ PROGRESS_VACUUM_PHASE_VACUUM_HEAP,
+ 0,
+ };
+#endif
Assert(vacrel->do_index_vacuuming);
Assert(vacrel->do_index_cleanup);
Assert(vacrel->num_index_scans > 0);
/* Report that we are now vacuuming the heap */
+ //pgstat_progress_update_multi_param(2, progress_inds, progress_vals);
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
@@ -3190,7 +3222,12 @@ dead_items_alloc(LVRelState *vacrel, int nworkers)
dead_items = (VacDeadItems *) palloc(vac_max_items_to_alloc_size(max_items));
dead_items->max_items = max_items;
dead_items->num_items = 0;
+ {
+ int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES};
+ const int64 progress_vals[] = {0};
+ pgstat_progress_update_multi_param(1, progress_inds, progress_vals);
+ }
vacrel->dead_items = dead_items;
}
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index c86e690980e..96710b84558 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1145,6 +1145,12 @@ acquire_sample_rows(Relation onerel, int elevel,
TableScanDesc scan;
BlockNumber nblocks;
BlockNumber blksdone = 0;
+ int64 progress_vals[2] = {0};
+ int const progress_inds[2] = {
+ PROGRESS_ANALYZE_BLOCKS_DONE,
+ PROGRESS_ANALYZE_BLOCKS_TOTAL
+ };
+
#ifdef USE_PREFETCH
int prefetch_maximum = 0; /* blocks to prefetch if enabled */
BlockSamplerData prefetch_bs;
@@ -1169,8 +1175,8 @@ acquire_sample_rows(Relation onerel, int elevel,
#endif
/* Report sampling block numbers */
- pgstat_progress_update_param(PROGRESS_ANALYZE_BLOCKS_TOTAL,
- nblocks);
+ progress_vals[1] = nblocks;
+ pgstat_progress_update_multi_param(2, progress_inds, progress_vals);
/* Prepare for sampling rows */
reservoir_init_selection_state(&rstate, targrows);
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index ee9b89a6726..8666d850660 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -912,6 +912,15 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
int total = 0;
int done = 0;
+ const int index[] = {
+ PROGRESS_WAITFOR_TOTAL,
+ PROGRESS_WAITFOR_DONE,
+ PROGRESS_WAITFOR_CURRENT_PID
+ };
+ const int64 values[] = {
+ 0, 0, 0
+ };
+
/* Done if no locks to wait for */
if (locktags == NIL)
return;
@@ -930,7 +939,10 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
}
if (progress)
+ {
+ pgstat_progress_update_multi_param(3, index, values);
pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, total);
+ }
/*
* Note: GetLockConflicts() never reports our own xid, hence we need not
@@ -960,19 +972,9 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, ++done);
}
}
- if (progress)
- {
- const int index[] = {
- PROGRESS_WAITFOR_TOTAL,
- PROGRESS_WAITFOR_DONE,
- PROGRESS_WAITFOR_CURRENT_PID
- };
- const int64 values[] = {
- 0, 0, 0
- };
+ if (progress)
pgstat_progress_update_multi_param(3, index, values);
- }
list_free_deep(holders);
}
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index d0566430fab..707ab929fac 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -116,6 +116,74 @@ pgstat_progress_asserts(void)
}
}
+static void
+pgstat_progress_assert_forward_progress(int command, int index,
+ int64 oldval, int64 newval)
+{
+ switch (command)
+ {
+ case PROGRESS_COMMAND_ANALYZE:
+
+ /*
+ * phase goes backwards for inheritance tables, which are sampled
+ * twice
+ */
+ if (index != PROGRESS_ANALYZE_CURRENT_CHILD_TABLE_RELID &&
+ index != PROGRESS_ANALYZE_PHASE)
+ Assert(newval >= oldval);
+ if (index == PROGRESS_ANALYZE_BLOCKS_TOTAL ||
+ index == PROGRESS_ANALYZE_EXT_STATS_TOTAL ||
+ index == PROGRESS_ANALYZE_CHILD_TABLES_TOTAL)
+ Assert(oldval == 0);
+ break;
+
+ case PROGRESS_COMMAND_CLUSTER:
+ if (index != PROGRESS_CLUSTER_INDEX_RELID)
+ Assert(newval >= oldval);
+ if (index == PROGRESS_CLUSTER_TOTAL_HEAP_BLKS)
+ Assert(oldval == 0);
+ break;
+
+ case PROGRESS_COMMAND_CREATE_INDEX:
+ if (index != PROGRESS_CREATEIDX_INDEX_OID &&
+ index != PROGRESS_CREATEIDX_SUBPHASE &&
+ index != PROGRESS_WAITFOR_CURRENT_PID &&
+ index != PROGRESS_CREATEIDX_ACCESS_METHOD_OID)
+ Assert(newval >= oldval);
+
+ if (index == PROGRESS_CREATEIDX_TUPLES_TOTAL ||
+ index == PROGRESS_CREATEIDX_PARTITIONS_TOTAL)
+ Assert(oldval == 0);
+ break;
+
+ case PROGRESS_COMMAND_BASEBACKUP:
+ if (index == PROGRESS_BASEBACKUP_BACKUP_TOTAL &&
+ oldval == 0 && newval == -1)
+ return; /* Do nothing: this is the initial "null"
+ * state before the size is estimated */
+ Assert(newval >= oldval);
+
+ if (index == PROGRESS_BASEBACKUP_BACKUP_TOTAL ||
+ index == PROGRESS_BASEBACKUP_TBLSPC_TOTAL)
+ Assert(oldval == 0);
+ break;
+
+ case PROGRESS_COMMAND_COPY:
+ Assert(newval >= oldval);
+ if (index == PROGRESS_COPY_BYTES_TOTAL)
+ Assert(oldval == 0);
+ break;
+ case PROGRESS_COMMAND_VACUUM:
+ Assert(newval >= oldval);
+ if (index == PROGRESS_VACUUM_TOTAL_HEAP_BLKS)
+ Assert(oldval == 0);
+ break;
+
+ case PROGRESS_COMMAND_INVALID:
+ break;
+ }
+}
+
/*-----------
* pgstat_progress_update_param() -
*
@@ -132,6 +200,15 @@ pgstat_progress_update_param(int index, int64 val)
if (!beentry || !pgstat_track_activities)
return;
+ if (index != PROGRESS_SCAN_BLOCKS_DONE)
+ {
+ /* Check that progress does not go backwards */
+ int64 oldval = beentry->st_progress_param[index];
+
+ pgstat_progress_assert_forward_progress(beentry->st_progress_command,
+ index, oldval, val);
+ }
+
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
beentry->st_progress_param[index] = val;
PGSTAT_END_WRITE_ACTIVITY(beentry);
--
2.25.1
