On 2020/11/21 2:32, Matthias van de Meent wrote:
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.

Good catch! I agree that this is a bug.


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.

Thanks for the patch! It basically looks good to me.

It's a bit waste of cycles to calculate and update the number of scanned
blocks every cycles. So I'm inclined to change the code as follows.
Thought?

+       BlockNumber     prev_cblock = InvalidBlockNumber;
<snip>
+                       if (prev_cblock != heapScan->rs_cblock)
+                       {
+                               
pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_BLKS_SCANNED,
+                                                                                  
      (heapScan->rs_cblock +
+                                                                                  
       heapScan->rs_nblocks -
+                                                                                  
       heapScan->rs_startblock
+                                                                                  
              ) % heapScan->rs_nblocks + 1);
+                               prev_cblock = heapScan->rs_cblock;
+                       }

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


Reply via email to