Re: Poorly thought out code in vacuum

On Fri, Jan 6, 2012 at 3:28 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jan 6, 2012 at 9:53 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On Fri, Jan 6, 2012 at 2:29 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Thu, Jan 5, 2012 at 7:37 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> I suppose Robert had something more intelligent in mind than a tight
>>>> loop when the buffer can't be exclusively locked, so maybe there is
>>>> some other change that should be made here instead.
>>>
>>> My intention was to skip the tuple, but I failed to notice the unusual
>>> way in which this loop iterates.  How about something like the
>>> attached?
>>
>> It solves the waiting issue, but leaves unused tuples in the heap that
>> previously would have been removed.
>>
>> I don't think that is a solution.
>
> Uh, we discussed this before the patch was committed, and you agreed
> it made sense to do that in the second heap scan just as we do it in
> the first heap scan.  If you now see a problem with that, that's fine,
> but please explain what the problem is, rather than just saying it's
> not acceptable for the patch to do the thing that the patch was
> designed to do.

My name is on that patch, so of course I accept responsibility for the
earlier decision making.

I *have* explained what is wrong. It "leaves unused tuples in the heap
that would previously have been removed".

More simply: lazy_vacuum_page() does some work and we can't skip that
work just because we don't get the lock. Elsewhere in the patch we
skipped getting the lock when there was no work to be done. In
lazy_vacuum_heap() we only visit pages that need further work, hence
every page is important.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services