agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedCLUSTER progress: wrong index_rebuild_count for tables with TOAST
6+ messages / 5 participants
[nested] [flat]
* CLUSTER progress: wrong index_rebuild_count for tables with TOAST
@ 2026-06-26 06:45 Adam Lee <adam8157@gmail.com>
2026-06-26 15:23 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Antonin Houska <ah@cybertec.at>
2026-08-27 16:01 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Nathan Bossart <nathandbossart@gmail.com>
2026-09-09 11:19 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Fujii Masao <masao.fujii@gmail.com>
0 siblings, 3 replies; 6+ messages in thread
From: Adam Lee @ 2026-06-26 06:45 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org; +Cc: Antonin Houska <ah@cybertec.at>; Álvaro Herrera <alvherre@kurilemu.de>
Hi,
When I run CLUSTER (or VACUUM FULL / REPACK) on a table that has a TOAST
table, pg_stat_progress_cluster shows a wrong index_rebuild_count. During
the heap scan the count is already equal to the number of indexes, but it
should be 0 until the indexes are rebuilt at the end.
A simple way to reproduce it. Run CLUSTER in one session, and query the
view from another session at the same time:
```
CREATE TABLE t (i int, x text);
INSERT INTO t SELECT g, repeat(md5(g::text), 1000)
FROM generate_series(1, 5) g;
CREATE INDEX ON t (i);
CLUSTER t USING t_i_idx;
-- phase "seq scanning heap", index_rebuild_count = 2 (should be 0)
```
The reason is that make_new_heap() creates the new TOAST table, and its
TOAST index, before the heap is scanned. Building that index reports
CREATE INDEX progress. CREATE INDEX and the cluster command use the same
progress field for different things, so the cluster command then shows a
CREATE INDEX value as its index_rebuild_count.
The fix is to not report progress for the TOAST index build, because it
is an internal index, not a user CREATE INDEX. The patch also adds an
isolation test that pauses CLUSTER at the start of the heap scan and
checks that index_rebuild_count is 0.
--
Adam
From 5d701f806b702d1003a4ca6ec56a98cb59b4920c Mon Sep 17 00:00:00 2001
From: Adam Lee <adam8157@gmail.com>
Date: Fri, 26 Jun 2026 14:31:25 +0800
Subject: [PATCH v1] Suppress CREATE INDEX progress while building a TOAST
index
create_toast_table() built the TOAST table's index without
INDEX_CREATE_SUPPRESS_PROGRESS, so index_build() reported CREATE INDEX
progress for it. By itself that is merely pointless -- a TOAST index is
never the target of a user-issued CREATE INDEX -- but it is actively
wrong when the TOAST table is created while another command is already
reporting progress for the same backend.
make_new_heap(), used by CLUSTER and VACUUM FULL, is exactly such a
case: it creates the new relation's TOAST table, and thus the TOAST
index, before the old heap is scanned. CREATE INDEX and REPACK share
the st_progress_param[] slots -- PROGRESS_CREATEIDX_PHASE and
PROGRESS_REPACK_INDEX_REBUILD_COUNT are both slot 9 -- so building the
TOAST index overwrote the in-progress cluster's index_rebuild_count.
pg_stat_progress_cluster therefore showed index_rebuild_count = 2 (the
numeric value of PROGRESS_CREATEIDX_PHASE_BUILD) while the command was
still scanning the heap, where it must read 0 until the indexes are
rebuilt at the end.
Pass INDEX_CREATE_SUPPRESS_PROGRESS for the TOAST index to keep the
build from touching the surrounding command's progress counters.
Also add an isolation test that suspends CLUSTER at the start of the
heap scan, using a new injection point, and confirms index_rebuild_count
is still 0 at that point.
---
src/backend/access/heap/heapam_handler.c | 7 +++
src/backend/catalog/toasting.c | 14 +++++-
src/test/modules/injection_points/Makefile | 1 +
.../expected/cluster_progress.out | 30 +++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/cluster_progress.spec | 50 +++++++++++++++++++
6 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/injection_points/expected/cluster_progress.out
create mode 100644 src/test/modules/injection_points/specs/cluster_progress.spec
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2268cc277bc..10c88953acb 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -45,6 +45,7 @@
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/rel.h"
#include "utils/tuplesort.h"
@@ -706,6 +707,12 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
slot = table_slot_create(OldHeap, NULL);
hslot = (BufferHeapTupleTableSlot *) slot;
+ /*
+ * Allow a test to observe progress reporting at this point, with the scan
+ * phase set but no tuples scanned yet.
+ */
+ INJECTION_POINT("heap-cluster-scan-start", NULL);
+
/*
* Scan through the OldHeap, either in OldIndex order or sequentially;
* copy each tuple into the NewHeap, or transiently to the tuplesort
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 4aa52a4bd25..d95a5ddf8c1 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -325,6 +325,17 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
coloptions[0] = 0;
coloptions[1] = 0;
+ /*
+ * Build the TOAST index with progress reporting suppressed. This is an
+ * internal index, never a user-issued CREATE INDEX, so reporting CREATE
+ * INDEX progress for it is meaningless. More importantly, a TOAST table
+ * (hence this index) can be created while another progress command is
+ * active for the same backend -- e.g. make_new_heap() during a
+ * CLUSTER/VACUUM FULL (REPACK). CREATE INDEX and REPACK share progress
+ * parameter slots (PROGRESS_CREATEIDX_PHASE vs
+ * PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an unsuppressed build would
+ * clobber the enclosing command's progress view.
+ */
index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid,
InvalidOid, InvalidOid,
indexInfo,
@@ -332,7 +343,8 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_SUPPRESS_PROGRESS,
+ 0, true, true, NULL);
table_close(toast_rel, NoLock);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..69ee1230046 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,6 +13,7 @@ REGRESS = injection_points hashagg reindex_conc vacuum
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
ISOLATION = basic \
+ cluster_progress \
inplace \
repack \
repack_temporal \
diff --git a/src/test/modules/injection_points/expected/cluster_progress.out b/src/test/modules/injection_points/expected/cluster_progress.out
new file mode 100644
index 00000000000..4fcf82ed402
--- /dev/null
+++ b/src/test/modules/injection_points/expected/cluster_progress.out
@@ -0,0 +1,30 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_cluster s2_progress s2_wakeup
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s1_cluster: CLUSTER cluster_progress USING cluster_progress_i; <waiting ...>
+step s2_progress:
+ SELECT relid::regclass, phase, index_rebuild_count
+ FROM pg_stat_progress_cluster;
+
+relid |phase |index_rebuild_count
+----------------+-----------------+-------------------
+cluster_progress|seq scanning heap| 0
+(1 row)
+
+step s2_wakeup: SELECT injection_points_wakeup('heap-cluster-scan-start');
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_cluster: <... completed>
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..3f4796102d1 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -44,6 +44,7 @@ tests += {
'isolation': {
'specs': [
'basic',
+ 'cluster_progress',
'inplace',
'repack',
'repack_temporal',
diff --git a/src/test/modules/injection_points/specs/cluster_progress.spec b/src/test/modules/injection_points/specs/cluster_progress.spec
new file mode 100644
index 00000000000..de9ead458ba
--- /dev/null
+++ b/src/test/modules/injection_points/specs/cluster_progress.spec
@@ -0,0 +1,50 @@
+# Verify pg_stat_progress_cluster.index_rebuild_count during the table-scan
+# phase of CLUSTER.
+#
+# A CLUSTER (REPACK) on a table that has a TOAST relation builds the new
+# table's TOAST index in make_new_heap(), before the heap is scanned. That
+# internal index build must not report CREATE INDEX progress into the
+# enclosing REPACK command: the two commands share progress slot 9
+# (PROGRESS_CREATEIDX_PHASE vs PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an
+# unsuppressed build leaves index_rebuild_count looking like a CREATE INDEX
+# phase value while the cluster is still scanning the heap. It must read 0
+# until indexes are actually rebuilt at the end.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+
+ -- A table with a TOAST relation (wide, toastable column) and an index to
+ -- cluster on.
+ CREATE TABLE cluster_progress (i int, t text);
+ INSERT INTO cluster_progress
+ SELECT g, repeat(md5(g::text), 1000) FROM generate_series(1, 5) g;
+ CREATE INDEX cluster_progress_i ON cluster_progress (i);
+}
+
+teardown
+{
+ DROP TABLE cluster_progress;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('heap-cluster-scan-start', 'wait');
+}
+step s1_cluster { CLUSTER cluster_progress USING cluster_progress_i; }
+teardown { SELECT injection_points_detach('heap-cluster-scan-start'); }
+
+session s2
+# CLUSTER is suspended at the start of the heap scan; index_rebuild_count must
+# be 0 here (indexes are rebuilt only at the end).
+step s2_progress
+{
+ SELECT relid::regclass, phase, index_rebuild_count
+ FROM pg_stat_progress_cluster;
+}
+step s2_wakeup { SELECT injection_points_wakeup('heap-cluster-scan-start'); }
+
+permutation s1_cluster s2_progress s2_wakeup
--
2.52.0
Attachments:
[text/plain] v1-0001-Suppress-CREATE-INDEX-progress-while-building-a-T.patch (8.4K, ../../aj4gJQMba0kClQmj@Mac/2-v1-0001-Suppress-CREATE-INDEX-progress-while-building-a-T.patch)
download | inline diff:
From 5d701f806b702d1003a4ca6ec56a98cb59b4920c Mon Sep 17 00:00:00 2001
From: Adam Lee <adam8157@gmail.com>
Date: Fri, 26 Jun 2026 14:31:25 +0800
Subject: [PATCH v1] Suppress CREATE INDEX progress while building a TOAST
index
create_toast_table() built the TOAST table's index without
INDEX_CREATE_SUPPRESS_PROGRESS, so index_build() reported CREATE INDEX
progress for it. By itself that is merely pointless -- a TOAST index is
never the target of a user-issued CREATE INDEX -- but it is actively
wrong when the TOAST table is created while another command is already
reporting progress for the same backend.
make_new_heap(), used by CLUSTER and VACUUM FULL, is exactly such a
case: it creates the new relation's TOAST table, and thus the TOAST
index, before the old heap is scanned. CREATE INDEX and REPACK share
the st_progress_param[] slots -- PROGRESS_CREATEIDX_PHASE and
PROGRESS_REPACK_INDEX_REBUILD_COUNT are both slot 9 -- so building the
TOAST index overwrote the in-progress cluster's index_rebuild_count.
pg_stat_progress_cluster therefore showed index_rebuild_count = 2 (the
numeric value of PROGRESS_CREATEIDX_PHASE_BUILD) while the command was
still scanning the heap, where it must read 0 until the indexes are
rebuilt at the end.
Pass INDEX_CREATE_SUPPRESS_PROGRESS for the TOAST index to keep the
build from touching the surrounding command's progress counters.
Also add an isolation test that suspends CLUSTER at the start of the
heap scan, using a new injection point, and confirms index_rebuild_count
is still 0 at that point.
---
src/backend/access/heap/heapam_handler.c | 7 +++
src/backend/catalog/toasting.c | 14 +++++-
src/test/modules/injection_points/Makefile | 1 +
.../expected/cluster_progress.out | 30 +++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../specs/cluster_progress.spec | 50 +++++++++++++++++++
6 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/injection_points/expected/cluster_progress.out
create mode 100644 src/test/modules/injection_points/specs/cluster_progress.spec
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 2268cc277bc..10c88953acb 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -45,6 +45,7 @@
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/rel.h"
#include "utils/tuplesort.h"
@@ -706,6 +707,12 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
slot = table_slot_create(OldHeap, NULL);
hslot = (BufferHeapTupleTableSlot *) slot;
+ /*
+ * Allow a test to observe progress reporting at this point, with the scan
+ * phase set but no tuples scanned yet.
+ */
+ INJECTION_POINT("heap-cluster-scan-start", NULL);
+
/*
* Scan through the OldHeap, either in OldIndex order or sequentially;
* copy each tuple into the NewHeap, or transiently to the tuplesort
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 4aa52a4bd25..d95a5ddf8c1 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -325,6 +325,17 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
coloptions[0] = 0;
coloptions[1] = 0;
+ /*
+ * Build the TOAST index with progress reporting suppressed. This is an
+ * internal index, never a user-issued CREATE INDEX, so reporting CREATE
+ * INDEX progress for it is meaningless. More importantly, a TOAST table
+ * (hence this index) can be created while another progress command is
+ * active for the same backend -- e.g. make_new_heap() during a
+ * CLUSTER/VACUUM FULL (REPACK). CREATE INDEX and REPACK share progress
+ * parameter slots (PROGRESS_CREATEIDX_PHASE vs
+ * PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an unsuppressed build would
+ * clobber the enclosing command's progress view.
+ */
index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid,
InvalidOid, InvalidOid,
indexInfo,
@@ -332,7 +343,8 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_SUPPRESS_PROGRESS,
+ 0, true, true, NULL);
table_close(toast_rel, NoLock);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index c01d2fb095c..69ee1230046 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,6 +13,7 @@ REGRESS = injection_points hashagg reindex_conc vacuum
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
ISOLATION = basic \
+ cluster_progress \
inplace \
repack \
repack_temporal \
diff --git a/src/test/modules/injection_points/expected/cluster_progress.out b/src/test/modules/injection_points/expected/cluster_progress.out
new file mode 100644
index 00000000000..4fcf82ed402
--- /dev/null
+++ b/src/test/modules/injection_points/expected/cluster_progress.out
@@ -0,0 +1,30 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_cluster s2_progress s2_wakeup
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s1_cluster: CLUSTER cluster_progress USING cluster_progress_i; <waiting ...>
+step s2_progress:
+ SELECT relid::regclass, phase, index_rebuild_count
+ FROM pg_stat_progress_cluster;
+
+relid |phase |index_rebuild_count
+----------------+-----------------+-------------------
+cluster_progress|seq scanning heap| 0
+(1 row)
+
+step s2_wakeup: SELECT injection_points_wakeup('heap-cluster-scan-start');
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s1_cluster: <... completed>
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 59dba1cb023..3f4796102d1 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -44,6 +44,7 @@ tests += {
'isolation': {
'specs': [
'basic',
+ 'cluster_progress',
'inplace',
'repack',
'repack_temporal',
diff --git a/src/test/modules/injection_points/specs/cluster_progress.spec b/src/test/modules/injection_points/specs/cluster_progress.spec
new file mode 100644
index 00000000000..de9ead458ba
--- /dev/null
+++ b/src/test/modules/injection_points/specs/cluster_progress.spec
@@ -0,0 +1,50 @@
+# Verify pg_stat_progress_cluster.index_rebuild_count during the table-scan
+# phase of CLUSTER.
+#
+# A CLUSTER (REPACK) on a table that has a TOAST relation builds the new
+# table's TOAST index in make_new_heap(), before the heap is scanned. That
+# internal index build must not report CREATE INDEX progress into the
+# enclosing REPACK command: the two commands share progress slot 9
+# (PROGRESS_CREATEIDX_PHASE vs PROGRESS_REPACK_INDEX_REBUILD_COUNT), so an
+# unsuppressed build leaves index_rebuild_count looking like a CREATE INDEX
+# phase value while the cluster is still scanning the heap. It must read 0
+# until indexes are actually rebuilt at the end.
+
+setup
+{
+ CREATE EXTENSION injection_points;
+
+ -- A table with a TOAST relation (wide, toastable column) and an index to
+ -- cluster on.
+ CREATE TABLE cluster_progress (i int, t text);
+ INSERT INTO cluster_progress
+ SELECT g, repeat(md5(g::text), 1000) FROM generate_series(1, 5) g;
+ CREATE INDEX cluster_progress_i ON cluster_progress (i);
+}
+
+teardown
+{
+ DROP TABLE cluster_progress;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('heap-cluster-scan-start', 'wait');
+}
+step s1_cluster { CLUSTER cluster_progress USING cluster_progress_i; }
+teardown { SELECT injection_points_detach('heap-cluster-scan-start'); }
+
+session s2
+# CLUSTER is suspended at the start of the heap scan; index_rebuild_count must
+# be 0 here (indexes are rebuilt only at the end).
+step s2_progress
+{
+ SELECT relid::regclass, phase, index_rebuild_count
+ FROM pg_stat_progress_cluster;
+}
+step s2_wakeup { SELECT injection_points_wakeup('heap-cluster-scan-start'); }
+
+permutation s1_cluster s2_progress s2_wakeup
--
2.52.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
@ 2026-06-26 15:23 ` Antonin Houska <ah@cybertec.at>
2 siblings, 0 replies; 6+ messages in thread
From: Antonin Houska @ 2026-06-26 15:23 UTC (permalink / raw)
To: Adam Lee <adam8157@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org; alvherre@kurilemu.de
Adam Lee <adam8157@gmail.com> wrote:
> The reason is that make_new_heap() creates the new TOAST table, and its
> TOAST index, before the heap is scanned. Building that index reports
> CREATE INDEX progress. CREATE INDEX and the cluster command use the same
> progress field for different things, so the cluster command then shows a
> CREATE INDEX value as its index_rebuild_count.
I'd prefer enhanced progress monitoring that I proposed earlier [1]. The patch
needs more work, I'll try to submit it for the next development cycle.
[1] https://www.postgresql.org/message-id/30939.1777888333%40localhost
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
@ 2026-08-27 16:01 ` Nathan Bossart <nathandbossart@gmail.com>
2 siblings, 0 replies; 6+ messages in thread
From: Nathan Bossart @ 2026-08-27 16:01 UTC (permalink / raw)
To: Adam Lee <adam8157@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Álvaro Herrera <alvherre@kurilemu.de>
On Fri, Jun 26, 2026 at 02:45:57PM +0800, Adam Lee wrote:
> When I run CLUSTER (or VACUUM FULL / REPACK) on a table that has a TOAST
> table, pg_stat_progress_cluster shows a wrong index_rebuild_count. During
> the heap scan the count is already equal to the number of indexes, but it
> should be 0 until the indexes are rebuilt at the end.
Hm. Isn't this a regression in v19 caused by commit 28d534e?
--
nathan
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
@ 2026-09-09 11:19 ` Fujii Masao <masao.fujii@gmail.com>
2026-09-13 12:52 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Álvaro Herrera <alvherre@kurilemu.de>
2 siblings, 1 reply; 6+ messages in thread
From: Fujii Masao @ 2026-09-09 11:19 UTC (permalink / raw)
To: Adam Lee <adam8157@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Álvaro Herrera <alvherre@kurilemu.de>
On Fri, Jun 26, 2026 at 3:46 PM Adam Lee <adam8157@gmail.com> wrote:
> The fix is to not report progress for the TOAST index build, because it
> is an internal index, not a user CREATE INDEX. The patch also adds an
> isolation test that pauses CLUSTER at the start of the heap scan and
> checks that index_rebuild_count is 0.
Thanks for the patch!
I think the approach in the patch, i.e., passing INDEX_CREATE_SUPPRESS_PROGRESS
to suppress progress reporting when creating TOAST indexes, looks good.
But, adding that test seems overkill to me. I'd prefer to simplify the patch
as in the attached 0001 patch. Thoughts?
While working on this patch, I also found a related but separate issue:
during REPACK (CONCURRENTLY), index_rebuild_count in
pg_stat_progress_repack and pg_stat_progress_cluster did not advance
as indexes were rebuilt. The attached 0002 patch fixes this issue.
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v2-0001-Suppress-progress-reporting-when-creating-TOAST-i.patch (1.9K, ../../CAHGQGwFUsrBvTurkSU8TEc=DZTunqteUXrasseqNMT7AMBZKRw@mail.gmail.com/2-v2-0001-Suppress-progress-reporting-when-creating-TOAST-i.patch)
download | inline diff:
From ec977948b4396be859d7d888bebe0e2c89799c99 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 9 Sep 2026 18:10:56 +0900
Subject: [PATCH v2 1/2] Suppress progress reporting when creating TOAST
indexes
Previously, when REPACK, CLUSTER, or VACUUM FULL created a transient TOAST
table, index_rebuild_count in pg_stat_progress_repack and
pg_stat_progress_cluster reported 2 before any indexes on the main table
had been rebuilt. This could mislead users monitoring the operation.
This happened because creating the TOAST index writes the CREATE INDEX phase
to the same progress slot used for the index rebuild count of the enclosing
command.
Fix this by passing INDEX_CREATE_SUPPRESS_PROGRESS when creating a TOAST
index, preserving the progress information of the enclosing command and
keeping the count at zero until the main-table indexes are rebuilt.
Backpatch-through: 19
---
src/backend/catalog/toasting.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 4aa52a4bd25..15a4267c64b 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -325,6 +325,10 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
coloptions[0] = 0;
coloptions[1] = 0;
+ /*
+ * Don't let index creation overwrite progress information for the command
+ * that caused the TOAST table to be created.
+ */
index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid,
InvalidOid, InvalidOid,
indexInfo,
@@ -332,7 +336,8 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_SUPPRESS_PROGRESS,
+ 0, true, true, NULL);
table_close(toast_rel, NoLock);
--
2.55.0
[application/octet-stream] v2-0002-Fix-index-rebuild-progress-reporting-for-REPACK-C.patch (1.2K, ../../CAHGQGwFUsrBvTurkSU8TEc=DZTunqteUXrasseqNMT7AMBZKRw@mail.gmail.com/3-v2-0002-Fix-index-rebuild-progress-reporting-for-REPACK-C.patch)
download | inline diff:
From 6125a57528451fb954e18cde65569ff1312d3273 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 9 Sep 2026 18:11:07 +0900
Subject: [PATCH v2 2/2] Fix index rebuild progress reporting for REPACK
(CONCURRENTLY)
Previously, during REPACK (CONCURRENTLY), index_rebuild_count in
pg_stat_progress_repack and pg_stat_progress_cluster did not advance
as indexes were rebuilt. This made it impossible for users monitoring the
operation to tell how many indexes had been completed.
Fix this by incrementing the count after each index is built on the new
heap, so that both views report the number of completed index builds.
Backpatch-through: 19
---
src/backend/commands/repack.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 57f28f1a420..9abe76a4815 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -3479,6 +3479,8 @@ build_new_indexes(Relation NewHeap, Relation OldHeap, List *OldIndexes)
result = lappend_oid(result, newindex);
index_close(ind, NoLock);
+
+ pgstat_progress_incr_param(PROGRESS_REPACK_INDEX_REBUILD_COUNT, 1);
}
return result;
--
2.55.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
2026-09-09 11:19 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Fujii Masao <masao.fujii@gmail.com>
@ 2026-09-13 12:52 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-09-16 09:42 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Fujii Masao <masao.fujii@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Álvaro Herrera @ 2026-09-13 12:52 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Adam Lee <adam8157@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On 2026-Sep-09, Fujii Masao wrote:
> I think the approach in the patch, i.e., passing INDEX_CREATE_SUPPRESS_PROGRESS
> to suppress progress reporting when creating TOAST indexes, looks good.
> But, adding that test seems overkill to me. I'd prefer to simplify the patch
> as in the attached 0001 patch. Thoughts?
I agree that the test is overkill -- after all, we don't test any of
progress reporting, and I'm not sure it's really a great approach to do
that by adding bespoke injection points.
Your 0001 looks good to me.
Maybe in a future release we can discuss a framework for making progress
updates visible in debug builds, so that they can be observed from a
new test framework.
> While working on this patch, I also found a related but separate issue:
> during REPACK (CONCURRENTLY), index_rebuild_count in
> pg_stat_progress_repack and pg_stat_progress_cluster did not advance
> as indexes were rebuilt. The attached 0002 patch fixes this issue.
Hmm, yeah, this patch looks good also.
I admit that the flow is a bit confusingly different in the concurrent
vs. non-concurrent cases: in the former, finish_heap_swap() is called
with reindex=false, so reindex_relation() is not called from there, and
instead we get these counter updates (with your patch) from
build_new_indexes(); in the concurrent patch, the counter updates come
from inside finish_heap_swap() instead.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"There's no problem so awful that you can't add some
guilt to it and make it even worse" (Calvin [& Hobbes])
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
2026-09-09 11:19 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Fujii Masao <masao.fujii@gmail.com>
2026-09-13 12:52 ` Re: CLUSTER progress: wrong index_rebuild_count for tables with TOAST Álvaro Herrera <alvherre@kurilemu.de>
@ 2026-09-16 09:42 ` Fujii Masao <masao.fujii@gmail.com>
0 siblings, 0 replies; 6+ messages in thread
From: Fujii Masao @ 2026-09-16 09:42 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Adam Lee <adam8157@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Sun, Sep 13, 2026 at 9:52 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> I agree that the test is overkill -- after all, we don't test any of
> progress reporting, and I'm not sure it's really a great approach to do
> that by adding bespoke injection points.
>
> Your 0001 looks good to me.
Thanks for the review! I've pushed the patch.
> Maybe in a future release we can discuss a framework for making progress
> updates visible in debug builds, so that they can be observed from a
> new test framework.
+1
> > While working on this patch, I also found a related but separate issue:
> > during REPACK (CONCURRENTLY), index_rebuild_count in
> > pg_stat_progress_repack and pg_stat_progress_cluster did not advance
> > as indexes were rebuilt. The attached 0002 patch fixes this issue.
>
> Hmm, yeah, this patch looks good also.
I've pushed this patch as well. Thanks!
Regards,
--
Fujii Masao
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-09-16 09:42 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-26 06:45 CLUSTER progress: wrong index_rebuild_count for tables with TOAST Adam Lee <adam8157@gmail.com>
2026-06-26 15:23 ` Antonin Houska <ah@cybertec.at>
2026-08-27 16:01 ` Nathan Bossart <nathandbossart@gmail.com>
2026-09-09 11:19 ` Fujii Masao <masao.fujii@gmail.com>
2026-09-13 12:52 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-09-16 09:42 ` Fujii Masao <masao.fujii@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox