Re: Poorly thought out code in vacuum

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Poorly thought out code in vacuum
Date: 2012-01-06 17:24:55
Message-ID: 17477.1325870695@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2012-01-06 17:29:04 Re: Progress on fast path sorting, btree index creation time
Previous Message Dave Cramer 2012-01-06 17:18:01 pgsphere