Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4f335a3

Browse files
committed
Repair two related errors in heap_lock_tuple: it was failing to recognize
cases where we already hold the desired lock "indirectly", either viamembership in a MultiXact or because the lock was originally taken by adifferent subtransaction of the current transaction. These cases must beaccounted for to avoid needless deadlocks and/or inappropriate replacement ofan exclusive lock with a shared lock. Per report from Clarence Gardner andsubsequent investigation.
1 parentb6b5aa1 commit4f335a3

File tree

4 files changed

+115
-46
lines changed

4 files changed

+115
-46
lines changed

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

Lines changed: 63 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.221 2006/11/05 22:42:07 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.222 2006/11/17 18:00:14 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -2359,6 +2359,8 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer,
23592359
ItemIdlp;
23602360
PageHeaderdp;
23612361
TransactionIdxid;
2362+
TransactionIdxmax;
2363+
uint16old_infomask;
23622364
uint16new_infomask;
23632365
LOCKMODEtuple_lock_type;
23642366
boolhave_tuple_lock= false;
@@ -2395,6 +2397,25 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer,
23952397

23962398
LockBuffer(*buffer,BUFFER_LOCK_UNLOCK);
23972399

2400+
/*
2401+
* If we wish to acquire share lock, and the tuple is already
2402+
* share-locked by a multixact that includes any subtransaction of the
2403+
* current top transaction, then we effectively hold the desired lock
2404+
* already. We *must* succeed without trying to take the tuple lock,
2405+
* else we will deadlock against anyone waiting to acquire exclusive
2406+
* lock. We don't need to make any state changes in this case.
2407+
*/
2408+
if (mode==LockTupleShared&&
2409+
(infomask&HEAP_XMAX_IS_MULTI)&&
2410+
MultiXactIdIsCurrent((MultiXactId)xwait))
2411+
{
2412+
Assert(infomask&HEAP_XMAX_SHARED_LOCK);
2413+
/* Probably can't hold tuple lock here, but may as well check */
2414+
if (have_tuple_lock)
2415+
UnlockTuple(relation,tid,tuple_lock_type);
2416+
returnHeapTupleMayBeUpdated;
2417+
}
2418+
23982419
/*
23992420
* Acquire tuple lock to establish our priority for the tuple.
24002421
* LockTuple will release us when we are next-in-line for the tuple.
@@ -2532,26 +2553,50 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer,
25322553
returnresult;
25332554
}
25342555

2556+
/*
2557+
* We might already hold the desired lock (or stronger), possibly under
2558+
* a different subtransaction of the current top transaction. If so,
2559+
* there is no need to change state or issue a WAL record. We already
2560+
* handled the case where this is true for xmax being a MultiXactId,
2561+
* so now check for cases where it is a plain TransactionId.
2562+
*
2563+
* Note in particular that this covers the case where we already hold
2564+
* exclusive lock on the tuple and the caller only wants shared lock.
2565+
* It would certainly not do to give up the exclusive lock.
2566+
*/
2567+
xmax=HeapTupleHeaderGetXmax(tuple->t_data);
2568+
old_infomask=tuple->t_data->t_infomask;
2569+
2570+
if (!(old_infomask& (HEAP_XMAX_INVALID |
2571+
HEAP_XMAX_COMMITTED |
2572+
HEAP_XMAX_IS_MULTI))&&
2573+
(mode==LockTupleShared ?
2574+
(old_infomask&HEAP_IS_LOCKED) :
2575+
(old_infomask&HEAP_XMAX_EXCL_LOCK))&&
2576+
TransactionIdIsCurrentTransactionId(xmax))
2577+
{
2578+
LockBuffer(*buffer,BUFFER_LOCK_UNLOCK);
2579+
/* Probably can't hold tuple lock here, but may as well check */
2580+
if (have_tuple_lock)
2581+
UnlockTuple(relation,tid,tuple_lock_type);
2582+
returnHeapTupleMayBeUpdated;
2583+
}
2584+
25352585
/*
25362586
* Compute the new xmax and infomask to store into the tuple. Note we do
25372587
* not modify the tuple just yet, because that would leave it in the wrong
25382588
* state if multixact.c elogs.
25392589
*/
25402590
xid=GetCurrentTransactionId();
25412591

2542-
new_infomask=tuple->t_data->t_infomask;
2543-
2544-
new_infomask &= ~(HEAP_XMAX_COMMITTED |
2545-
HEAP_XMAX_INVALID |
2546-
HEAP_XMAX_IS_MULTI |
2547-
HEAP_IS_LOCKED |
2548-
HEAP_MOVED);
2592+
new_infomask=old_infomask& ~(HEAP_XMAX_COMMITTED |
2593+
HEAP_XMAX_INVALID |
2594+
HEAP_XMAX_IS_MULTI |
2595+
HEAP_IS_LOCKED |
2596+
HEAP_MOVED);
25492597

25502598
if (mode==LockTupleShared)
25512599
{
2552-
TransactionIdxmax=HeapTupleHeaderGetXmax(tuple->t_data);
2553-
uint16old_infomask=tuple->t_data->t_infomask;
2554-
25552600
/*
25562601
* If this is the first acquisition of a shared lock in the current
25572602
* transaction, set my per-backend OldestMemberMXactId setting. We can
@@ -2592,32 +2637,13 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer,
25922637
}
25932638
elseif (TransactionIdIsInProgress(xmax))
25942639
{
2595-
if (TransactionIdEquals(xmax,xid))
2596-
{
2597-
/*
2598-
* If the old locker is ourselves, we'll just mark the
2599-
* tuple again with our own TransactionId.However we
2600-
* have to consider the possibility that we had exclusive
2601-
* rather than shared lock before --- if so, be careful to
2602-
* preserve the exclusivity of the lock.
2603-
*/
2604-
if (!(old_infomask&HEAP_XMAX_SHARED_LOCK))
2605-
{
2606-
new_infomask &= ~HEAP_XMAX_SHARED_LOCK;
2607-
new_infomask |=HEAP_XMAX_EXCL_LOCK;
2608-
mode=LockTupleExclusive;
2609-
}
2610-
}
2611-
else
2612-
{
2613-
/*
2614-
* If the Xmax is a valid TransactionId, then we need to
2615-
* create a new MultiXactId that includes both the old
2616-
* locker and our own TransactionId.
2617-
*/
2618-
xid=MultiXactIdCreate(xmax,xid);
2619-
new_infomask |=HEAP_XMAX_IS_MULTI;
2620-
}
2640+
/*
2641+
* If the XMAX is a valid TransactionId, then we need to
2642+
* create a new MultiXactId that includes both the old
2643+
* locker and our own TransactionId.
2644+
*/
2645+
xid=MultiXactIdCreate(xmax,xid);
2646+
new_infomask |=HEAP_XMAX_IS_MULTI;
26212647
}
26222648
else
26232649
{

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
4343
* Portions Copyright (c) 1994, Regents of the University of California
4444
*
45-
* $PostgreSQL: pgsql/src/backend/access/transam/multixact.c,v 1.21 2006/10/0400:29:49 momjian Exp $
45+
* $PostgreSQL: pgsql/src/backend/access/transam/multixact.c,v 1.22 2006/11/17 18:00:15 tgl Exp $
4646
*
4747
*-------------------------------------------------------------------------
4848
*/
@@ -366,7 +366,6 @@ bool
366366
MultiXactIdIsRunning(MultiXactIdmulti)
367367
{
368368
TransactionId*members;
369-
TransactionIdmyXid;
370369
intnmembers;
371370
inti;
372371

@@ -380,12 +379,14 @@ MultiXactIdIsRunning(MultiXactId multi)
380379
return false;
381380
}
382381

383-
/* checking for myself is cheap */
384-
myXid=GetTopTransactionId();
385-
382+
/*
383+
* Checking for myself is cheap compared to looking in shared memory,
384+
* so first do the equivalent of MultiXactIdIsCurrent(). This is not
385+
* needed for correctness, it's just a fast path.
386+
*/
386387
for (i=0;i<nmembers;i++)
387388
{
388-
if (TransactionIdEquals(members[i],myXid))
389+
if (TransactionIdIsCurrentTransactionId(members[i]))
389390
{
390391
debug_elog3(DEBUG2,"IsRunning: I (%d) am running!",i);
391392
pfree(members);
@@ -416,6 +417,44 @@ MultiXactIdIsRunning(MultiXactId multi)
416417
return false;
417418
}
418419

420+
/*
421+
* MultiXactIdIsCurrent
422+
*Returns true if the current transaction is a member of the MultiXactId.
423+
*
424+
* We return true if any live subtransaction of the current top-level
425+
* transaction is a member. This is appropriate for the same reason that a
426+
* lock held by any such subtransaction is globally equivalent to a lock
427+
* held by the current subtransaction: no such lock could be released without
428+
* aborting this subtransaction, and hence releasing its locks. So it's not
429+
* necessary to add the current subxact to the MultiXact separately.
430+
*/
431+
bool
432+
MultiXactIdIsCurrent(MultiXactIdmulti)
433+
{
434+
boolresult= false;
435+
TransactionId*members;
436+
intnmembers;
437+
inti;
438+
439+
nmembers=GetMultiXactIdMembers(multi,&members);
440+
441+
if (nmembers<0)
442+
return false;
443+
444+
for (i=0;i<nmembers;i++)
445+
{
446+
if (TransactionIdIsCurrentTransactionId(members[i]))
447+
{
448+
result= true;
449+
break;
450+
}
451+
}
452+
453+
pfree(members);
454+
455+
returnresult;
456+
}
457+
419458
/*
420459
* MultiXactIdSetOldestMember
421460
*Save the oldest MultiXactId this transaction could be a member of.

‎src/backend/utils/time/tqual.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
* Portions Copyright (c) 1994, Regents of the University of California
3333
*
3434
* IDENTIFICATION
35-
* $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.99 2006/11/05 22:42:09 tgl Exp $
35+
* $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.100 2006/11/17 18:00:15 tgl Exp $
3636
*
3737
*-------------------------------------------------------------------------
3838
*/
@@ -511,7 +511,10 @@ HeapTupleSatisfiesToast(HeapTupleHeader tuple, Buffer buffer)
511511
*HeapTupleUpdated: The tuple was updated by a committed transaction.
512512
*
513513
*HeapTupleBeingUpdated: The tuple is being updated by an in-progress
514-
*transaction other than the current transaction.
514+
*transaction other than the current transaction. (Note: this includes
515+
*the case where the tuple is share-locked by a MultiXact, even if the
516+
*MultiXact includes the current transaction. Callers that want to
517+
*distinguish that case must test for it themselves.)
515518
*/
516519
HTSU_Result
517520
HeapTupleSatisfiesUpdate(HeapTupleHeadertuple,CommandIdcurcid,

‎src/include/access/multixact.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/access/multixact.h,v 1.10 2006/03/24 04:32:13 tgl Exp $
9+
* $PostgreSQL: pgsql/src/include/access/multixact.h,v 1.11 2006/11/17 18:00:15 tgl Exp $
1010
*/
1111
#ifndefMULTIXACT_H
1212
#defineMULTIXACT_H
@@ -45,6 +45,7 @@ typedef struct xl_multixact_create
4545
externMultiXactIdMultiXactIdCreate(TransactionIdxid1,TransactionIdxid2);
4646
externMultiXactIdMultiXactIdExpand(MultiXactIdmulti,TransactionIdxid);
4747
externboolMultiXactIdIsRunning(MultiXactIdmulti);
48+
externboolMultiXactIdIsCurrent(MultiXactIdmulti);
4849
externvoidMultiXactIdWait(MultiXactIdmulti);
4950
externboolConditionalMultiXactIdWait(MultiXactIdmulti);
5051
externvoidMultiXactIdSetOldestMember(void);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp