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

Commitadf6d5d

Browse files
committed
Fix visibility check when XID is committed in CLOG but not in procarray.
TransactionIdIsInProgress had a fast path to return 'false' if thesingle-item CLOG cache said that the transaction was known to becommitted. However, that was wrong, because a transaction is firstmarked as committed in the CLOG but doesn't become visible to othersuntil it has removed its XID from the proc array. That could lead to anerror: ERROR: t_xmin is uncommitted in tuple to be updatedor for an UPDATE to go ahead without blocking, before the previousUPDATE on the same row was made visible.The window is usually very short, but synchronous replication makes itmuch wider, because the wait for synchronous replica happens in thatwindow.Another thing that makes it hard to hit is that it's hard to get sucha commit-in-progress transaction into the single item CLOG cache.Normally, if you call TransactionIdIsInProgress on such a transaction,it determines that the XID is in progress without checking the CLOGand without populating the cache. One way to prime the cache is toexplicitly call pg_xact_status() on the XID. Another way is to use alot of subtransactions, so that the subxid cache in the proc array isoverflown, making TransactionIdIsInProgress rely on pg_subtrans andCLOG checks.This has been broken ever since it was introduced in 2008, but the racecondition is very hard to hit, especially without synchronousreplication. There were a couple of reports of the error starting fromsummer 2021, but no one was able to find the root cause then.TransactionIdIsKnownCompleted() is now unused. In 'master', remove it,but I left it in place in backbranches in case it's used by extensions.Also change pg_xact_status() to check TransactionIdIsInProgress().Previously, it only checked the CLOG, and returned "committed" beforethe transaction was actually made visible to other queries. Note thatthis also means that you cannot use pg_xact_status() to reproduce thebug anymore, even if the code wasn't fixed.Report and analysis by Konstantin Knizhnik. Patch by Simon Riggs, withthe pg_xact_status() change added by me.Author: Simon RiggsReviewed-by: Andres FreundDiscussion:https://www.postgresql.org/message-id/flat/4da7913d-398c-e2ad-d777-f752cf7f0bbb%40garret.ru
1 parent7201cd1 commitadf6d5d

File tree

4 files changed

+23
-47
lines changed

4 files changed

+23
-47
lines changed

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -219,33 +219,6 @@ TransactionIdDidAbort(TransactionId transactionId)
219219
return false;
220220
}
221221

