On 12/18/14, 3:02 PM, Alvaro Herrera wrote:
Andres Freund wrote:
On 2014-12-18 16:41:04 -0300, Alvaro Herrera wrote:
+                       if (scan_all)
+                               appendStringInfo(&buf, _("waited for %d buffer 
pins\n"),
+                                                                
vacrelstats->pinned_pages);
+                       else
+                               appendStringInfo(&buf,
+                                                                _("skipped %d pages 
due to buffer pins\n"),
+                                                                
vacrelstats->pinned_pages);

Unless I miss something this is, as mentioned before, not
correct. scan_all doesn't imply at all that we waited for buffer
pins. We only do so if lazy_check_needs_freeze(buf). Which usually won't
be true for a *significant* number of pages.

Ah, interesting, I didn't remember we had that.  I guess one possible
tweak is to discount the pages we skip from pinned_pages; or we could
keep a separate count of pages waited for.  Jim, up for a patch?

I would prefer that we at least count if we initially don't get the lock; 
presumably that number is always low anyway and in that case I think we're done 
with this. If it turns out it is common to initially miss the pin then we could 
do something fancier.

So how about if in the scan_all case we say something like "unable to initially 
acquire pin on %d buffers\n"?

(Happy to do the patch either way, but I'd like us to decide what we're doing 
first. ;)
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Reply via email to