Rework activation of commit timestamps during recovery

The activation and deactivation of commit timestamp tracking has not
been handled consistently for a primary or standbys at recovery.  The
facility can be activated at three different moments of recovery:
- The beginning, where a primary would use the GUC value for the
decision-making, and where a standby relies on the contents of the
control file.
- When replaying a XLOG_PARAMETER_CHANGE record at redo.
- The end, where both primary and standby rely on the GUC value.

Using the GUC value for a primary at the beginning of recovery causes
problems with commit timestamp access when doing crash recovery.
Particularly, when replaying transaction commits, it could be possible
that an attempt to read commit timestamps is done for a transaction
which committed at a moment when track_commit_timestamp was disabled.

A test case is added to reproduce the failure.  The test works down to
v11 as it takes advantage of transaction commits within procedures.

Reported-by: Hailong Li
Author: Masahiko Sawasa, Michael Paquier
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/11224478-a782-203b-1f17-e4797b39bdf0@qunar.com
Backpatch-through: 9.5, where commit timestamps have been introduced.

diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 60fb9eeb0612ace98da2ec6183b16b7622892da1..d7637b37e8da18d89f539cd0c728db1c1b427704 100644 (file)
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -573,10 +573,9 @@ CompleteCommitTsInitialization(void)
     * any leftover data.
     *
     * Conversely, we activate the module if the feature is enabled.  This is
-    * not necessary in a master system because we already did it earlier, but
-    * if we're in a standby server that got promoted which had the feature
-    * enabled and was following a master that had the feature disabled, this
-    * is where we turn it on locally.
+    * necessary for primary and standby as the activation depends on the
+    * control file contents at the beginning of recovery or when a
+    * XLOG_PARAMETER_CHANGE is replayed.
     */
    if (!track_commit_timestamp)
        DeactivateCommitTs();
@@ -586,7 +585,7 @@ CompleteCommitTsInitialization(void)
 
 /*
  * Activate or deactivate CommitTs' upon reception of a XLOG_PARAMETER_CHANGE
- * XLog record in a standby.
+ * XLog record during recovery.
  */
 void
 CommitTsParameterChange(bool newvalue, bool oldvalue)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 9b7a47fdfe8919764ed25eb326f3ac1ee0320e6f..b78009e888f00deed65c0398d2a56e957f5dabfa 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6719,11 +6719,12 @@ StartupXLOG(void)
    StartupMultiXact();
 
    /*
-    * Ditto commit timestamps.  In a standby, we do it if setting is enabled
-    * in ControlFile; in a master we base the decision on the GUC itself.
+    * Ditto for commit timestamps.  Activate the facility if the setting is
+    * enabled in the control file, as there should be no tracking of commit
+    * timestamps done when the setting was disabled.  This facility can be
+    * started or stopped when replaying a XLOG_PARAMETER_CHANGE record.
     */
-   if (ArchiveRecoveryRequested ?
-       ControlFile->track_commit_timestamp : track_commit_timestamp)
+   if (ControlFile->track_commit_timestamp)
        StartupCommitTs();
 
    /*