Re: Poorly thought out code in vacuum

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.

Actually, on further review, I do see another problem: we need to pass
the scan_all flag down to lazy_vacuum_heap, and only do this
conditionally if that flag is not set. Otherwise we might skip a page
but still advance relfrozenxid, which would definitely be bad. But
that looks easy to fix, and I don't see any other reason why this
would be either unsafe or undesirable.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company