Treat 2PC commit/abort the same as regular xacts in recovery.

There were several oversights in recovery code where COMMIT/ABORT PREPARED
records were ignored:

* pg_last_xact_replay_timestamp() (wasn't updated for 2PC commits)
* recovery_min_apply_delay (2PC commits were applied immediately)
* recovery_target_xid (recovery would not stop if the XID used 2PC)

The first of those was reported by Sergiy Zuban in bug #11032, analyzed by
Tom Lane and Andres Freund. The bug was always there, but was masked before
commit d19bd29f07aef9e508ff047d128a4046cc8bc1e2, because COMMIT PREPARED
always created an extra regular transaction that was WAL-logged.

Backpatch to all supported versions (older versions didn't have all the
features and therefore didn't have all of the above bugs).

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 164b22f66a65181963983de5fe1579467c392ad5..f6b32b8ee2c456be06cdcaed61054111717f0e37 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4494,6 +4494,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
    bool        stopsHere;
    uint8       record_info;
    TimestampTz recordXtime;
+   TransactionId recordXid;
    char        recordRPName[MAXFNAMELEN];
 
    /* We only consider stopping at COMMIT, ABORT or RESTORE POINT records */
@@ -4506,6 +4507,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
        recordXactCommitData = (xl_xact_commit_compact *) XLogRecGetData(record);
        recordXtime = recordXactCommitData->xact_time;
+       recordXid = record->xl_xid;
    }
    else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_COMMIT)
    {
@@ -4513,6 +4515,15 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
        recordXactCommitData = (xl_xact_commit *) XLogRecGetData(record);
        recordXtime = recordXactCommitData->xact_time;
+       recordXid = record->xl_xid;
+   }
+   else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_COMMIT_PREPARED)
+   {
+       xl_xact_commit_prepared *recordXactCommitData;
+
+       recordXactCommitData = (xl_xact_commit_prepared *) XLogRecGetData(record);
+       recordXtime = recordXactCommitData->crec.xact_time;
+       recordXid = recordXactCommitData->xid;
    }
    else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_ABORT)
    {
@@ -4520,6 +4531,15 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
        recordXactAbortData = (xl_xact_abort *) XLogRecGetData(record);
        recordXtime = recordXactAbortData->xact_time;
+       recordXid = record->xl_xid;
+   }
+   else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_ABORT_PREPARED)
+   {
+       xl_xact_abort_prepared *recordXactAbortData;
+
+       recordXactAbortData = (xl_xact_abort_prepared *) XLogRecGetData(record);
+       recordXtime = recordXactAbortData->arec.xact_time;
+       recordXid = recordXactAbortData->xid;
    }
    else if (record->xl_rmid == RM_XLOG_ID && record_info == XLOG_RESTORE_POINT)
    {
@@ -4527,6 +4547,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
        recordRestorePointData = (xl_restore_point *) XLogRecGetData(record);
        recordXtime = recordRestorePointData->rp_time;
+       recordXid = InvalidTransactionId;
        strlcpy(recordRPName, recordRestorePointData->rp_name, MAXFNAMELEN);
    }
    else
@@ -4555,7 +4576,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
         * they complete. A higher numbered xid will complete before you about
         * 50% of the time...
         */
-       stopsHere = (record->xl_xid == recoveryTargetXid);
+       stopsHere = (recordXid == recoveryTargetXid);
        if (stopsHere)
            *includeThis = recoveryTargetInclusive;
    }
@@ -4590,11 +4611,13 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
    if (stopsHere)
    {
-       recoveryStopXid = record->xl_xid;
+       recoveryStopXid = recordXid;
        recoveryStopTime = recordXtime;
        recoveryStopAfter = *includeThis;
 
-       if (record_info == XLOG_XACT_COMMIT_COMPACT || record_info == XLOG_XACT_COMMIT)
+       if (record_info == XLOG_XACT_COMMIT_COMPACT ||
+           record_info == XLOG_XACT_COMMIT ||
+           record_info == XLOG_XACT_COMMIT_PREPARED)
        {
            if (recoveryStopAfter)
                ereport(LOG,
@@ -4607,7 +4630,8 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
                                recoveryStopXid,
                                timestamptz_to_str(recoveryStopTime))));
        }
-       else if (record_info == XLOG_XACT_ABORT)
+       else if (record_info == XLOG_XACT_ABORT ||
+                record_info == XLOG_XACT_ABORT_PREPARED)
        {
            if (recoveryStopAfter)
                ereport(LOG,

diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 835f6acbee0e10ee51e5a2295429efd5141e3b77..6cae504adc9a3cda23c2de6829f469024086b0b3 100644 (file)
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -180,8 +180,7 @@ typedef struct xl_xact_abort
 /*
  * COMMIT_PREPARED and ABORT_PREPARED are identical to COMMIT/ABORT records
  * except that we have to store the XID of the prepared transaction explicitly
- * --- the XID in the record header will be for the transaction doing the
- * COMMIT PREPARED or ABORT PREPARED command.
+ * --- the XID in the record header will be invalid.
  */
 
 typedef struct xl_xact_commit_prepared