222-
/*
223-
* TransactionIdIsKnownCompleted
224-
*True iff transaction associated with the identifier is currently
225-
*known to have either committed or aborted.
226-
*
227-
* This does NOT look into pg_xact but merely probes our local cache
228-
* (and so it's not named TransactionIdDidComplete, which would be the
229-
* appropriate name for a function that worked that way). The intended
230-
* use is just to short-circuit TransactionIdIsInProgress calls when doing
231-
* repeated heapam_visibility.c checks for the same XID. If this isn't
232-
* extremely fast then it will be counterproductive.
233-
*
234-
* Note:
235-
*Assumes transaction identifier is valid.
236-
*/
237-
bool
238-
TransactionIdIsKnownCompleted(TransactionIdtransactionId)
239-
{
240-
if (TransactionIdEquals(transactionId,cachedFetchXid))
241-
{
242-
/* If it's in the cache at all, it must be completed. */
243-
return true;
244-
}
245-
246-
return false;
247-
}
248-
249222
/*
250223
* TransactionIdCommitTree
251224
*Marks the given transaction and children as committed

‎src/backend/storage/ipc/procarray.c‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,11 @@ static ProcArrayStruct *procArray;
261261

262262
staticPGPROC*allProcs;
263263

264+
/*
265+
* Cache to reduce overhead of repeated calls to TransactionIdIsInProgress()
266+
*/
267+
staticTransactionIdcachedXidIsNotInProgress=InvalidTransactionId;
268+
264269
/*
265270
* Bookkeeping for tracking emulated transactions in recovery
266271
*/
@@ -1396,7 +1401,7 @@ TransactionIdIsInProgress(TransactionId xid)
13961401
* already known to be completed, we can fall out without any access to
13971402
* shared memory.
13981403
*/
1399-
if (TransactionIdIsKnownCompleted(xid))
1404+
if (TransactionIdEquals(cachedXidIsNotInProgress,xid))
14001405
{
14011406
xc_by_known_xact_inc();
14021407
return false;
@@ -1554,6 +1559,7 @@ TransactionIdIsInProgress(TransactionId xid)
15541559
if (nxids==0)
15551560
{
15561561
xc_no_overflow_inc();
1562+
cachedXidIsNotInProgress=xid;
15571563
return false;
15581564
}
15591565

@@ -1568,7 +1574,10 @@ TransactionIdIsInProgress(TransactionId xid)
15681574
xc_slow_answer_inc();
15691575

15701576
if (TransactionIdDidAbort(xid))
1577+
{
1578+
cachedXidIsNotInProgress=xid;
15711579
return false;
1580+
}
15721581

15731582
/*
15741583
* It isn't aborted, so check whether the transaction tree it belongs to
@@ -1586,6 +1595,7 @@ TransactionIdIsInProgress(TransactionId xid)
15861595
}
15871596
}
15881597

1598+
cachedXidIsNotInProgress=xid;
15891599
return false;
15901600
}
15911601

‎src/backend/utils/adt/xid8funcs.c‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include"miscadmin.h"
3737
#include"postmaster/postmaster.h"
3838
#include"storage/lwlock.h"
39+
#include"storage/procarray.h"
3940
#include"utils/builtins.h"
4041
#include"utils/memutils.h"
4142
#include"utils/snapmgr.h"
@@ -676,29 +677,22 @@ pg_xact_status(PG_FUNCTION_ARGS)
676677
{
677678
Assert(TransactionIdIsValid(xid));
678679

679-
if (TransactionIdIsCurrentTransactionId(xid))
680+
/*
681+
* Like when doing visiblity checks on a row, check whether the
682+
* transaction is still in progress before looking into the CLOG.
683+
* Otherwise we would incorrectly return "committed" for a transaction
684+
* that is committing and has already updated the CLOG, but hasn't
685+
* removed its XID from the proc array yet. (See comment on that race
686+
* condition at the top of heapam_visibility.c)
687+
*/
688+
if (TransactionIdIsInProgress(xid))
680689
status="in progress";
681690
elseif (TransactionIdDidCommit(xid))
682691
status="committed";
683-
elseif (TransactionIdDidAbort(xid))
684-
status="aborted";
685692
else
686693
{
687-
/*
688-
* The xact is not marked as either committed or aborted in clog.
689-
*
690-
* It could be a transaction that ended without updating clog or
691-
* writing an abort record due to a crash. We can safely assume
692-
* it's aborted if it isn't committed and is older than our
693-
* snapshot xmin.
694-
*
695-
* Otherwise it must be in-progress (or have been at the time we
696-
* checked commit/abort status).
697-
*/
698-
if (TransactionIdPrecedes(xid,GetActiveSnapshot()->xmin))
699-
status="aborted";
700-
else
701-
status="in progress";
694+
/* it must have aborted or crashed */
695+
status="aborted";
702696
}
703697
}
704698
else

‎src/include/access/transam.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ extern PGDLLIMPORT VariableCache ShmemVariableCache;
273273
*/
274274
externboolTransactionIdDidCommit(TransactionIdtransactionId);
275275
externboolTransactionIdDidAbort(TransactionIdtransactionId);
276-
externboolTransactionIdIsKnownCompleted(TransactionIdtransactionId);
277276
externvoidTransactionIdCommitTree(TransactionIdxid,intnxids,TransactionId*xids);
278277
externvoidTransactionIdAsyncCommitTree(TransactionIdxid,intnxids,TransactionId*xids,XLogRecPtrlsn);
279278
externvoidTransactionIdAbortTree(TransactionIdxid,intnxids,TransactionId*xids);

0 commit comments

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