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

Commit663d2e4

Browse files
committed
Don't TransactionIdDidAbort in HeapTupleGetUpdateXid
It is dangerous to do so, because some code expects to be able to see what'sthe true Xmax even if it is aborted (particularly while traversing HOTchains). So don't do it, and instead rely on the callers to verify forabortedness, if necessary.Several race conditions and bugs fixed in the process. One isolation testchanges the expected output due to these.This also reverts commitc235a6a, which is no longer necessary.Backpatch to 9.3, where this function was introduced.Andres Freund
1 parent215ac4a commit663d2e4

File tree

4 files changed

+75
-74
lines changed

4 files changed

+75
-74
lines changed

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3164,7 +3164,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
31643164
if (!HEAP_XMAX_IS_LOCKED_ONLY(oldtup.t_data->t_infomask))
31653165
update_xact=HeapTupleGetUpdateXid(oldtup.t_data);
31663166

3167-
/* there was no UPDATE in the MultiXact; or it aborted. */
3167+
/*
3168+
* There was no UPDATE in the MultiXact; or it aborted. No
3169+
* TransactionIdIsInProgress() call needed here, since we called
3170+
* MultiXactIdWait() above.
3171+
*/
31683172
if (!TransactionIdIsValid(update_xact)||
31693173
TransactionIdDidAbort(update_xact))
31703174
can_continue= true;
@@ -5424,6 +5428,9 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
54245428
* Given a multixact Xmax and corresponding infomask, which does not have the
54255429
* HEAP_XMAX_LOCK_ONLY bit set, obtain and return the Xid of the updating
54265430
* transaction.
5431+
*
5432+
* Caller is expected to check the status of the updating transaction, if
5433+
* necessary.
54275434
*/
54285435
staticTransactionId
54295436
MultiXactIdGetUpdateXid(TransactionIdxmax,uint16t_infomask)
@@ -5448,19 +5455,11 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask)
54485455
for (i=0;i<nmembers;i++)
54495456
{
54505457
/* Ignore lockers */
5451-
if (members[i].status==MultiXactStatusForKeyShare||
5452-
members[i].status==MultiXactStatusForShare||
5453-
members[i].status==MultiXactStatusForNoKeyUpdate||
5454-
members[i].status==MultiXactStatusForUpdate)
5458+
if (!ISUPDATE_from_mxstatus(members[i].status))
54555459
continue;
54565460

5457-
/* ignore aborted transactions */
5458-
if (TransactionIdDidAbort(members[i].xid))
5459-
continue;
5460-
/* there should be at most one non-aborted updater */
5461+
/* there can be at most one updater */
54615462
Assert(update_xact==InvalidTransactionId);
5462-
Assert(members[i].status==MultiXactStatusNoKeyUpdate||
5463-
members[i].status==MultiXactStatusUpdate);
54645463
update_xact=members[i].xid;
54655464
#ifndefUSE_ASSERT_CHECKING
54665465

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

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -466,22 +466,12 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
466466
break;
467467

468468
caseHEAPTUPLE_DELETE_IN_PROGRESS:
469-
{
470-
TransactionIdxmax;
471-
472-
/*
473-
* This tuple may soon become DEAD. Update the hint field
474-
* so that the page is reconsidered for pruning in future.
475-
* If there was a MultiXactId updater, and it aborted after
476-
* HTSV checked, then we will get an invalid Xid here.
477-
* There is no need for future pruning of the page in that
478-
* case, so skip it.
479-
*/
480-
xmax=HeapTupleHeaderGetUpdateXid(htup);
481-
if (TransactionIdIsValid(xmax))
482-
heap_prune_record_prunable(prstate,xmax);
483-
}
484-
469+
/*
470+
* This tuple may soon become DEAD. Update the hint field
471+
* so that the page is reconsidered for pruning in future.
472+
*/
473+
heap_prune_record_prunable(prstate,
474+
HeapTupleHeaderGetUpdateXid(htup));
485475
break;
486476

487477
caseHEAPTUPLE_LIVE:

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

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,9 @@ HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer)
222222
TransactionIdxmax;
223223

224224
xmax=HeapTupleGetUpdateXid(tuple);
225-
if (!TransactionIdIsValid(xmax))
226-
return true;
225+
226+
/* not LOCKED_ONLY, so it has to have an xmax */
227+
Assert(TransactionIdIsValid(xmax));
227228

228229
/* updating subtransaction must have aborted */
229230
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -276,14 +277,17 @@ HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer)
276277
return true;
277278

278279
xmax=HeapTupleGetUpdateXid(tuple);
279-
if (!TransactionIdIsValid(xmax))
280-
return true;
280+
281+
/* not LOCKED_ONLY, so it has to have an xmax */
282+
Assert(TransactionIdIsValid(xmax));
283+
281284
if (TransactionIdIsCurrentTransactionId(xmax))
282285
return false;
283286
if (TransactionIdIsInProgress(xmax))
284287
return true;
285288
if (TransactionIdDidCommit(xmax))
286289
return false;
290+
/* it must have aborted or crashed */
287291
return true;
288292
}
289293

@@ -690,8 +694,9 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
690694
TransactionIdxmax;
691695

692696
xmax=HeapTupleGetUpdateXid(tuple);
693-
if (!TransactionIdIsValid(xmax))
694-
returnHeapTupleMayBeUpdated;
697+
698+
/* not LOCKED_ONLY, so it has to have an xmax */
699+
Assert(TransactionIdIsValid(xmax));
695700

696701
/* updating subtransaction must have aborted */
697702
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -766,14 +771,9 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
766771
}
767772

768773
xmax=HeapTupleGetUpdateXid(tuple);
769-
if (!TransactionIdIsValid(xmax))
770-
{
771-
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
772-
returnHeapTupleBeingUpdated;
773774

774-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
775-
returnHeapTupleMayBeUpdated;
776-
}
775+
/* not LOCKED_ONLY, so it has to have an xmax */
776+
Assert(TransactionIdIsValid(xmax));
777777

778778
if (TransactionIdIsCurrentTransactionId(xmax))
779779
{
@@ -783,13 +783,18 @@ HeapTupleSatisfiesUpdate(HeapTupleHeader tuple, CommandId curcid,
783783
returnHeapTupleInvisible;/* updated before scan started */
784784
}
785785

786-
if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
786+
if (TransactionIdIsInProgress(xmax))
787787
returnHeapTupleBeingUpdated;
788788

789789
if (TransactionIdDidCommit(xmax))
790790
returnHeapTupleUpdated;
791+
792+
/* no member, even just a locker, alive anymore */
793+
if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
794+
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,
795+
InvalidTransactionId);
796+
791797
/* it must have aborted or crashed */
792-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
793798
returnHeapTupleMayBeUpdated;
794799
}
795800

@@ -911,8 +916,9 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Snapshot snapshot,
911916
TransactionIdxmax;
912917

913918
xmax=HeapTupleGetUpdateXid(tuple);
914-
if (!TransactionIdIsValid(xmax))
915-
return true;
919+
920+
/* not LOCKED_ONLY, so it has to have an xmax */
921+
Assert(TransactionIdIsValid(xmax));
916922

917923
/* updating subtransaction must have aborted */
918924
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -969,8 +975,10 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Snapshot snapshot,
969975
return true;
970976

971977
xmax=HeapTupleGetUpdateXid(tuple);
972-
if (!TransactionIdIsValid(xmax))
973-
return true;
978+
979+
/* not LOCKED_ONLY, so it has to have an xmax */
980+
Assert(TransactionIdIsValid(xmax));
981+
974982
if (TransactionIdIsCurrentTransactionId(xmax))
975983
return false;
976984
if (TransactionIdIsInProgress(xmax))
@@ -980,6 +988,7 @@ HeapTupleSatisfiesDirty(HeapTupleHeader tuple, Snapshot snapshot,
980988
}
981989
if (TransactionIdDidCommit(xmax))
982990
return false;
991+
/* it must have aborted or crashed */
983992
return true;
984993
}
985994

@@ -1104,8 +1113,9 @@ HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, Snapshot snapshot,
11041113
TransactionIdxmax;
11051114

11061115
xmax=HeapTupleGetUpdateXid(tuple);
1107-
if (!TransactionIdIsValid(xmax))
1108-
return true;
1116+
1117+
/* not LOCKED_ONLY, so it has to have an xmax */
1118+
Assert(TransactionIdIsValid(xmax));
11091119

11101120
/* updating subtransaction must have aborted */
11111121
if (!TransactionIdIsCurrentTransactionId(xmax))
@@ -1164,8 +1174,10 @@ HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, Snapshot snapshot,
11641174
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
11651175

11661176
xmax=HeapTupleGetUpdateXid(tuple);
1167-
if (!TransactionIdIsValid(xmax))
1168-
return true;
1177+
1178+
/* not LOCKED_ONLY, so it has to have an xmax */
1179+
Assert(TransactionIdIsValid(xmax));
1180+
11691181
if (TransactionIdIsCurrentTransactionId(xmax))
11701182
{
11711183
if (HeapTupleHeaderGetCmax(tuple) >=snapshot->curcid)
@@ -1182,6 +1194,7 @@ HeapTupleSatisfiesMVCC(HeapTupleHeader tuple, Snapshot snapshot,
11821194
return true;/* treat as still in progress */
11831195
return false;
11841196
}
1197+
/* it must have aborted or crashed */
11851198
return true;
11861199
}
11871200

@@ -1376,8 +1389,10 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
13761389
Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask));
13771390

13781391
xmax=HeapTupleGetUpdateXid(tuple);
1379-
if (!TransactionIdIsValid(xmax))
1380-
returnHEAPTUPLE_LIVE;
1392+
1393+
/* not LOCKED_ONLY, so it has to have an xmax */
1394+
Assert(TransactionIdIsValid(xmax));
1395+
13811396
if (TransactionIdIsInProgress(xmax))
13821397
returnHEAPTUPLE_DELETE_IN_PROGRESS;
13831398
elseif (TransactionIdDidCommit(xmax))
@@ -1390,8 +1405,10 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
13901405
Assert(!(tuple->t_infomask&HEAP_XMAX_COMMITTED));
13911406

13921407
xmax=HeapTupleGetUpdateXid(tuple);
1393-
if (!TransactionIdIsValid(xmax))
1394-
returnHEAPTUPLE_LIVE;
1408+
1409+
/* not LOCKED_ONLY, so it has to have an xmax */
1410+
Assert(TransactionIdIsValid(xmax));
1411+
13951412
/* multi is not running -- updating xact cannot be */
13961413
Assert(!TransactionIdIsInProgress(xmax));
13971414
if (TransactionIdDidCommit(xmax))
@@ -1401,22 +1418,13 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin,
14011418
else
14021419
returnHEAPTUPLE_DEAD;
14031420
}
1404-
else
1405-
{
1406-
/*
1407-
* Not in Progress, Not Committed, so either Aborted or crashed.
1408-
*/
1409-
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
1410-
returnHEAPTUPLE_LIVE;
1411-
}
14121421

14131422
/*
1414-
*Deleter committed, but perhaps it was recent enough that some open
1415-
*transactions could still seethetuple.
1423+
*Not in Progress, Not Committed, so either Aborted or crashed.
1424+
*RemovetheXmax.
14161425
*/
1417-
1418-
/* Otherwise, it's dead and removable */
1419-
returnHEAPTUPLE_DEAD;
1426+
SetHintBits(tuple,buffer,HEAP_XMAX_INVALID,InvalidTransactionId);
1427+
returnHEAPTUPLE_LIVE;
14201428
}
14211429

14221430
if (!(tuple->t_infomask&HEAP_XMAX_COMMITTED))
@@ -1655,8 +1663,9 @@ HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple)
16551663

16561664
/* ... but if it's a multi, then perhaps the updating Xid aborted. */
16571665
xmax=HeapTupleGetUpdateXid(tuple);
1658-
if (!TransactionIdIsValid(xmax))/* shouldn't happen .. */
1659-
return true;
1666+
1667+
/* not LOCKED_ONLY, so it has to have an xmax */
1668+
Assert(TransactionIdIsValid(xmax));
16601669

16611670
if (TransactionIdIsCurrentTransactionId(xmax))
16621671
return false;

‎src/test/isolation/expected/delete-abort-savept.out

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ key value
2323
step s1svp: SAVEPOINT f;
2424
step s1d: DELETE FROM foo;
2525
step s1r: ROLLBACK TO f;
26-
step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
27-
step s1c: COMMIT;
28-
step s2l: <... completed>
26+
step s2l: SELECT * FROM foo FOR UPDATE;
2927
key value
3028

3129
1 1
30+
step s1c: COMMIT;
3231
step s2c: COMMIT;
3332

3433
starting permutation: s1l s1svp s1d s1r s2l s2c s1c
@@ -39,8 +38,12 @@ key value
3938
step s1svp: SAVEPOINT f;
4039
step s1d: DELETE FROM foo;
4140
step s1r: ROLLBACK TO f;
42-
step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
43-
invalid permutation detected
41+
step s2l: SELECT * FROM foo FOR UPDATE;
42+
key value
43+
44+
1 1
45+
step s2c: COMMIT;
46+
step s1c: COMMIT;
4447

4548
starting permutation: s1l s1svp s1d s2l s1r s1c s2c
4649
step s1l: SELECT * FROM foo FOR KEY SHARE;

0 commit comments

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