Potential vacuum bug? - Mailing list pgsql-hackers

From Tom Lane
Subject Potential vacuum bug?
Date
Msg-id 3263.947519047@sss.pgh.pa.us
Whole thread Raw
Responses RE: [HACKERS] Potential vacuum bug?
List pgsql-hackers
While chasing the VACUUM problem reported by Stephen Birch, I noticed
something that looks like a potential trouble spot.  Vacuum's initial
scan routine, vc_scanheap, runs through the table to mark tuples as
known committed or known dead, if possible (consulting the transaction
log for tuples not yet so marked).  It does the right things as far as
marking committed/dead if it sees a tuple marked HEAP_MOVED_OFF or
HEAP_MOVED_IN, which could only be there if a prior VACUUM failed
partway through.  But it doesn't *clear* those bits.  Seems to me that
that will screw up the subsequent vc_rpfheap procedure --- in
particular, leaving a HEAP_MOVED_IN flag set will cause vc_rpfheap to
complain (correctly!) about 'HEAP_MOVED_IN not expected', whereas
leaving HEAP_MOVED_OFF set will confuse vc_rpfheap because it will
think it moved the tuple itself.

In short, if we really want to recover from a failed VACUUM then we'd
better clear those bits during vc_scanheap.  I am thinking that the
code starting at about line 720 ought to look like
           if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))           {               if
(tuple.t_data->t_infomask& HEAP_XMIN_INVALID)                   tupgone = true;               else if
(tuple.t_data->t_infomask& HEAP_MOVED_OFF)               {                   // mark tuple commited or invalid as
appropriate,                  // same as before
 
add >>>             tuple.t_data->t_infomask &= ~HEAP_MOVED_OFF;               }               else if
(tuple.t_data->t_infomask& HEAP_MOVED_IN)               {                   // mark tuple commited or invalid as
appropriate,                  // same as before
 
add >>>             tuple.t_data->t_infomask &= ~HEAP_MOVED_IN;               }               else               {
            // other cases same as before               }           }
 

add >>>     if (tuple.t_data->t_infomask & (HEAP_MOVED_OFF | HEAP_MOVED_IN))
add >>>     {
add >>>         elog(NOTICE, "Clearing unexpected HEAP_MOVED flag");
add >>>         tuple.t_data->t_infomask &= ~(HEAP_MOVED_OFF | HEAP_MOVED_IN);
add >>>         pgchanged = true;
add >>>     }

Comments anyone?
        regards, tom lane


pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Number of index fields configurable
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] Number of index fields configurable