From 1df21ddb19c6e764fc9378c900515a5d642ad820 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Sun, 14 Oct 2018 22:23:21 +0900 Subject: [PATCH] Avoid duplicate XIDs at recovery when building initial snapshot On a primary, sets of XLOG_RUNNING_XACTS records are generated on a periodic basis to allow recovery to build the initial state of transactions for a hot standby. The set of transaction IDs is created by scanning all the entries in ProcArray. However it happens that its logic never counted on the fact that two-phase transactions finishing to prepare can put ProcArray in a state where there are two entries with the same transaction ID, one for the initial transaction which gets cleared when prepare finishes, and a second, dummy, entry to track that the transaction is still running after prepare finishes. This way ensures a continuous presence of the transaction so as callers of for example TransactionIdIsInProgress() are always able to see it as alive. So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase transaction finishes to prepare, the record can finish with duplicated XIDs, which is a state expected by design. If this record gets applied on a standby to initial its recovery state, then it would simply fail, so the odds of facing this failure are very low in practice. It would be tempting to change the generation of XLOG_RUNNING_XACTS so as duplicates are removed on the source, but this requires to hold on ProcArrayLock for longer and this would impact all workloads, particularly those using heavily two-phase transactions. XLOG_RUNNING_XACTS is also actually used only to initialize the standby state at recovery, so instead the solution is taken to discard duplicates when applying the initial snapshot. Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru Backpatch-through: 9.3 --- src/backend/storage/ipc/procarray.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index bf2f4dbed2..908f62d37e 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -799,10 +799,21 @@ ProcArrayApplyRecoveryInfo(RunningTransactions running) qsort(xids, nxids, sizeof(TransactionId), xidComparator); /* - * Add the sorted snapshot into KnownAssignedXids + * Add the sorted snapshot into KnownAssignedXids. The running-xacts + * snapshot may include duplicated xids because of prepared + * transactions, so ignore them. */ for (i = 0; i < nxids; i++) + { + if (i > 0 && TransactionIdEquals(xids[i - 1], xids[i])) + { + elog(DEBUG1, + "found duplicated transaction %u for KnownAssignedXids insertion", + xids[i]); + continue; + } KnownAssignedXidsAdd(xids[i], xids[i], true); + } KnownAssignedXidsDisplay(trace_recovery(DEBUG3)); } @@ -1898,7 +1909,8 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) * GetRunningTransactionData -- returns information about running transactions. * * Similar to GetSnapshotData but returns more information. We include - * all PGXACTs with an assigned TransactionId, even VACUUM processes. + * all PGXACTs with an assigned TransactionId, even VACUUM processes and + * prepared transactions. * * We acquire XidGenLock and ProcArrayLock, but the caller is responsible for * releasing them. Acquiring XidGenLock ensures that no new XIDs enter the proc @@ -1912,6 +1924,11 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc) * This is never executed during recovery so there is no need to look at * KnownAssignedXids. * + * Dummy PGXACTs from prepared transaction are included, meaning that this + * may return entries with duplicated TransactionId values coming from + * transaction finishing to prepare. Nothing is done about duplicated + * entries here to not hold on ProcArrayLock more than necessary. + * * We don't worry about updating other counters, we want to keep this as * simple as possible and leave GetSnapshotData() as the primary code for * that bookkeeping.