On 2014-12-18 16:05:23 -0600, Jim Nasby wrote:
> 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.
Also, naming the number of pages we could *not* pin, pinned_pages?
Really?
pinskipped_pages,skipped_pages,unpinned_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?
This is still wrong. I think just counting skipped pages, without
distinct messages for waiting/not waiting, is good enough for
now. Everything else would only be actually meaningful if we actually
tracked the waiting time.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
