On Wed, 2007-08-08 at 23:23 -0400, Tom Lane wrote:
> I wrote:
> > ... Since we've whacked the tqual.c logic around recently,
> > the problem might actually lie there...
>
> In fact, I bet this is a result of the async-commit patch. The places
> where vacuum.c bleats "HEAP_MOVED_OFF was expected" are all places where
> it is looking at a tuple not marked XMIN_COMMITTED; it expects that
> after its first pass over the table, *every* tuple is either
> XMIN_COMMITTED or one that it moved. Async commit changed tqual.c
> so that tuples that are in fact known committed might not get marked
> XMIN_COMMITTED right away. The patch tries to prevent this from
> happening within VACUUM FULL by means of
>
> /*
> * VACUUM FULL assumes that all tuple states are well-known prior to
> * moving tuples around --- see comment "known dead" in repair_frag(),
> * as well as simplifications in tqual.c. So before we start we must
> * ensure that any asynchronously-committed transactions with changes
> * against this table have been flushed to disk. It's sufficient to do
> * this once after we've acquired AccessExclusiveLock.
> */
> XLogAsyncCommitFlush();
>
> but I bet lunch that that's not good enough. I still haven't reproduced
> it, but I'm thinking that the inexact bookkeeping that we created for
> clog page LSNs allows tuples to not get marked if the right sort of
> timing of concurrent transactions happens.
>
> Not sure about the best solution for this.
Good hunch. I plugged this hole earlier, but on further inspection I can
see the plug wasn't wide enough. XLogAsyncCommitFlush() is good enough,
but HeapTupleSatisfiesVacuum() still allowed the inexact bookkeeping to
sometimes skip hint bit setting, when executed with concurrent
transactions touching other tables.
ISTM that if we call HeapTupleSatisfiesVacuum() with an additional
boolean parameter, force, we can tell VF to always set the hint bits in
every case, not just HEAP_MOVED_IN and HEAP_MOVED_OUT.
Patch enclosed, but a little crufty. Gotta run now, talk later.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.354
diff -c -r1.354 vacuum.c
*** src/backend/commands/vacuum.c 1 Aug 2007 22:45:08 -0000 1.354
--- src/backend/commands/vacuum.c 9 Aug 2007 07:24:41 -0000
***************
*** 1384,1390 ****
tuple.t_len = ItemIdGetLength(itemid);
ItemPointerSet(&(tuple.t_self), blkno, offnum);
! switch (HeapTupleSatisfiesVacuum(tuple.t_data, OldestXmin, buf))
{
case HEAPTUPLE_DEAD:
tupgone = true; /* we can delete the tuple */
--- 1384,1390 ----
tuple.t_len = ItemIdGetLength(itemid);
ItemPointerSet(&(tuple.t_self), blkno, offnum);
! switch (HeapTupleSatisfiesVacuumFull(tuple.t_data, OldestXmin, buf))
{
case HEAPTUPLE_DEAD:
tupgone = true; /* we can delete the tuple */
***************
*** 1998,2004 ****
break;
}
/* must check for DEAD or MOVED_IN tuple, too */
! nextTstatus = HeapTupleSatisfiesVacuum(nextTdata,
OldestXmin,
nextBuf);
if (nextTstatus == HEAPTUPLE_DEAD ||
--- 1998,2004 ----
break;
}
/* must check for DEAD or MOVED_IN tuple, too */
! nextTstatus = HeapTupleSatisfiesVacuumFull(nextTdata,
OldestXmin,
nextBuf);
if (nextTstatus == HEAPTUPLE_DEAD ||
Index: src/backend/utils/time/tqual.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/time/tqual.c,v
retrieving revision 1.103
diff -c -r1.103 tqual.c
*** src/backend/utils/time/tqual.c 1 Aug 2007 22:45:09 -0000 1.103
--- src/backend/utils/time/tqual.c 9 Aug 2007 07:24:43 -0000
***************
*** 77,82 ****
--- 77,85 ----
TransactionId RecentGlobalXmin = InvalidTransactionId;
/* local functions */
+ static HTSV_Result HeapTupleSatisfiesVacuumInternal(HeapTupleHeader tuple,
+ TransactionId OldestXmin, Buffer buffer, bool locked);
+
static bool XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot);
***************
*** 1045,1054 ****
* deleted by XIDs >= OldestXmin are deemed "recently dead"; they might
* still be visible to some open transaction, so we can't remove them,
* even if we see that the deleting transaction has committed.
*/
HTSV_Result
! HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
! Buffer buffer)
{
/*
* Has inserting transaction committed?
--- 1048,1060 ----
* deleted by XIDs >= OldestXmin are deemed "recently dead"; they might
* still be visible to some open transaction, so we can't remove them,
* even if we see that the deleting transaction has committed.
+ *
+ * If the heap we are checking is exclusively locked, we can skip certain checks
+ * because we know that no transactions that wrote data are still active.
*/
HTSV_Result
! HeapTupleSatisfiesVacuumInternal(HeapTupleHeader tuple, TransactionId OldestXmin,
! Buffer buffer, bool locked)
{
/*
* Has inserting transaction committed?
***************
*** 1106,1112 ****
}
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
HeapTupleSetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
! HeapTupleHeaderGetXmin(tuple));
else
{
/*
--- 1112,1119 ----
}
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
HeapTupleSetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
! (locked ? InvalidTransactionId :
! HeapTupleHeaderGetXmin(tuple)));
else
{
/*
***************
*** 1144,1155 ****
{
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
{
! if (MultiXactIdIsRunning(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_LIVE;
}
else
{
! if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_LIVE;
}
--- 1151,1162 ----
{
if (tuple->t_infomask & HEAP_XMAX_IS_MULTI)
{
! if (!locked && MultiXactIdIsRunning(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_LIVE;
}
else
{
! if (!locked && TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_LIVE;
}
***************
*** 1173,1183 ****
if (!(tuple->t_infomask & HEAP_XMAX_COMMITTED))
{
! if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_DELETE_IN_PROGRESS;
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
HeapTupleSetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED,
! HeapTupleHeaderGetXmax(tuple));
else
{
/*
--- 1180,1191 ----
if (!(tuple->t_infomask & HEAP_XMAX_COMMITTED))
{
! if (!locked && TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return HEAPTUPLE_DELETE_IN_PROGRESS;
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
HeapTupleSetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED,
! (locked ? InvalidTransactionId :
! HeapTupleHeaderGetXmax(tuple)));
else
{
/*
***************
*** 1221,1226 ****
--- 1229,1247 ----
return HEAPTUPLE_DEAD;
}
+ HTSV_Result
+ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
+ Buffer buffer)
+ {
+ return HeapTupleSatisfiesVacuumInternal(tuple, OldestXmin, buffer, false);
+ }
+
+ HTSV_Result
+ HeapTupleSatisfiesVacuumFull(HeapTupleHeader tuple, TransactionId OldestXmin,
+ Buffer buffer)
+ {
+ return HeapTupleSatisfiesVacuumInternal(tuple, OldestXmin, buffer, true);
+ }
/*
* GetTransactionSnapshot
Index: src/include/utils/tqual.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/tqual.h,v
retrieving revision 1.67
diff -c -r1.67 tqual.h
*** src/include/utils/tqual.h 25 Jul 2007 12:22:54 -0000 1.67
--- src/include/utils/tqual.h 9 Aug 2007 07:24:45 -0000
***************
*** 144,149 ****
--- 144,151 ----
CommandId curcid, Buffer buffer);
extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTupleHeader tuple,
TransactionId OldestXmin, Buffer buffer);
+ extern HTSV_Result HeapTupleSatisfiesVacuumFull(HeapTupleHeader tuple,
+ TransactionId OldestXmin, Buffer buffer);
extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
