From 78f7ccc9822bc5b2206c36308640e2aad94e4f55 Mon Sep 17 00:00:00 2001 From: "Vadim B. Mikheev" Date: Thu, 10 Jun 1999 14:17:12 +0000 Subject: [PATCH] 1. Fix for elog(ERROR, "EvalPlanQual: t_xmin is uncommitted ?!") and possibly for other cases too: DO NOT cache status of transaction in unknown state (i.e. non-committed and non-aborted ones) Example: T1 reads row updated/inserted by running T2 and cache T2 status. T2 commits. Now T1 reads a row updated by T2 and with HEAP_XMAX_COMMITTED in t_infomask (so cached T2 status is not changed). Now T1 EvalPlanQual gets updated row version without HEAP_XMIN_COMMITTED -> TransactionIdDidCommit(t_xmin) and TransactionIdDidAbort(t_xmin) return FALSE and T2 decides that t_xmin is not committed and gets ERROR above. It's too late to find more smart way to handle such cases and so I just changed xact status caching and got rid TransactionIdFlushCache() from code. Changed: transam.c, xact.c, lmgr.c and transam.h - last three just because of TransactionIdFlushCache() is removed. 2. heapam.c: T1 marked a row for update. T2 waits for T1 commit/abort. T1 commits. T3 updates the row before T2 locks row page. Now T2 sees that new row t_xmax is different from xact id (T1) T2 was waiting for. Old code did Assert here. New one goes to HeapTupleSatisfiesUpdate. Obvious changes too. 3. Added Assert to vacuum.c 4. bufmgr.c: break Assert(buf->r_locks == 0 && !buf->ri_lock) into two Asserts. --- src/backend/access/heap/heapam.c | 29 ++++++++++++++++++++++------- src/backend/access/transam/transam.c | 21 +++++++++------------ src/backend/access/transam/xact.c | 5 +---- src/backend/commands/vacuum.c | 4 +++- src/backend/storage/buffer/bufmgr.c | 5 +++-- src/backend/storage/lmgr/lmgr.c | 4 +--- src/include/access/transam.h | 3 +-- 7 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 1ea78ec863..39f6d223eb 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.43 1999/05/25 16:07:04 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.44 1999/06/10 14:17:05 vadim Exp $ * * * INTERFACE ROUTINES @@ -1152,8 +1152,13 @@ l1: LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); if (TransactionIdDidAbort(xwait)) goto l1; - /* concurrent xact committed */ - Assert(tp.t_data->t_xmax == xwait); + /* + * xwait is committed but if xwait had just marked + * the tuple for update then some other xaction could + * update this tuple before we got to this point. + */ + if (tp.t_data->t_xmax != xwait) + goto l1; if (!(tp.t_data->t_infomask & HEAP_XMAX_COMMITTED)) { tp.t_data->t_infomask |= HEAP_XMAX_COMMITTED; @@ -1242,8 +1247,13 @@ l2: LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); if (TransactionIdDidAbort(xwait)) goto l2; - /* concurrent xact committed */ - Assert(oldtup.t_data->t_xmax == xwait); + /* + * xwait is committed but if xwait had just marked + * the tuple for update then some other xaction could + * update this tuple before we got to this point. + */ + if (oldtup.t_data->t_xmax != xwait) + goto l2; if (!(oldtup.t_data->t_infomask & HEAP_XMAX_COMMITTED)) { oldtup.t_data->t_infomask |= HEAP_XMAX_COMMITTED; @@ -1359,8 +1369,13 @@ l3: LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE); if (TransactionIdDidAbort(xwait)) goto l3; - /* concurrent xact committed */ - Assert(tuple->t_data->t_xmax == xwait); + /* + * xwait is committed but if xwait had just marked + * the tuple for update then some other xaction could + * update this tuple before we got to this point. + */ + if (tuple->t_data->t_xmax != xwait) + goto l3; if (!(tuple->t_data->t_infomask & HEAP_XMAX_COMMITTED)) { tuple->t_data->t_infomask |= HEAP_XMAX_COMMITTED; diff --git a/src/backend/access/transam/transam.c b/src/backend/access/transam/transam.c index fe1d57d769..03bb786f4b 100644 --- a/src/backend/access/transam/transam.c +++ b/src/backend/access/transam/transam.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/transam/transam.c,v 1.26 1999/05/25 16:07:45 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/transam/transam.c,v 1.27 1999/06/10 14:17:06 vadim Exp $ * * NOTES * This file contains the high level access-method interface to the @@ -172,8 +172,14 @@ TransactionLogTest(TransactionId transactionId, /* transaction id to test */ if (!fail) { - TransactionIdStore(transactionId, &cachedTestXid); - cachedTestXidStatus = xidstatus; + /* + * DO NOT cache status for transactions in unknown state !!! + */ + if (xidstatus == XID_COMMIT || xidstatus == XID_ABORT) + { + TransactionIdStore(transactionId, &cachedTestXid); + cachedTestXidStatus = xidstatus; + } return (bool) (status == xidstatus); } @@ -576,12 +582,3 @@ TransactionIdAbort(TransactionId transactionId) TransactionLogUpdate(transactionId, XID_ABORT); } - -void -TransactionIdFlushCache() -{ - - TransactionIdStore(AmiTransactionId, &cachedTestXid); - cachedTestXidStatus = XID_COMMIT; - -} diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index d03136b508..4689a2f82e 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.40 1999/06/06 20:19:34 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.41 1999/06/10 14:17:06 vadim Exp $ * * NOTES * Transaction aborts can now occur two ways: @@ -514,8 +514,6 @@ CommandCounterIncrement() AtCommit_Cache(); AtStart_Cache(); - TransactionIdFlushCache(); - } void @@ -822,7 +820,6 @@ StartTransaction() { TransactionState s = CurrentTransactionState; - TransactionIdFlushCache(); FreeXactSnapshot(); XactIsoLevel = DefaultXactIsoLevel; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 7c1a8cd600..bfc9094b48 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.107 1999/06/06 20:19:34 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.108 1999/06/10 14:17:07 vadim Exp $ * *------------------------------------------------------------------------- */ @@ -1351,6 +1351,8 @@ vc_rpfheap(VRelStats *vacrelstats, Relation onerel, if (!ItemIdIsUsed(Pitemid)) elog(ERROR, "Parent itemid marked as unused"); Ptp.t_data = (HeapTupleHeader) PageGetItem(Ppage, Pitemid); + Assert(ItemPointerEquals(&(vtld.new_tid), + &(Ptp.t_data->t_ctid))); Assert(Ptp.t_data->t_xmax == tp.t_data->t_xmin); #ifdef NOT_USED /* I'm not sure that this will wotk properly... */ diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 9b7a41b4cb..f0b397dbea 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.53 1999/05/29 03:58:43 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.54 1999/06/10 14:17:09 vadim Exp $ * *------------------------------------------------------------------------- */ @@ -2007,7 +2007,8 @@ LockBuffer(Buffer buffer, int mode) else if (BufferLocks[buffer - 1] & BL_W_LOCK) { Assert(buf->w_lock); - Assert(buf->r_locks == 0 && !buf->ri_lock); + Assert(buf->r_locks == 0); + Assert(!buf->ri_lock); Assert(!(BufferLocks[buffer - 1] & (BL_R_LOCK | BL_RI_LOCK))) buf->w_lock = false; BufferLocks[buffer - 1] &= ~BL_W_LOCK; diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index f7ab4acffb..58ee47fe61 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lmgr.c,v 1.26 1999/05/31 01:48:13 vadim Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lmgr.c,v 1.27 1999/06/10 14:17:11 vadim Exp $ * *------------------------------------------------------------------------- */ @@ -324,8 +324,6 @@ XactLockTableWait(TransactionId xid) LockAcquire(LockTableId, &tag, ShareLock); LockRelease(LockTableId, &tag, ShareLock); - TransactionIdFlushCache(); - /* * Transaction was committed/aborted/crashed - we have to update * pg_log if transaction is still marked as running. diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 23fc2b59f0..2551278f12 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: transam.h,v 1.21 1999/05/25 22:42:35 momjian Exp $ + * $Id: transam.h,v 1.22 1999/06/10 14:17:12 vadim Exp $ * * NOTES * Transaction System Version 101 now support proper oid @@ -145,7 +145,6 @@ extern bool TransactionIdDidCommit(TransactionId transactionId); extern bool TransactionIdDidAbort(TransactionId transactionId); extern void TransactionIdCommit(TransactionId transactionId); extern void TransactionIdAbort(TransactionId transactionId); -extern void TransactionIdFlushCache(void); /* in transam/transsup.c */ extern void AmiTransactionOverride(bool flag); -- 2.11.0