Received: from magus.postgresql.org (magus.postgresql.org [87.238.57.229])
	by mail.postgresql.org (Postfix) with ESMTP id 33759B1BBF2
	for <pgsql-hackers@postgresql.org>;
	Fri,  6 Jan 2012 13:25:16 -0400 (AST)
Received: from sss.pgh.pa.us ([66.207.139.130])
	by magus.postgresql.org with esmtp (Exim 4.72)
	(envelope-from <tgl@sss.pgh.pa.us>) id 1RjDXT-0001C4-S5
	for pgsql-hackers@postgresql.org; Fri, 06 Jan 2012 17:25:14 +0000
Received: from sss2.sss.pgh.pa.us (tgl@localhost [127.0.0.1])
	by sss.pgh.pa.us (8.14.2/8.14.2) with ESMTP id q06HOtN5017478;
	Fri, 6 Jan 2012 12:24:55 -0500 (EST)
To: Robert Haas <robertmhaas@gmail.com>
cc: Simon Riggs <simon@2ndquadrant.com>, pgsql-hackers@postgresql.org
Subject: Re: Poorly thought out code in vacuum 
In-reply-to: 
 <CA+TgmobiR=sNTsknS8F8MNqEp1ftbCZHX_YkWEBDCH5+yfnpdQ@mail.gmail.com>
References: <25422.1325810273@sss.pgh.pa.us>
	<CA+TgmoYeYaZfa-oKqpu9Y=+vSdQf=kCcRxq-M30JPX4w+vkSpw@mail.gmail.com>
	<CA+U5nM+DZ9jraZ_wCUBqq4+5jpEhrEW13ySpYn40GZmAntM6-Q@mail.gmail.com>
	<CA+TgmobzJz9xRiF=At4nEy2DOnu-gXdeyfP1ajM4BNyFPQfVoA@mail.gmail.com>
	<CA+U5nM+E8WjQ0DbDogAuRupVny53xvhNcJjAV4cJ51iWKSnFKA@mail.gmail.com>
	<CA+TgmobiR=sNTsknS8F8MNqEp1ftbCZHX_YkWEBDCH5+yfnpdQ@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Fri, 06 Jan 2012 11:37:25 -0500"
Date: Fri, 06 Jan 2012 12:24:55 -0500
Message-ID: <17477.1325870695@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
X-Pg-Spam-Score: -1.9 (-)
X-Archive-Number: 201201/326
X-Sequence-Number: 200919

I started to wonder how likely it would be that some other process would
sit on a buffer pin for so long as to allow 134217727 iterations of
ReadBufferExtended, even given the slowdowns associated with
CLOBBER_CACHE_ALWAYS.  This led to some fruitless searching for possible
deadlock conditions, but eventually I realized that there's a much
simpler explanation: if PrivateRefCount > 1 then
ConditionalLockBufferForCleanup always fails.  This means that once
ConditionalLockBufferForCleanup has failed once, the currently committed
code in lazy_vacuum_heap is guaranteed to loop until it gets a failure
in enlarging the ResourceOwner buffer-reference array.  Which in turn
means that neither of you ever actually exercised the skip case, or
you'd have noticed the problem.  Nor are the current regression tests
well designed to exercise the case.  (There might well be failures of
this type happening from time to time in autovacuum, but we'd not see
any reported failure in the buildfarm, unless we went trawling in
postmaster logs.)

So at this point I've got serious doubts as to the quality of testing of
that whole patch, not just this part.

			regards, tom lane