On Thu, Oct 11, 2018 at 08:04:11PM +0300, Konstantin Knizhnik wrote: > Proposed patch is attached.
The problem I have with this patch doing the duplication removal and qsort work in LogCurrentRunningXacts is that it would still lock ProcArrayLock until the WAL record has been written with wal_level = logical. With many sessions and 2PC transactions running around, this would be a performance impact for any deployments every time a checkpoint happens or every time the bgwriter decide to log a standby snapshot. Hence I would go instead with the attached, which does the legwork at recovery, which is a one-time code path as you mentioned. Okay, this makes the recovery a bit longer but that's way better than impacting all deployments of Postgres, even those not using 2PC when normally running. And as the sorting phase already happens we just need to do something like the attached. One thing that we could also do for HEAD is to add in RunningTransactionsData if the transaction comes from a 2PC entry, and allow recovery to do more sanity checks. This would require a WAL format change. The proposed patch needs to be back-patched down to 9.3. -- Michael
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index bf2f4dbed2..6317b856a1 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -799,10 +799,20 @@ 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 from prepared transaction for KnownAssignedXids",
+ xids[i]);
+ continue;
+ }
KnownAssignedXidsAdd(xids[i], xids[i], true);
+ }
KnownAssignedXidsDisplay(trace_recovery(DEBUG3));
}
@@ -1898,7 +1908,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 +1923,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.
signature.asc
Description: PGP signature
