Re: [HACKERS] Missing increment of vacrelstats->pinskipped_pages

[Kyotaro HORIGUCHI]

Hello,

At Tue, 28 Mar 2017 14:15:06 -0400, Jim Nasby  wrote in 
<61de5a80-2ffd-6b05-61e4-210fcb299...@nasby.net>
> lazy_vacuum_heap() does not count pages that it skips due to not
> obtaining the buffer cleanup lock. vacuum_pinskipped.patch fixes
> that. That should be backpatched to 9.5.
> 
> vacuum_comment.patch cleans up a comment in lazy_scan_heap().

> --- a/src/backend/commands/vacuumlazy.c
> +++ b/src/backend/commands/vacuumlazy.c
> @@ -1404,6 +1404,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats 
> *vacrelstats)
>   if (!ConditionalLockBufferForCleanup(buf))
>   {
>   ReleaseBuffer(buf);
> + vacrelstats->pinskipped_pages++;
>   ++tupindex;
>   continue;


This is within a dead-tuple removal loop so pinskipped pages
shouldn't be increment here. If several dead-tuples are in a
page, the page will be counted as many times as the number of the
tuples.

On the contrary, the page must not have been skipped since the
dead tuples are the result of a scan on the page.

The skipped *tuples* just remain dead and shown as "# dead row
versios cannot be removed yet".

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Missing increment of vacrelstats->pinskipped_pages

[Jim Nasby]

lazy_vacuum_heap() does not count pages that it skips due to not 
obtaining the buffer cleanup lock. vacuum_pinskipped.patch fixes that. 
That should be backpatched to 9.5.


vacuum_comment.patch cleans up a comment in lazy_scan_heap().
--
Jim C. Nasby, Data Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net
diff --git a/src/backend/commands/vacuumlazy.c 
b/src/backend/commands/vacuumlazy.c
index 5b43a66bdc..6f7a5b4818 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -530,9 +530,9 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats 
*vacrelstats,
 * safely set for relfrozenxid or relminmxid.
 *
 * Before entering the main loop, establish the invariant that
-* next_unskippable_block is the next block number >= blkno that's not 
we
-* can't skip based on the visibility map, either all-visible for a
-* regular scan or all-frozen for an aggressive scan.  We set it to
+* next_unskippable_block is the next block number >= blkno that we
+* can't skip based on the visibility map (either all-visible for a
+* regular scan or all-frozen for an aggressive scan).  We set it to
 * nblocks if there's no such block.  We also set up the skipping_blocks
 * flag correctly at this stage.
 *
diff --git a/src/backend/commands/vacuumlazy.c 
b/src/backend/commands/vacuumlazy.c
index 5b43a66bdc..5a5a4ba48b 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -1404,6 +1404,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
if (!ConditionalLockBufferForCleanup(buf))
{
ReleaseBuffer(buf);
+   vacrelstats->pinskipped_pages++;
++tupindex;
continue;
}

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers