Skip to content

Commit 78f7ccc

Browse files
vadim4ovadim4o
committed
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.
1 parent c37ecaf commit 78f7ccc

File tree

7 files changed

+40
-31
lines changed

7 files changed

+40
-31
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.43 1999/05/25 16:07:04 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.44 1999/06/10 14:17:05 vadim Exp $
1111
*
1212
*
1313
* INTERFACE ROUTINES
@@ -1152,8 +1152,13 @@ heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
11521152
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
11531153
if (TransactionIdDidAbort(xwait))
11541154
goto l1;
1155-
/* concurrent xact committed */
1156-
Assert(tp.t_data->t_xmax == xwait);
1155+
/*
1156+
* xwait is committed but if xwait had just marked
1157+
* the tuple for update then some other xaction could
1158+
* update this tuple before we got to this point.
1159+
*/
1160+
if (tp.t_data->t_xmax != xwait)
1161+
goto l1;
11571162
if (!(tp.t_data->t_infomask & HEAP_XMAX_COMMITTED))
11581163
{
11591164
tp.t_data->t_infomask |= HEAP_XMAX_COMMITTED;
@@ -1242,8 +1247,13 @@ heap_replace(Relation relation, ItemPointer otid, HeapTuple newtup,
12421247
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
12431248
if (TransactionIdDidAbort(xwait))
12441249
goto l2;
1245-
/* concurrent xact committed */
1246-
Assert(oldtup.t_data->t_xmax == xwait);
1250+
/*
1251+
* xwait is committed but if xwait had just marked
1252+
* the tuple for update then some other xaction could
1253+
* update this tuple before we got to this point.
1254+
*/
1255+
if (oldtup.t_data->t_xmax != xwait)
1256+
goto l2;
12471257
if (!(oldtup.t_data->t_infomask & HEAP_XMAX_COMMITTED))
12481258
{
12491259
oldtup.t_data->t_infomask |= HEAP_XMAX_COMMITTED;
@@ -1359,8 +1369,13 @@ heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer)
13591369
LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);
13601370
if (TransactionIdDidAbort(xwait))
13611371
goto l3;
1362-
/* concurrent xact committed */
1363-
Assert(tuple->t_data->t_xmax == xwait);
1372+
/*
1373+
* xwait is committed but if xwait had just marked
1374+
* the tuple for update then some other xaction could
1375+
* update this tuple before we got to this point.
1376+
*/
1377+
if (tuple->t_data->t_xmax != xwait)
1378+
goto l3;
13641379
if (!(tuple->t_data->t_infomask & HEAP_XMAX_COMMITTED))
13651380
{
13661381
tuple->t_data->t_infomask |= HEAP_XMAX_COMMITTED;

‎src/backend/access/transam/transam.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/access/transam/transam.c,v 1.26 1999/05/25 16:07:45 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/access/transam/transam.c,v 1.27 1999/06/10 14:17:06 vadim Exp $
1111
*
1212
* NOTES
1313
* This file contains the high level access-method interface to the
@@ -172,8 +172,14 @@ TransactionLogTest(TransactionId transactionId, /* transaction id to test */
172172

173173
if (!fail)
174174
{
175-
TransactionIdStore(transactionId, &cachedTestXid);
176-
cachedTestXidStatus = xidstatus;
175+
/*
176+
* DO NOT cache status for transactions in unknown state !!!
177+
*/
178+
if (xidstatus == XID_COMMIT || xidstatus == XID_ABORT)
179+
{
180+
TransactionIdStore(transactionId, &cachedTestXid);
181+
cachedTestXidStatus = xidstatus;
182+
}
177183
return (bool) (status == xidstatus);
178184
}
179185

@@ -576,12 +582,3 @@ TransactionIdAbort(TransactionId transactionId)
576582

577583
TransactionLogUpdate(transactionId, XID_ABORT);
578584
}
579-
580-
void
581-
TransactionIdFlushCache()
582-
{
583-
584-
TransactionIdStore(AmiTransactionId, &cachedTestXid);
585-
cachedTestXidStatus = XID_COMMIT;
586-
587-
}

‎src/backend/access/transam/xact.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.40 1999/06/06 20:19:34 vadim Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.41 1999/06/10 14:17:06 vadim Exp $
1111
*
1212
* NOTES
1313
* Transaction aborts can now occur two ways:
@@ -514,8 +514,6 @@ CommandCounterIncrement()
514514
AtCommit_Cache();
515515
AtStart_Cache();
516516

517-
TransactionIdFlushCache();
518-
519517
}
520518

521519
void
@@ -822,7 +820,6 @@ StartTransaction()
822820
{
823821
TransactionState s = CurrentTransactionState;
824822

825-
TransactionIdFlushCache();
826823
FreeXactSnapshot();
827824
XactIsoLevel = DefaultXactIsoLevel;
828825

‎src/backend/commands/vacuum.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.107 1999/06/06 20:19:34 vadim Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.108 1999/06/10 14:17:07 vadim Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1351,6 +1351,8 @@ vc_rpfheap(VRelStats *vacrelstats, Relation onerel,
13511351
if (!ItemIdIsUsed(Pitemid))
13521352
elog(ERROR, "Parent itemid marked as unused");
13531353
Ptp.t_data = (HeapTupleHeader) PageGetItem(Ppage, Pitemid);
1354+
Assert(ItemPointerEquals(&(vtld.new_tid),
1355+
&(Ptp.t_data->t_ctid)));
13541356
Assert(Ptp.t_data->t_xmax == tp.t_data->t_xmin);
13551357

13561358
#ifdef NOT_USED /* I'm not sure that this will wotk properly... */

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.53 1999/05/29 03:58:43 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.54 1999/06/10 14:17:09 vadim Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -2007,7 +2007,8 @@ LockBuffer(Buffer buffer, int mode)
20072007
else if (BufferLocks[buffer - 1] & BL_W_LOCK)
20082008
{
20092009
Assert(buf->w_lock);
2010-
Assert(buf->r_locks == 0 && !buf->ri_lock);
2010+
Assert(buf->r_locks == 0);
2011+
Assert(!buf->ri_lock);
20112012
Assert(!(BufferLocks[buffer - 1] & (BL_R_LOCK | BL_RI_LOCK)))
20122013
buf->w_lock = false;
20132014
BufferLocks[buffer - 1] &= ~BL_W_LOCK;

‎src/backend/storage/lmgr/lmgr.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lmgr.c,v 1.26 1999/05/31 01:48:13 vadim Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/storage/lmgr/lmgr.c,v 1.27 1999/06/10 14:17:11 vadim Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -324,8 +324,6 @@ XactLockTableWait(TransactionId xid)
324324
LockAcquire(LockTableId, &tag, ShareLock);
325325
LockRelease(LockTableId, &tag, ShareLock);
326326

327-
TransactionIdFlushCache();
328-
329327
/*
330328
* Transaction was committed/aborted/crashed - we have to update
331329
* pg_log if transaction is still marked as running.

‎src/include/access/transam.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*
77
* Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Id: transam.h,v 1.21 1999/05/25 22:42:35 momjian Exp $
9+
* $Id: transam.h,v 1.22 1999/06/10 14:17:12 vadim Exp $
1010
*
1111
* NOTES
1212
* Transaction System Version 101 now support proper oid
@@ -145,7 +145,6 @@ extern bool TransactionIdDidCommit(TransactionId transactionId);
145145
extern bool TransactionIdDidAbort(TransactionId transactionId);
146146
extern void TransactionIdCommit(TransactionId transactionId);
147147
extern void TransactionIdAbort(TransactionId transactionId);
148-
extern void TransactionIdFlushCache(void);
149148

150149
/* in transam/transsup.c */
151150
extern void AmiTransactionOverride(bool flag);

0 commit comments

Comments
 (0)