From 5d701f806b702d1003a4ca6ec56a98cb59b4920c Mon Sep 17 00:00:00 2001 From: Adam Lee 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; +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