Adjust time qual checking code so that we always check TransactionIdIsInProgress

before we check commit/abort status.  Formerly this was done in some paths
but not all, with the result that a transaction might be considered
committed for some purposes before it became committed for others.
Per example found by Jan Wieck.
This commit is contained in:
Tom Lane 2005-05-07 21:22:36 +00:00
parent 17eb867e98
commit aba1f93e45

View file

@ -11,12 +11,28 @@
* "hint" status bits if we see that the inserting or deleting transaction
* has now committed or aborted.
*
* NOTE: must check TransactionIdIsInProgress (which looks in PGPROC array)
* before TransactionIdDidCommit/TransactionIdDidAbort (which look in
* pg_clog). Otherwise we have a race condition: we might decide that a
* just-committed transaction crashed, because none of the tests succeed.
* xact.c is careful to record commit/abort in pg_clog before it unsets
* MyProc->xid in PGPROC array. That fixes that problem, but it also
* means there is a window where TransactionIdIsInProgress and
* TransactionIdDidCommit will both return true. If we check only
* TransactionIdDidCommit, we could consider a tuple committed when a
* later GetSnapshotData call will still think the originating transaction
* is in progress, which leads to application-level inconsistency. The
* upshot is that we gotta check TransactionIdIsInProgress first in all
* code paths, except for a few cases where we are looking at
* subtransactions of our own main transaction and so there can't be any
* race condition.
*
*
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.81 2004/12/31 22:02:56 pgsql Exp $
* $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.81.4.1 2005/05/07 21:22:36 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -144,20 +160,20 @@ HeapTupleSatisfiesItself(HeapTupleHeader tuple, Buffer buffer)
return false;
}
else if (!TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
return false;
}
else
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_COMMITTED;
SetBufferCommitInfoNeedsSave(buffer);
}
else
{
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return false;
}
}
/* by here, the inserting transaction has committed */
@ -179,13 +195,14 @@ HeapTupleSatisfiesItself(HeapTupleHeader tuple, Buffer buffer)
return false;
}
if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return true;
if (!TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
{
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return true;
}
@ -318,20 +335,20 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple, Buffer buffer)
else
return false; /* deleted before scan started */
}
else if (!TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
return false;
}
else
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_COMMITTED;
SetBufferCommitInfoNeedsSave(buffer);
}
else
{
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return false;
}
}
/* by here, the inserting transaction has committed */
@ -356,13 +373,14 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple, Buffer buffer)
return false; /* deleted before scan started */
}
if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return true;
if (!TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
{
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return true;
}
@ -532,20 +550,20 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
return HeapTupleInvisible; /* updated before scan
* started */
}
else if (!TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
return HeapTupleInvisible;
}
else
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_COMMITTED;
SetBufferCommitInfoNeedsSave(buffer);
}
else
{
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return HeapTupleInvisible;
}
}
/* by here, the inserting transaction has committed */
@ -571,16 +589,15 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
return HeapTupleInvisible; /* updated before scan started */
}
if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return HeapTupleBeingUpdated;
if (!TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
{
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return HeapTupleMayBeUpdated;
}
/* running xact */
return HeapTupleBeingUpdated; /* in updation by other */
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return HeapTupleMayBeUpdated;
}
/* xmax transaction committed */
@ -684,23 +701,24 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Buffer buffer)
return false;
}
else if (!TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return false;
}
SnapshotDirty->xmin = HeapTupleHeaderGetXmin(tuple);
/* XXX shouldn't we fall through to look at xmax? */
return true; /* in insertion by other */
}
else
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_COMMITTED;
SetBufferCommitInfoNeedsSave(buffer);
}
else
{
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return false;
}
}
/* by here, the inserting transaction has committed */
@ -723,17 +741,18 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Buffer buffer)
return false;
}
if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
{
SnapshotDirty->xmax = HeapTupleHeaderGetXmax(tuple);
return true;
}
if (!TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
{
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return true;
}
/* running xact */
SnapshotDirty->xmax = HeapTupleHeaderGetXmax(tuple);
return true; /* in updation by other */
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return true;
}
/* xmax transaction committed */
@ -847,20 +866,20 @@ HeapTupleSatisfiesSnapshot(HeapTupleHeader tuple, Snapshot snapshot,
else
return false; /* deleted before scan started */
}
else if (!TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
return false;
}
else
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
{
tuple->t_infomask |= HEAP_XMIN_COMMITTED;
SetBufferCommitInfoNeedsSave(buffer);
}
else
{
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMIN_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return false;
}
}
/*
@ -915,13 +934,14 @@ HeapTupleSatisfiesSnapshot(HeapTupleHeader tuple, Snapshot snapshot,
return false; /* deleted before scan started */
}
if (TransactionIdIsInProgress(HeapTupleHeaderGetXmax(tuple)))
return true;
if (!TransactionIdDidCommit(HeapTupleHeaderGetXmax(tuple)))
{
if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
{
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
}
/* it must have aborted or crashed */
tuple->t_infomask |= HEAP_XMAX_INVALID;
SetBufferCommitInfoNeedsSave(buffer);
return true;
}
@ -985,13 +1005,6 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
*
* If the inserting transaction aborted, then the tuple was never visible
* to any other transaction, so we can delete it immediately.
*
* NOTE: must check TransactionIdIsInProgress (which looks in PROC array)
* before TransactionIdDidCommit/TransactionIdDidAbort (which look in
* pg_clog). Otherwise we have a race condition where we might decide
* that a just-committed transaction crashed, because none of the
* tests succeed. xact.c is careful to record commit/abort in pg_clog
* before it unsets MyProc->xid in PROC array.
*/
if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
{