Re: [PATCHES] Trivial patch to double vacuum speed on tables with no indexes

Gregory Stark wrote:
> 
> Bruce Momjian <bruce@momjian.us> writes:
> 
> > Tom Lane wrote:
> >> Bruce Momjian <bruce@momjian.us> writes:
> >> > Patch applied.  Thanks.
> >> 
> >> Wait a minute.   This patch changes the behavior so that
> >> LockBufferForCleanup is applied to *every* heap page, not only the ones
> >> where there are removable tuples.  It's not hard to imagine scenarios
> >> where that results in severe system-wide performance degradation.
> >> Has there been any real-world testing of this idea?
> >
> > I see the no-index case now:
> >
> > +               if (nindexes)
> > +                       LockBuffer(buf, BUFFER_LOCK_SHARE);
> > +               else
> > +                       LockBufferForCleanup(buf);
> >
> > Let's see what Greg says, or revert.
> 
> Hm, that's a good point. I could return it to the original method where it
> released the share lock and did he LockBufferForCleanup only if necessary. I
> thought it was awkward to acquire a lock then release it to acquire a
> different lock on the same buffer but it's true that it doesn't always have to
> acquire the second lock.

This rush to apply patches just because no one seems to be capable of
keeping up with them not being reviewed, is starting to get a bit
worrisome.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support