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
andin 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
somethingfancier.
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
