Hi, The pg_stat_progress_cluster view can report incorrect heap_blks_scanned values when synchronize_seqscans is enabled, because it allows the sequential heap scan to not start at block 0. This can result in wraparounds in the heap_blks_scanned column when the table scan wraps around, and starting the next phase with heap_blks_scanned != heap_blks_total. This issue was introduced with the pg_stat_progress_cluster view.
The attached patch fixes the issue by accounting for a non-0 heapScan->rs_startblock and calculating the correct number with a non-0 heapScan->rs_startblock in mind. The issue is reproducible starting from PG12 by stopping a CLUSTER-command while it is sequential-scanning the table (seqscan enforceable through enable_indexscan = off), and then re-starting the seqscanning CLUSTER command (without other load/seq-scans on the table). Any thoughts? Matthias van de Meent
From 12f7617f4999b7bf85cc2c9ef4c3b96aac3b26e8 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekew...@gmail.com> Date: Fri, 20 Nov 2020 16:23:59 +0100 Subject: [PATCH v1] Fix CLUSTER progress reporting of number of blocks scanned. The heapScan need not start at block 0, so heapScan->rs_cblock need not be the correct value for amount of blocks scanned. A more correct value is ((heapScan->rs_cblock - heapScan->rs_startblock + heapScan->rs_nblocks) % heapScan->rs_nblocks), as it accounts for the wraparound and the initial offset of the heapScan. Signed-off-by: Matthias van de Meent <boekew...@gmail.com> --- src/backend/access/heap/heapam_handler.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index dcaea7135f..3d0433633b 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -798,9 +798,17 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, /* * In scan-and-sort mode and also VACUUM FULL, set heap blocks * scanned + * + * Note that heapScan may start at an offset and wrap around, i.e. + * rs_startblock may be >0, and rs_cblock may end with a number + * below rs_startblock. To prevent showing this wraparound to the + * user, we offset rs_cblock by rs_startblock (modulo rs_nblocks). */ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED, - heapScan->rs_cblock + 1); + (heapScan->rs_cblock + + heapScan->rs_nblocks - + heapScan->rs_startblock + ) % heapScan->rs_nblocks + 1); } tuple = ExecFetchSlotHeapTuple(slot, false, NULL); -- 2.20.1
