diff --git a/src/backend/access/transam/transam.c b/src/backend/access/transam/transam.c index dbc5f884e88..5865810135e 100644 --- a/src/backend/access/transam/transam.c +++ b/src/backend/access/transam/transam.c @@ -219,33 +219,6 @@ TransactionIdDidAbort(TransactionId transactionId) return false; } -/* - * TransactionIdIsKnownCompleted - * True iff transaction associated with the identifier is currently - * known to have either committed or aborted. - * - * This does NOT look into pg_xact but merely probes our local cache - * (and so it's not named TransactionIdDidComplete, which would be the - * appropriate name for a function that worked that way). The intended - * use is just to short-circuit TransactionIdIsInProgress calls when doing - * repeated heapam_visibility.c checks for the same XID. If this isn't - * extremely fast then it will be counterproductive. - * - * Note: - * Assumes transaction identifier is valid. - */ -bool -TransactionIdIsKnownCompleted(TransactionId transactionId) -{ - if (TransactionIdEquals(transactionId, cachedFetchXid)) - { - /* If it's in the cache at all, it must be completed. */ - return true; - } - - return false; -} - /* * TransactionIdCommitTree * Marks the given transaction and children as committed diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 4da53a5b3fd..dadaa958a84 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -261,6 +261,11 @@ static ProcArrayStruct *procArray; static PGPROC *allProcs; +/* + * Cache to reduce overhead of repeated calls to TransactionIdIsInProgress() + */ +static TransactionId cachedXidIsNotInProgress = InvalidTransactionId; + /* * Bookkeeping for tracking emulated transactions in recovery */ @@ -1396,7 +1401,7 @@ TransactionIdIsInProgress(TransactionId xid) * already known to be completed, we can fall out without any access to * shared memory. */ - if (TransactionIdIsKnownCompleted(xid)) + if (TransactionIdEquals(cachedXidIsNotInProgress, xid)) { xc_by_known_xact_inc(); return false; @@ -1554,6 +1559,7 @@ TransactionIdIsInProgress(TransactionId xid) if (nxids == 0) { xc_no_overflow_inc(); + cachedXidIsNotInProgress = xid; return false; } @@ -1568,7 +1574,10 @@ TransactionIdIsInProgress(TransactionId xid) xc_slow_answer_inc(); if (TransactionIdDidAbort(xid)) + { + cachedXidIsNotInProgress = xid; return false; + } /* * It isn't aborted, so check whether the transaction tree it belongs to @@ -1586,6 +1595,7 @@ TransactionIdIsInProgress(TransactionId xid) } } + cachedXidIsNotInProgress = xid; return false; } diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index 6c57ec3d358..d8e40b3b969 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -36,6 +36,7 @@ #include "miscadmin.h" #include "postmaster/postmaster.h" #include "storage/lwlock.h" +#include "storage/procarray.h" #include "utils/builtins.h" #include "utils/memutils.h" #include "utils/snapmgr.h" @@ -676,29 +677,22 @@ pg_xact_status(PG_FUNCTION_ARGS) { Assert(TransactionIdIsValid(xid)); - if (TransactionIdIsCurrentTransactionId(xid)) + /* + * Like when doing visiblity checks on a row, check whether the + * transaction is still in progress before looking into the CLOG. + * Otherwise we would incorrectly return "committed" for a transaction + * that is committing and has already updated the CLOG, but hasn't + * removed its XID from the proc array yet. (See comment on that race + * condition at the top of heapam_visibility.c) + */ + if (TransactionIdIsInProgress(xid)) status = "in progress"; else if (TransactionIdDidCommit(xid)) status = "committed"; - else if (TransactionIdDidAbort(xid)) - status = "aborted"; else { - /* - * The xact is not marked as either committed or aborted in clog. - * - * It could be a transaction that ended without updating clog or - * writing an abort record due to a crash. We can safely assume - * it's aborted if it isn't committed and is older than our - * snapshot xmin. - * - * Otherwise it must be in-progress (or have been at the time we - * checked commit/abort status). - */ - if (TransactionIdPrecedes(xid, GetActiveSnapshot()->xmin)) - status = "aborted"; - else - status = "in progress"; + /* it must have aborted or crashed */ + status = "aborted"; } } else diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 338dfca5a0b..775471d2a7d 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -273,7 +273,6 @@ extern PGDLLIMPORT VariableCache ShmemVariableCache; */ extern bool TransactionIdDidCommit(TransactionId transactionId); extern bool TransactionIdDidAbort(TransactionId transactionId); -extern bool TransactionIdIsKnownCompleted(TransactionId transactionId); extern void TransactionIdCommitTree(TransactionId xid, int nxids, TransactionId *xids); extern void TransactionIdAsyncCommitTree(TransactionId xid, int nxids, TransactionId *xids, XLogRecPtr lsn); extern void TransactionIdAbortTree(TransactionId xid, int nxids, TransactionId *xids);