[PATCH] Support for basic ALTER TABLE progress reporting.
Started by Jiří Kavalík7 months ago6 messages
Hi,
while testing int->bigint migrations for our db at work I really missed
ALTER TABLE progress reporting and during the waits I checked the code.
It seems to me that ALTERs can be mostly categorized as
1) trivial ones - metadata rewrites, fast adding/removing columns, trying
to change column type to one already present etc. not much to report here
2) scanning ones - adding constraints - it imho gives enough info to report
blocks total and scanned and tuples scanned
3) rewrites - actually changing data or types - add number of written
blocks/tuples
3b) index rewrites - report number of indexes processed
From that it seems to me that the basic info is very similar to already
present CLUSTER/VACUUM-FULL reporting so I tried to tap into that and just
add a support for a new command.
I identified a handful of places where to add the reporting for ALTERs and
it seems to work,
What I changed:
`commands/progress.h`
- new cluster reporting command + new phase for FK checks
`commands/tablecmds.c`
- start and end reporting inside `ATRewriteTables()`
- report blocks total, blocks and tuples scanned and possibly tuples
written in `ATRewriteTable`
- add at least phase info in `validateForeignKeyConstraint`, possibly more
if the check cannot be done by left join
`catalog/system_views.sql`
- output for the new command and phase
`catalog/storage.c`
- number of blocks processed in `RelationCopyStorage()` for the case table
is moved between tablespaces by direct copying
+ some basic documentation updates
What I did not have to change - index rebuilds used by CLUSTER reported
their progress already, it just was not shown without a valid command
configured.
I ran some manual tests locally + ran regression tests and it seems to work
fine.
The reporting may be a bit crude and may be missing some phases but it
covers the IO-heavy operations with some reasonable numbers. (well, not the
FK check by left anti-join, but I don't want to mess with that + maybe
number of FKs checked might be shown?)
Thanks
Best regards
jkavalik
Attachments:
0001-ALTER-TABLE-progress-support.patchtext/plain; charset=US-ASCII; name=0001-ALTER-TABLE-progress-support.patchDownload
From 12cfbbc3448237733193fbed8a4383ba8656237e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Kaval=C3=ADk?= <jkavalik@gmail.com>
Date: Sun, 25 May 2025 23:23:56 +0200
Subject: [PATCH] ALTER TABLE progress support
---
doc/src/sgml/monitoring.sgml | 13 +++----
doc/src/sgml/ref/alter_table.sgml | 10 ++++++
src/backend/catalog/storage.c | 7 ++++
src/backend/catalog/system_views.sql | 2 ++
src/backend/commands/tablecmds.c | 54 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 2 ++
src/test/regress/expected/rules.out | 2 ++
7 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 4265a22d4de..09307c5f490 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -400,7 +400,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<row>
<entry><structname>pg_stat_progress_cluster</structname><indexterm><primary>pg_stat_progress_cluster</primary></indexterm></entry>
<entry>One row for each backend running
- <command>CLUSTER</command> or <command>VACUUM FULL</command>, showing current progress.
+ <command>CLUSTER</command>, <command>VACUUM FULL</command> or <command>ALTER TABLE</command>, showing current progress.
See <xref linkend="cluster-progress-reporting"/>.
</entry>
</row>
@@ -5492,7 +5492,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<productname>PostgreSQL</productname> has the ability to report the progress of
certain commands during command execution. Currently, the only commands
which support progress reporting are <command>ANALYZE</command>,
- <command>CLUSTER</command>,
+ <command>CLUSTER</command>, <command>ALTER TABLE</command>,
<command>CREATE INDEX</command>, <command>VACUUM</command>,
<command>COPY</command>,
and <xref linkend="protocol-replication-base-backup"/> (i.e., replication
@@ -5738,8 +5738,9 @@ FROM pg_stat_get_backend_idset() AS backendid;
</indexterm>
<para>
- Whenever <command>CLUSTER</command> or <command>VACUUM FULL</command> is
- running, the <structname>pg_stat_progress_cluster</structname> view will
+ Whenever <command>CLUSTER</command>, <command>VACUUM FULL</command>
+ or <command>ALTER TABLE</command> is running,
+ the <structname>pg_stat_progress_cluster</structname> view will
contain a row for each backend that is currently running either command.
The tables below describe the information that will be reported and
provide information about how to interpret it.
@@ -5801,7 +5802,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>command</structfield> <type>text</type>
</para>
<para>
- The command that is running. Either <literal>CLUSTER</literal> or <literal>VACUUM FULL</literal>.
+ The command that is running. Either <literal>CLUSTER</literal>, <literal>VACUUM FULL</literal> or <literal>ALTER TABLE</literal>.
</para></entry>
</row>
@@ -5884,7 +5885,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</table>
<table id="cluster-phases">
- <title>CLUSTER and VACUUM FULL Phases</title>
+ <title>CLUSTER, VACUUM FULL and ALTER TABLE Phases</title>
<tgroup cols="2">
<colspec colname="col1" colwidth="1*"/>
<colspec colname="col2" colwidth="2*"/>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index d63f3a621ac..228f27ac5fc 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1859,6 +1859,16 @@ ALTER TABLE measurement
</para>
</refsect1>
+ <refsect1>
+ <title>Progress Reporting</title>
+ <para>
+ When an <command>ALTER TABLE</command> operation rewrites the table, progress
+ can be monitored via the <literal>pg_stat_progress_cluster</literal> system view,
+ similar to <command>CLUSTER</command> and <command>VACUUM FULL</command> commands.
+ The command type will be reported as <literal>'ALTER TABLE'</literal>.
+ </para>
+ </refsect1>
+
<refsect1>
<title>See Also</title>
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 227df90f89c..79d14d86bc9 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -26,6 +26,7 @@
#include "access/xlogutils.h"
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/bulk_write.h"
@@ -505,6 +506,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
nblocks = smgrnblocks(src, forkNum);
+ /* Report expected number of block to copy */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, nblocks);
+
for (blkno = 0; blkno < nblocks; blkno++)
{
BulkWriteBuffer buf;
@@ -556,6 +560,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
* page including any unused space.
*/
smgr_bulk_write(bulkstate, blkno, buf, false);
+
+ /* Update progress report */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, blkno + 1);
}
smgr_bulk_finish(bulkstate);
}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 08f780a2e63..99a6ee0060f 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1252,6 +1252,7 @@ CREATE VIEW pg_stat_progress_cluster AS
S.relid AS relid,
CASE S.param1 WHEN 1 THEN 'CLUSTER'
WHEN 2 THEN 'VACUUM FULL'
+ WHEN 3 THEN 'ALTER TABLE'
END AS command,
CASE S.param2 WHEN 0 THEN 'initializing'
WHEN 1 THEN 'seq scanning heap'
@@ -1261,6 +1262,7 @@ CREATE VIEW pg_stat_progress_cluster AS
WHEN 5 THEN 'swapping relation files'
WHEN 6 THEN 'rebuilding index'
WHEN 7 THEN 'performing final cleanup'
+ WHEN 8 THEN 'checking foreign key constraints'
END AS phase,
CAST(S.param3 AS oid) AS cluster_index_relid,
S.param4 AS heap_tuples_scanned,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..9ad05d39164 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -59,6 +59,7 @@
#include "commands/comment.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/sequence.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
@@ -5839,6 +5840,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
+ /* Start progress reporting */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tab->relid);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
+
/*
* If we change column data types, the operation has to be propagated
* to tables that use this table's rowtype as a column type.
@@ -5979,6 +5984,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
*/
ATRewriteTable(tab, OIDNewHeap);
+ /* Report that we are now swapping relation files */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES);
/*
* Swap the physical files of the old and new heaps, then rebuild
* indexes and discard the old heap. We can use RecentXmin for
@@ -6090,6 +6098,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
table_close(rel, NoLock);
}
+ /* Report that we are now doing clean up */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP);
+
/* Finally, run any afterStmts that were queued up */
foreach(ltab, *wqueue)
{
@@ -6104,6 +6116,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
CommandCounterIncrement();
}
}
+
+ pgstat_progress_end_command();
}
/*
@@ -6129,6 +6143,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
BulkInsertState bistate;
int ti_options;
ExprState *partqualstate = NULL;
+ int numTuples = 0;
/*
* Open the relation(s). We have surely already locked the existing
@@ -6138,6 +6153,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
oldTupDesc = tab->oldDesc;
newTupDesc = RelationGetDescr(oldrel); /* includes all mods */
+ /* Update progress reporting - we are actually scanning and possibly rewriting the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(oldrel));
+
if (OidIsValid(OIDNewHeap))
{
Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
@@ -6245,6 +6264,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleTableSlot *oldslot;
TupleTableSlot *newslot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
MemoryContext oldCxt;
List *dropped_attrs = NIL;
ListCell *lc;
@@ -6344,6 +6364,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
*/
snapshot = RegisterSnapshot(GetLatestSnapshot());
scan = table_beginscan(oldrel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
+ heapScan->rs_nblocks);
/*
* Switch to per-tuple memory context and reset it for each tuple
@@ -6354,6 +6377,13 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
if (tab->rewrite > 0)
{
@@ -6402,6 +6432,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ExecStoreVirtualTuple(newslot);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
/*
* Now, evaluate any expressions whose inputs come from the
* new tuple. We assume these columns won't reference each
@@ -6522,6 +6555,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
CHECK_FOR_INTERRUPTS();
}
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ heapScan->rs_nblocks);
MemoryContextSwitchTo(oldCxt);
table_endscan(scan);
@@ -13638,10 +13673,12 @@ validateForeignKeyConstraint(char *conname,
{
TupleTableSlot *slot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
Trigger trig = {0};
Snapshot snapshot;
MemoryContext oldcxt;
MemoryContext perTupCxt;
+ int numTuples = 0;
ereport(DEBUG1,
(errmsg_internal("validating foreign key constraint \"%s\"", conname)));
@@ -13660,6 +13697,10 @@ validateForeignKeyConstraint(char *conname,
trig.tginitdeferred = false;
/* we needn't fill in remaining fields */
+ /* Report that we are now checking foreign keys */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_CHECK_FKEYS);
+
/*
* See if we can do it with a single LEFT JOIN query. A false result
* indicates we must proceed with the fire-the-trigger method. We can't do
@@ -13677,6 +13718,7 @@ validateForeignKeyConstraint(char *conname,
snapshot = RegisterSnapshot(GetLatestSnapshot());
slot = table_slot_create(rel, NULL);
scan = table_beginscan(rel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
perTupCxt = AllocSetContextCreate(CurrentMemoryContext,
"validateForeignKeyConstraint",
@@ -13712,6 +13754,14 @@ validateForeignKeyConstraint(char *conname,
RI_FKey_check_ins(fcinfo);
MemoryContextReset(perTupCxt);
+
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
}
MemoryContextSwitchTo(oldcxt);
@@ -16776,6 +16826,10 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
*/
rel = relation_open(tableOid, lockmode);
+ /* Update progress reporting - we are copying the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(rel));
+
/* Check first if relation can be moved to new tablespace */
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
{
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 7c736e7b03b..56585742838 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -74,10 +74,12 @@
#define PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES 5
#define PROGRESS_CLUSTER_PHASE_REBUILD_INDEX 6
#define PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP 7
+#define PROGRESS_CLUSTER_PHASE_CHECK_FKEYS 8
/* Commands of PROGRESS_CLUSTER */
#define PROGRESS_CLUSTER_COMMAND_CLUSTER 1
#define PROGRESS_CLUSTER_COMMAND_VACUUM_FULL 2
+#define PROGRESS_CLUSTER_COMMAND_ALTER_TABLE 3 /* New command type */
/* Progress parameters for CREATE INDEX */
/* 3, 4 and 5 reserved for "waitfor" metrics */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 6cf828ca8d0..3b8a0ec0756 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1982,6 +1982,7 @@ pg_stat_progress_cluster| SELECT s.pid,
CASE s.param1
WHEN 1 THEN 'CLUSTER'::text
WHEN 2 THEN 'VACUUM FULL'::text
+ WHEN 3 THEN 'ALTER TABLE'::text
ELSE NULL::text
END AS command,
CASE s.param2
@@ -1993,6 +1994,7 @@ pg_stat_progress_cluster| SELECT s.pid,
WHEN 5 THEN 'swapping relation files'::text
WHEN 6 THEN 'rebuilding index'::text
WHEN 7 THEN 'performing final cleanup'::text
+ WHEN 8 THEN 'checking foreign key constraints'::text
ELSE NULL::text
END AS phase,
(s.param3)::oid AS cluster_index_relid,
--
2.34.1
Re: [PATCH] Support for basic ALTER TABLE progress reporting.
On Mon, Jun 2, 2025 at 3:35 PM Jiří Kavalík <jkavalik@gmail.com> wrote:
What I changed:
`commands/tablecmds.c`
- start and end reporting inside `ATRewriteTables()`
- report blocks total, blocks and tuples scanned and possibly tuples written in `ATRewriteTable`
- add at least phase info in `validateForeignKeyConstraint`, possibly more if the check cannot be done by left join
hi.
similar to DoCopyTo, processed as uint64,
in ATRewriteTable, numTuples should be also uint64 not int?
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
Later we may do constraint checks in ``foreach(l, tab->constraints)``.
putting it after table_tuple_insert would be more appropriate, IMHO.static void
ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context)
{
ListCell *ltab;
/* Go through each table that needs to be checked or rewritten */
foreach(ltab, *wqueue)
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
/* Relations without storage may be ignored here */
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
/* Start progress reporting */
pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tab->relid);
pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
}
Perhaps this is a bit early—we haven't completed the error checks yet.
we have several "ereport(ERROR..." in below.
maybe place it before ATRewriteTable, IMHO.
Re: [PATCH] Support for basic ALTER TABLE progress reporting.
Hi, thank you for the review!
On Fri, Jun 6, 2025 at 10:37 AM jian he <jian.universality@gmail.com> wrote:
On Mon, Jun 2, 2025 at 3:35 PM Jiří Kavalík <jkavalik@gmail.com> wrote:
What I changed:
`commands/tablecmds.c`
- start and end reporting inside `ATRewriteTables()`
- report blocks total, blocks and tuples scanned and possibly tupleswritten in `ATRewriteTable`
- add at least phase info in `validateForeignKeyConstraint`, possibly
more if the check cannot be done by left join
hi.
similar to DoCopyTo, processed as uint64,
in ATRewriteTable, numTuples should be also uint64 not int?
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN, + numTuples); +
Yes, that makes sense, updated patch attached
Later we may do constraint checks in ``foreach(l, tab->constraints)``.
putting it after table_tuple_insert would be more appropriate, IMHO.
As I understand it, if we get an error from any of the constraints, the
ALTER fails anyway so there seems to be no window for mismatch. But it
might make sense to move it into the same block where `table_tuple_insert`
is anyway.
static void
ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE
lockmode,
AlterTableUtilityContext *context)
{
ListCell *ltab;
/* Go through each table that needs to be checked or rewritten */
foreach(ltab, *wqueue)
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
/* Relations without storage may be ignored here */
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
/* Start progress reporting */
pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER,
tab->relid);
pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
}Perhaps this is a bit early—we haven't completed the error checks yet.
we have several "ereport(ERROR..." in below.
maybe place it before ATRewriteTable, IMHO.
There are three different branches under it, two of them containing
`ATRewriteTable()` calls. So I picked a place before that branching instead
of starting in two separate places. It seems simpler but thats my only
argument so I am open to other opinions (for both this and where to put the
"tuples_written" update).
Best regards
jkavalik
Attachments:
0001-ALTER-TABLE-progress-support.patchtext/plain; charset=US-ASCII; name=0001-ALTER-TABLE-progress-support.patchDownload
From e5a60149438308d4d5e107f9cc36d5d2f659188d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Kaval=C3=ADk?= <jkavalik@gmail.com>
Date: Sun, 25 May 2025 23:23:56 +0200
Subject: [PATCH] ALTER TABLE progress support
---
doc/src/sgml/monitoring.sgml | 13 +++----
doc/src/sgml/ref/alter_table.sgml | 10 ++++++
src/backend/catalog/storage.c | 7 ++++
src/backend/catalog/system_views.sql | 2 ++
src/backend/commands/tablecmds.c | 54 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 2 ++
src/test/regress/expected/rules.out | 2 ++
7 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 4265a22d4de..09307c5f490 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -400,7 +400,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<row>
<entry><structname>pg_stat_progress_cluster</structname><indexterm><primary>pg_stat_progress_cluster</primary></indexterm></entry>
<entry>One row for each backend running
- <command>CLUSTER</command> or <command>VACUUM FULL</command>, showing current progress.
+ <command>CLUSTER</command>, <command>VACUUM FULL</command> or <command>ALTER TABLE</command>, showing current progress.
See <xref linkend="cluster-progress-reporting"/>.
</entry>
</row>
@@ -5492,7 +5492,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<productname>PostgreSQL</productname> has the ability to report the progress of
certain commands during command execution. Currently, the only commands
which support progress reporting are <command>ANALYZE</command>,
- <command>CLUSTER</command>,
+ <command>CLUSTER</command>, <command>ALTER TABLE</command>,
<command>CREATE INDEX</command>, <command>VACUUM</command>,
<command>COPY</command>,
and <xref linkend="protocol-replication-base-backup"/> (i.e., replication
@@ -5738,8 +5738,9 @@ FROM pg_stat_get_backend_idset() AS backendid;
</indexterm>
<para>
- Whenever <command>CLUSTER</command> or <command>VACUUM FULL</command> is
- running, the <structname>pg_stat_progress_cluster</structname> view will
+ Whenever <command>CLUSTER</command>, <command>VACUUM FULL</command>
+ or <command>ALTER TABLE</command> is running,
+ the <structname>pg_stat_progress_cluster</structname> view will
contain a row for each backend that is currently running either command.
The tables below describe the information that will be reported and
provide information about how to interpret it.
@@ -5801,7 +5802,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>command</structfield> <type>text</type>
</para>
<para>
- The command that is running. Either <literal>CLUSTER</literal> or <literal>VACUUM FULL</literal>.
+ The command that is running. Either <literal>CLUSTER</literal>, <literal>VACUUM FULL</literal> or <literal>ALTER TABLE</literal>.
</para></entry>
</row>
@@ -5884,7 +5885,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</table>
<table id="cluster-phases">
- <title>CLUSTER and VACUUM FULL Phases</title>
+ <title>CLUSTER, VACUUM FULL and ALTER TABLE Phases</title>
<tgroup cols="2">
<colspec colname="col1" colwidth="1*"/>
<colspec colname="col2" colwidth="2*"/>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index d63f3a621ac..228f27ac5fc 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1859,6 +1859,16 @@ ALTER TABLE measurement
</para>
</refsect1>
+ <refsect1>
+ <title>Progress Reporting</title>
+ <para>
+ When an <command>ALTER TABLE</command> operation rewrites the table, progress
+ can be monitored via the <literal>pg_stat_progress_cluster</literal> system view,
+ similar to <command>CLUSTER</command> and <command>VACUUM FULL</command> commands.
+ The command type will be reported as <literal>'ALTER TABLE'</literal>.
+ </para>
+ </refsect1>
+
<refsect1>
<title>See Also</title>
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 227df90f89c..79d14d86bc9 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -26,6 +26,7 @@
#include "access/xlogutils.h"
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/bulk_write.h"
@@ -505,6 +506,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
nblocks = smgrnblocks(src, forkNum);
+ /* Report expected number of block to copy */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, nblocks);
+
for (blkno = 0; blkno < nblocks; blkno++)
{
BulkWriteBuffer buf;
@@ -556,6 +560,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
* page including any unused space.
*/
smgr_bulk_write(bulkstate, blkno, buf, false);
+
+ /* Update progress report */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, blkno + 1);
}
smgr_bulk_finish(bulkstate);
}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 08f780a2e63..99a6ee0060f 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1252,6 +1252,7 @@ CREATE VIEW pg_stat_progress_cluster AS
S.relid AS relid,
CASE S.param1 WHEN 1 THEN 'CLUSTER'
WHEN 2 THEN 'VACUUM FULL'
+ WHEN 3 THEN 'ALTER TABLE'
END AS command,
CASE S.param2 WHEN 0 THEN 'initializing'
WHEN 1 THEN 'seq scanning heap'
@@ -1261,6 +1262,7 @@ CREATE VIEW pg_stat_progress_cluster AS
WHEN 5 THEN 'swapping relation files'
WHEN 6 THEN 'rebuilding index'
WHEN 7 THEN 'performing final cleanup'
+ WHEN 8 THEN 'checking foreign key constraints'
END AS phase,
CAST(S.param3 AS oid) AS cluster_index_relid,
S.param4 AS heap_tuples_scanned,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..67a3466295c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -59,6 +59,7 @@
#include "commands/comment.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/sequence.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
@@ -5839,6 +5840,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
+ /* Start progress reporting */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tab->relid);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
+
/*
* If we change column data types, the operation has to be propagated
* to tables that use this table's rowtype as a column type.
@@ -5979,6 +5984,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
*/
ATRewriteTable(tab, OIDNewHeap);
+ /* Report that we are now swapping relation files */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES);
/*
* Swap the physical files of the old and new heaps, then rebuild
* indexes and discard the old heap. We can use RecentXmin for
@@ -6090,6 +6098,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
table_close(rel, NoLock);
}
+ /* Report that we are now doing clean up */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP);
+
/* Finally, run any afterStmts that were queued up */
foreach(ltab, *wqueue)
{
@@ -6104,6 +6116,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
CommandCounterIncrement();
}
}
+
+ pgstat_progress_end_command();
}
/*
@@ -6129,6 +6143,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
BulkInsertState bistate;
int ti_options;
ExprState *partqualstate = NULL;
+ int64 numTuples = 0;
/*
* Open the relation(s). We have surely already locked the existing
@@ -6138,6 +6153,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
oldTupDesc = tab->oldDesc;
newTupDesc = RelationGetDescr(oldrel); /* includes all mods */
+ /* Update progress reporting - we are actually scanning and possibly rewriting the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(oldrel));
+
if (OidIsValid(OIDNewHeap))
{
Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
@@ -6245,6 +6264,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleTableSlot *oldslot;
TupleTableSlot *newslot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
MemoryContext oldCxt;
List *dropped_attrs = NIL;
ListCell *lc;
@@ -6344,6 +6364,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
*/
snapshot = RegisterSnapshot(GetLatestSnapshot());
scan = table_beginscan(oldrel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
+ heapScan->rs_nblocks);
/*
* Switch to per-tuple memory context and reset it for each tuple
@@ -6354,6 +6377,13 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
if (tab->rewrite > 0)
{
@@ -6402,6 +6432,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ExecStoreVirtualTuple(newslot);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
/*
* Now, evaluate any expressions whose inputs come from the
* new tuple. We assume these columns won't reference each
@@ -6522,6 +6555,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
CHECK_FOR_INTERRUPTS();
}
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ heapScan->rs_nblocks);
MemoryContextSwitchTo(oldCxt);
table_endscan(scan);
@@ -13638,10 +13673,12 @@ validateForeignKeyConstraint(char *conname,
{
TupleTableSlot *slot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
Trigger trig = {0};
Snapshot snapshot;
MemoryContext oldcxt;
MemoryContext perTupCxt;
+ int64 numTuples = 0;
ereport(DEBUG1,
(errmsg_internal("validating foreign key constraint \"%s\"", conname)));
@@ -13660,6 +13697,10 @@ validateForeignKeyConstraint(char *conname,
trig.tginitdeferred = false;
/* we needn't fill in remaining fields */
+ /* Report that we are now checking foreign keys */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_CHECK_FKEYS);
+
/*
* See if we can do it with a single LEFT JOIN query. A false result
* indicates we must proceed with the fire-the-trigger method. We can't do
@@ -13677,6 +13718,7 @@ validateForeignKeyConstraint(char *conname,
snapshot = RegisterSnapshot(GetLatestSnapshot());
slot = table_slot_create(rel, NULL);
scan = table_beginscan(rel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
perTupCxt = AllocSetContextCreate(CurrentMemoryContext,
"validateForeignKeyConstraint",
@@ -13712,6 +13754,14 @@ validateForeignKeyConstraint(char *conname,
RI_FKey_check_ins(fcinfo);
MemoryContextReset(perTupCxt);
+
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
}
MemoryContextSwitchTo(oldcxt);
@@ -16776,6 +16826,10 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
*/
rel = relation_open(tableOid, lockmode);
+ /* Update progress reporting - we are copying the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(rel));
+
/* Check first if relation can be moved to new tablespace */
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
{
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 7c736e7b03b..56585742838 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -74,10 +74,12 @@
#define PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES 5
#define PROGRESS_CLUSTER_PHASE_REBUILD_INDEX 6
#define PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP 7
+#define PROGRESS_CLUSTER_PHASE_CHECK_FKEYS 8
/* Commands of PROGRESS_CLUSTER */
#define PROGRESS_CLUSTER_COMMAND_CLUSTER 1
#define PROGRESS_CLUSTER_COMMAND_VACUUM_FULL 2
+#define PROGRESS_CLUSTER_COMMAND_ALTER_TABLE 3 /* New command type */
/* Progress parameters for CREATE INDEX */
/* 3, 4 and 5 reserved for "waitfor" metrics */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 6cf828ca8d0..3b8a0ec0756 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1982,6 +1982,7 @@ pg_stat_progress_cluster| SELECT s.pid,
CASE s.param1
WHEN 1 THEN 'CLUSTER'::text
WHEN 2 THEN 'VACUUM FULL'::text
+ WHEN 3 THEN 'ALTER TABLE'::text
ELSE NULL::text
END AS command,
CASE s.param2
@@ -1993,6 +1994,7 @@ pg_stat_progress_cluster| SELECT s.pid,
WHEN 5 THEN 'swapping relation files'::text
WHEN 6 THEN 'rebuilding index'::text
WHEN 7 THEN 'performing final cleanup'::text
+ WHEN 8 THEN 'checking foreign key constraints'::text
ELSE NULL::text
END AS phase,
(s.param3)::oid AS cluster_index_relid,
--
2.34.1
Re: [PATCH] Support for basic ALTER TABLE progress reporting.
hi.
within ATRewriteTable we have:
pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
RelationGetNumberOfBlocks(oldrel));
pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
heapScan->rs_nblocks);
PROGRESS_CLUSTER_TOTAL_HEAP_BLKS
value is fixed, we only need to call pgstat_progress_update_param once here?
another patch [1]https://commitfest.postgresql.org/patch/5117 is expected to refactor pg_stat_progress_cluster a lot, so I'm
unsure whether it's a good idea to put CLUSTER, VACUUM FULL, or ALTER TABLE into
pg_stat_progress_cluster.
alternatively, we could introduce a separate progress report specifically for
ALTER TABLE, allowing us to distinguish between table rewrite and table
scan.
Fwd: [PATCH] Support for basic ALTER TABLE progress reporting.
I did not add CC to the list to my reply so forwarding..
---------- Forwarded message ---------
From: Jiří Kavalík <jkavalik@gmail.com>
Date: Sun, Jul 20, 2025 at 8:22 PM
Subject: Re: [PATCH] Support for basic ALTER TABLE progress reporting.
To: jian he <jian.universality@gmail.com>
Hello,
On Tue, Jul 8, 2025 at 3:42 PM jian he <jian.universality@gmail.com> wrote:
hi.
within ATRewriteTable we have:
pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
RelationGetNumberOfBlocks(oldrel));
pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
heapScan->rs_nblocks);PROGRESS_CLUSTER_TOTAL_HEAP_BLKS
value is fixed, we only need to call pgstat_progress_update_param once
here?
Yes, that was redundant, removed.
another patch [1] is expected to refactor pg_stat_progress_cluster a lot,
so I'm
unsure whether it's a good idea to put CLUSTER, VACUUM FULL, or ALTER
TABLE into
pg_stat_progress_cluster.
alternatively, we could introduce a separate progress report specifically
for
ALTER TABLE, allowing us to distinguish between table rewrite and table
scan.
[1]: https://commitfest.postgresql.org/patch/5117
I noticed that but not sure if it is targeting v19? I hoped to make the
change as small as possible, but if it would collide with the refactoring
then it makes sense to separate the functionality.
I am attaching the updated patch for the current "minimal" version for now.
But I will look into making it a standalone feature.
Thank you for your insights.
Best regards
jkavalik
Attachments:
0003-ALTER-TABLE-progress-support.patchapplication/x-patch; name=0003-ALTER-TABLE-progress-support.patchDownload
From 8d2ca7a6510dc646045cc3be552a61941ff28c6e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Kaval=C3=ADk?= <jkavalik@gmail.com>
Date: Sun, 25 May 2025 23:23:56 +0200
Subject: [PATCH] ALTER TABLE progress support
---
doc/src/sgml/monitoring.sgml | 13 +++----
doc/src/sgml/ref/alter_table.sgml | 10 ++++++
src/backend/catalog/storage.c | 7 ++++
src/backend/catalog/system_views.sql | 2 ++
src/backend/commands/tablecmds.c | 53 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 2 ++
src/test/regress/expected/rules.out | 2 ++
7 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 823afe1b30b..02d70a86731 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -400,7 +400,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<row>
<entry><structname>pg_stat_progress_cluster</structname><indexterm><primary>pg_stat_progress_cluster</primary></indexterm></entry>
<entry>One row for each backend running
- <command>CLUSTER</command> or <command>VACUUM FULL</command>, showing current progress.
+ <command>CLUSTER</command>, <command>VACUUM FULL</command> or <command>ALTER TABLE</command>, showing current progress.
See <xref linkend="cluster-progress-reporting"/>.
</entry>
</row>
@@ -5494,7 +5494,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<productname>PostgreSQL</productname> has the ability to report the progress of
certain commands during command execution. Currently, the only commands
which support progress reporting are <command>ANALYZE</command>,
- <command>CLUSTER</command>,
+ <command>CLUSTER</command>, <command>ALTER TABLE</command>,
<command>CREATE INDEX</command>, <command>VACUUM</command>,
<command>COPY</command>,
and <xref linkend="protocol-replication-base-backup"/> (i.e., replication
@@ -5740,8 +5740,9 @@ FROM pg_stat_get_backend_idset() AS backendid;
</indexterm>
<para>
- Whenever <command>CLUSTER</command> or <command>VACUUM FULL</command> is
- running, the <structname>pg_stat_progress_cluster</structname> view will
+ Whenever <command>CLUSTER</command>, <command>VACUUM FULL</command>
+ or <command>ALTER TABLE</command> is running,
+ the <structname>pg_stat_progress_cluster</structname> view will
contain a row for each backend that is currently running either command.
The tables below describe the information that will be reported and
provide information about how to interpret it.
@@ -5803,7 +5804,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>command</structfield> <type>text</type>
</para>
<para>
- The command that is running. Either <literal>CLUSTER</literal> or <literal>VACUUM FULL</literal>.
+ The command that is running. Either <literal>CLUSTER</literal>, <literal>VACUUM FULL</literal> or <literal>ALTER TABLE</literal>.
</para></entry>
</row>
@@ -5886,7 +5887,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
</table>
<table id="cluster-phases">
- <title>CLUSTER and VACUUM FULL Phases</title>
+ <title>CLUSTER, VACUUM FULL and ALTER TABLE Phases</title>
<tgroup cols="2">
<colspec colname="col1" colwidth="1*"/>
<colspec colname="col2" colwidth="2*"/>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1e4f26c13f6..865b14ba7d9 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1859,6 +1859,16 @@ ALTER TABLE measurement
</para>
</refsect1>
+ <refsect1>
+ <title>Progress Reporting</title>
+ <para>
+ When an <command>ALTER TABLE</command> operation rewrites the table, progress
+ can be monitored via the <literal>pg_stat_progress_cluster</literal> system view,
+ similar to <command>CLUSTER</command> and <command>VACUUM FULL</command> commands.
+ The command type will be reported as <literal>'ALTER TABLE'</literal>.
+ </para>
+ </refsect1>
+
<refsect1>
<title>See Also</title>
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 227df90f89c..79d14d86bc9 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -26,6 +26,7 @@
#include "access/xlogutils.h"
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/bulk_write.h"
@@ -505,6 +506,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
nblocks = smgrnblocks(src, forkNum);
+ /* Report expected number of block to copy */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, nblocks);
+
for (blkno = 0; blkno < nblocks; blkno++)
{
BulkWriteBuffer buf;
@@ -556,6 +560,9 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
* page including any unused space.
*/
smgr_bulk_write(bulkstate, blkno, buf, false);
+
+ /* Update progress report */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, blkno + 1);
}
smgr_bulk_finish(bulkstate);
}
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index b2d5332effc..9d68d02ec26 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1260,6 +1260,7 @@ CREATE VIEW pg_stat_progress_cluster AS
S.relid AS relid,
CASE S.param1 WHEN 1 THEN 'CLUSTER'
WHEN 2 THEN 'VACUUM FULL'
+ WHEN 3 THEN 'ALTER TABLE'
END AS command,
CASE S.param2 WHEN 0 THEN 'initializing'
WHEN 1 THEN 'seq scanning heap'
@@ -1269,6 +1270,7 @@ CREATE VIEW pg_stat_progress_cluster AS
WHEN 5 THEN 'swapping relation files'
WHEN 6 THEN 'rebuilding index'
WHEN 7 THEN 'performing final cleanup'
+ WHEN 8 THEN 'checking foreign key constraints'
END AS phase,
CAST(S.param3 AS oid) AS cluster_index_relid,
S.param4 AS heap_tuples_scanned,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb811520c29..43376539209 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -59,6 +59,7 @@
#include "commands/comment.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/sequence.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
@@ -5838,6 +5839,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
+ /* Start progress reporting */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tab->relid);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND, PROGRESS_CLUSTER_COMMAND_ALTER_TABLE);
+
/*
* If we change column data types, the operation has to be propagated
* to tables that use this table's rowtype as a column type.
@@ -5978,6 +5983,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
*/
ATRewriteTable(tab, OIDNewHeap);
+ /* Report that we are now swapping relation files */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES);
/*
* Swap the physical files of the old and new heaps, then rebuild
* indexes and discard the old heap. We can use RecentXmin for
@@ -6089,6 +6097,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
table_close(rel, NoLock);
}
+ /* Report that we are now doing clean up */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP);
+
/* Finally, run any afterStmts that were queued up */
foreach(ltab, *wqueue)
{
@@ -6103,6 +6115,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
CommandCounterIncrement();
}
}
+
+ pgstat_progress_end_command();
}
/*
@@ -6128,6 +6142,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
BulkInsertState bistate;
int ti_options;
ExprState *partqualstate = NULL;
+ int64 numTuples = 0;
/*
* Open the relation(s). We have surely already locked the existing
@@ -6137,6 +6152,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
oldTupDesc = tab->oldDesc;
newTupDesc = RelationGetDescr(oldrel); /* includes all mods */
+ /* Update progress reporting - we are actually scanning and possibly rewriting the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP);
+
if (OidIsValid(OIDNewHeap))
{
Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
@@ -6244,6 +6262,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
TupleTableSlot *oldslot;
TupleTableSlot *newslot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
MemoryContext oldCxt;
List *dropped_attrs = NIL;
ListCell *lc;
@@ -6343,6 +6362,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
*/
snapshot = RegisterSnapshot(GetLatestSnapshot());
scan = table_beginscan(oldrel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS,
+ heapScan->rs_nblocks);
/*
* Switch to per-tuple memory context and reset it for each tuple
@@ -6353,6 +6375,13 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
while (table_scan_getnextslot(scan, ForwardScanDirection, oldslot))
{
TupleTableSlot *insertslot;
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
if (tab->rewrite > 0)
{
@@ -6401,6 +6430,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
ExecStoreVirtualTuple(newslot);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN,
+ numTuples);
+
/*
* Now, evaluate any expressions whose inputs come from the
* new tuple. We assume these columns won't reference each
@@ -6521,6 +6553,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
CHECK_FOR_INTERRUPTS();
}
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ heapScan->rs_nblocks);
MemoryContextSwitchTo(oldCxt);
table_endscan(scan);
@@ -13664,10 +13698,12 @@ validateForeignKeyConstraint(char *conname,
{
TupleTableSlot *slot;
TableScanDesc scan;
+ HeapScanDesc heapScan;
Trigger trig = {0};
Snapshot snapshot;
MemoryContext oldcxt;
MemoryContext perTupCxt;
+ int64 numTuples = 0;
ereport(DEBUG1,
(errmsg_internal("validating foreign key constraint \"%s\"", conname)));
@@ -13686,6 +13722,10 @@ validateForeignKeyConstraint(char *conname,
trig.tginitdeferred = false;
/* we needn't fill in remaining fields */
+ /* Report that we are now checking foreign keys */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
+ PROGRESS_CLUSTER_PHASE_CHECK_FKEYS);
+
/*
* See if we can do it with a single LEFT JOIN query. A false result
* indicates we must proceed with the fire-the-trigger method. We can't do
@@ -13703,6 +13743,7 @@ validateForeignKeyConstraint(char *conname,
snapshot = RegisterSnapshot(GetLatestSnapshot());
slot = table_slot_create(rel, NULL);
scan = table_beginscan(rel, snapshot, 0, NULL);
+ heapScan = (HeapScanDesc) scan;
perTupCxt = AllocSetContextCreate(CurrentMemoryContext,
"validateForeignKeyConstraint",
@@ -13738,6 +13779,14 @@ validateForeignKeyConstraint(char *conname,
RI_FKey_check_ins(fcinfo);
MemoryContextReset(perTupCxt);
+
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+ (heapScan->rs_cblock +
+ heapScan->rs_nblocks -
+ heapScan->rs_startblock
+ ) % heapScan->rs_nblocks + 1);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_SCANNED,
+ ++numTuples);
}
MemoryContextSwitchTo(oldcxt);
@@ -16828,6 +16877,10 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
*/
rel = relation_open(tableOid, lockmode);
+ /* Update progress reporting - we are copying the table */
+ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_WRITE_NEW_HEAP);
+ pgstat_progress_update_param(PROGRESS_CLUSTER_TOTAL_HEAP_BLKS, RelationGetNumberOfBlocks(rel));
+
/* Check first if relation can be moved to new tablespace */
if (!CheckRelationTableSpaceMove(rel, newTableSpace))
{
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 7c736e7b03b..56585742838 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -74,10 +74,12 @@
#define PROGRESS_CLUSTER_PHASE_SWAP_REL_FILES 5
#define PROGRESS_CLUSTER_PHASE_REBUILD_INDEX 6
#define PROGRESS_CLUSTER_PHASE_FINAL_CLEANUP 7
+#define PROGRESS_CLUSTER_PHASE_CHECK_FKEYS 8
/* Commands of PROGRESS_CLUSTER */
#define PROGRESS_CLUSTER_COMMAND_CLUSTER 1
#define PROGRESS_CLUSTER_COMMAND_VACUUM_FULL 2
+#define PROGRESS_CLUSTER_COMMAND_ALTER_TABLE 3 /* New command type */
/* Progress parameters for CREATE INDEX */
/* 3, 4 and 5 reserved for "waitfor" metrics */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index dce8c672b40..b0d498d0339 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1986,6 +1986,7 @@ pg_stat_progress_cluster| SELECT s.pid,
CASE s.param1
WHEN 1 THEN 'CLUSTER'::text
WHEN 2 THEN 'VACUUM FULL'::text
+ WHEN 3 THEN 'ALTER TABLE'::text
ELSE NULL::text
END AS command,
CASE s.param2
@@ -1997,6 +1998,7 @@ pg_stat_progress_cluster| SELECT s.pid,
WHEN 5 THEN 'swapping relation files'::text
WHEN 6 THEN 'rebuilding index'::text
WHEN 7 THEN 'performing final cleanup'::text
+ WHEN 8 THEN 'checking foreign key constraints'::text
ELSE NULL::text
END AS phase,
(s.param3)::oid AS cluster_index_relid,
--
2.34.1
Import Notes
Reply to msg id not found: CAKMhz2kn9UpgQfVLVNXiot8vXtHjv1MKHP2vsyqK0jn8hC0w@mail.gmail.com
Re: Fwd: [PATCH] Support for basic ALTER TABLE progress reporting.
On 2025-Jul-21, Jiří Kavalík wrote:
I noticed that but not sure if it is targeting v19?
It is -- I'm working on that patch.
I hoped to make the change as small as possible, but if it would
collide with the refactoring then it makes sense to separate the
functionality.
I think from the users' point of view it makes little sense to report
the progress of ALTER TABLE rewriting in the same view as CLUSTER. This
is irrespective of the implementation difficulty. I mean, as a user
trying to monitor an operation, why would I be querying
pg_stat_progress_cluster? That's just weird. On the other hand, ALTER
TABLE has a need to report whether it's rewriting the table or just
reading it to recheck or validate constraints. If you try to cram that
bit of state in the CLUSTER report, it's going to look strange, because
it doesn't make sense for CLUSTER. I think the distinction is rather
important: for example if you're just checking constraints, you don't
need to rebuild indexes. This is information that the user would be
really happy to know.
In any case, having a separate progress.h definition isn't really all
that much extra work: it's just one more view and a few #defines to give
symbolic names to each counter.
Another point that might be worth keeping in mind is ALTER TABLE's
recursion mechanism for inheritance and partitioning. As a user I
imagine I would like to know how far we are not just in processing the
current relation, but also how many children are done and how many are
still left.
Also worth keeping in mind: some AT subcommands use the "standard"
recursion mechanism that simply adds extra subcommands to the queue for
each child relation, whereas other AT subcommands implement recursion at
execution time. Those work very differently.
What about multi-command ALTER TABLE? I think this is easy because we
preprocess the command to collect a list of actions to execute, and then
rnu a single check/rewrite scan on each table. So you may not need to
do anything special. But it's worth thinking about.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"People get annoyed when you try to debug them." (Larry Wall)