To: Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,  pgsql-hackers@postgresql.org,
	pgsql-patches@postgresql.org
Subject: Re: [HACKERS] Trivial patch to double vacuum speed on tables with no
	indexes
In-Reply-To: <200609042147.k84Ll3P04850@momjian.us> (Bruce Momjian's message
	of "Mon, 4 Sep 2006 17:47:02 -0400 (EDT)")
References: <200609042147.k84Ll3P04850@momjian.us>
From: Gregory Stark <stark@enterprisedb.com>
Organization: EnterpriseDB
Date: Mon, 04 Sep 2006 23:01:32 +0100
Message-ID: <87bqpvi94z.fsf@stark.enterprisedb.com>
User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii


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.


-- 
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com