Re: Poorly thought out code in vacuum

On Fri, Jan 6, 2012 at 10:51 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> 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.

That is not true.  We skip vacuumable pages in the first heap pass
even if they contain dead tuples, unless scan_all is set or we can get
the buffer lock without waiting.

> In
> lazy_vacuum_heap() we only visit pages that need further work, hence
> every page is important.

If the system were to crash after the first heap pass and the index
vacuuming, but before the second heap pass, nothing bad would happen.
The next vacuum would collect those same dead tuples, scan the indexes
for them (and not find them, since we already removed them), and then
remove them from the heap.

We already made a decision that it's a good idea to skip vacuuming a
page on which we can't immediately obtain a cleanup lock, because
waiting for the cleanup lock can cause the autovacuum worker to get
stuck indefinitely, starving the table and in some cases the entire
cluster of adequate vacuuming.  That logic is just as valid in the
second heap pass as it is in the first one.

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