Re: heap vacuum & cleanup locks

On Tue, Nov 8, 2011 at 10:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Interesting idea.  I think in general we insist that you must have a
>> buffer content lock to inspect the tuple visibility info, in which
>> case that would be safe.  But I'm not sure we do that absolutely
>> everywhere.  For instance, just last night I noticed this:
>
>>                         /*
>>                          * If xmin isn't what we're expecting, the
>> slot must have been
>>                          * recycled and reused for an unrelated tuple.
>>  This implies that
>>                          * the latest version of the row was deleted,
>> so we need do
>>                          * nothing.  (Should be safe to examine xmin
>> without getting
>>                          * buffer's content lock, since xmin never
>> changes in an existing
>>                          * tuple.)
>>                          */
>>                         if
>
> Hmm ... I think that code is OK but the comment needs work.  Here we are
> necessarily looking for a pretty recent value of xmin (it has to be
> later than GlobalXmin), so there's no need to worry that it might get
> changed to FrozenXID.

OK.  Here's another possible concern: what happens if the page we're
freezing contains a dead tuple?  It looks to me like
heap_freeze_tuple() is written so as not to require a cleanup lock -
indeed, the comments claim it's called when holding only a share lock
on the buffer, which doesn't appear to match what lazy_scan_heap() is
actually doing.  But it does seem to assume that any tuples that still
exist are all-visible, which only works if vacuum has already pruned
the page.

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