Re: Bug in VACUUM FULL ? - Mailing list pgsql-hackers

> "Pavan Deolasee" <pavan.deolasee@enterprisedb.com> writes:
>> The problem mentioned before is hard to reproduce with the
>> suggested change, but its not completely gone away. I have
>> seen that again on CVS HEAD with the patch applied.
>> I am facing another issue with VACUUM FULL. This
>> problem gets reproduced with HOT very easily, but takes
>> few attempts to reproduce with CVS HEAD, but it
>> certainly exists.

I've developed the attached patch against HEAD, and no longer see any
funny behavior.  Would appreciate it if you'd test some more, though.

            regards, tom lane

Index: vacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.348
diff -c -r1.348 vacuum.c
*** vacuum.c    13 Mar 2007 00:33:40 -0000    1.348
--- vacuum.c    13 Mar 2007 18:00:14 -0000
***************
*** 1880,1885 ****
--- 1880,1894 ----
               * To be on the safe side, we abandon the repair_frag process if
               * we cannot find the parent tuple in vtlinks.    This may be overly
               * conservative; AFAICS it would be safe to move the chain.
+              *
+              * Also, because we distinguish DEAD and RECENTLY_DEAD tuples
+              * using OldestXmin, which is a rather coarse test, it is quite
+              * possible to have an update chain in which a tuple we think is
+              * RECENTLY_DEAD links forward to one that is definitely DEAD.
+              * In such a case the RECENTLY_DEAD tuple must actually be dead,
+              * but it seems too complicated to try to make VACUUM remove it.
+              * We treat each contiguous set of RECENTLY_DEAD tuples as a
+              * separately movable chain, ignoring any intervening DEAD ones.
               */
              if (((tuple.t_data->t_infomask & HEAP_UPDATED) &&
                   !TransactionIdPrecedes(HeapTupleHeaderGetXmin(tuple.t_data),
***************
*** 1892,1897 ****
--- 1901,1907 ----
                  Buffer        Cbuf = buf;
                  bool        freeCbuf = false;
                  bool        chain_move_failed = false;
+                 bool        moved_target = false;
                  ItemPointerData Ctid;
                  HeapTupleData tp = tuple;
                  Size        tlen = tuple_len;
***************
*** 1919,1925 ****
                   * If this tuple is in the begin/middle of the chain then we
                   * have to move to the end of chain.  As with any t_ctid
                   * chase, we have to verify that each new tuple is really the
!                  * descendant of the tuple we came from.
                   */
                  while (!(tp.t_data->t_infomask & (HEAP_XMAX_INVALID |
                                                    HEAP_IS_LOCKED)) &&
--- 1929,1941 ----
                   * If this tuple is in the begin/middle of the chain then we
                   * have to move to the end of chain.  As with any t_ctid
                   * chase, we have to verify that each new tuple is really the
!                  * descendant of the tuple we came from; however, here we
!                  * need even more than the normal amount of paranoia.
!                  * If t_ctid links forward to a tuple determined to be DEAD,
!                  * then depending on where that tuple is, it might already
!                  * have been removed, and perhaps even replaced by a MOVED_IN
!                  * tuple.  We don't want to include any DEAD tuples in the
!                  * chain, so we have to recheck HeapTupleSatisfiesVacuum.
                   */
                  while (!(tp.t_data->t_infomask & (HEAP_XMAX_INVALID |
                                                    HEAP_IS_LOCKED)) &&
***************
*** 1933,1938 ****
--- 1949,1955 ----
                      OffsetNumber nextOffnum;
                      ItemId        nextItemid;
                      HeapTupleHeader nextTdata;
+                     HTSV_Result    nextTstatus;

                      nextTid = tp.t_data->t_ctid;
                      priorXmax = HeapTupleHeaderGetXmax(tp.t_data);
***************
*** 1963,1968 ****
--- 1980,1998 ----
                          ReleaseBuffer(nextBuf);
                          break;
                      }
+                     /* must check for DEAD or MOVED_IN tuple, too */
+                     nextTstatus = HeapTupleSatisfiesVacuum(nextTdata,
+                                                            OldestXmin,
+                                                            nextBuf);
+                     if (nextTstatus == HEAPTUPLE_DEAD ||
+                         nextTstatus == HEAPTUPLE_INSERT_IN_PROGRESS)
+                     {
+                         ReleaseBuffer(nextBuf);
+                         break;
+                     }
+                     /* if it's MOVED_OFF we shoulda moved this one with it */
+                     if (nextTstatus == HEAPTUPLE_DELETE_IN_PROGRESS)
+                         elog(ERROR, "updated tuple is already HEAP_MOVED_OFF");
                      /* OK, switch our attention to the next tuple in chain */
                      tp.t_data = nextTdata;
                      tp.t_self = nextTid;
***************
*** 2034,2039 ****
--- 2064,2074 ----
                      free_vtmove--;
                      num_vtmove++;

+                     /* Remember if we reached the original target tuple */
+                     if (ItemPointerGetBlockNumber(&tp.t_self) == blkno &&
+                         ItemPointerGetOffsetNumber(&tp.t_self) == offnum)
+                         moved_target = true;
+
                      /* Done if at beginning of chain */
                      if (!(tp.t_data->t_infomask & HEAP_UPDATED) ||
                       TransactionIdPrecedes(HeapTupleHeaderGetXmin(tp.t_data),
***************
*** 2102,2107 ****
--- 2137,2149 ----
                      ReleaseBuffer(Cbuf);
                  freeCbuf = false;

+                 /* Double-check that we will move the current target tuple */
+                 if (!moved_target && !chain_move_failed)
+                 {
+                     elog(DEBUG2, "failed to chain back to target --- cannot continue repair_frag");
+                     chain_move_failed = true;
+                 }
+
                  if (chain_move_failed)
                  {
                      /*