On 2015-09-28 18:59, Robert Haas wrote:
The patch looks good to me except the following minor points.
* or not. Normal path through RecordTransactionCommit() will be related
* to a transaction commit XLog record, and so should pass "false" here.
The above source comment of TransactionTreeSetCommitTsData() seems to
need to be updated.
+ * Note: if you change this functions you should also look at
+ * RecordTransactionCommitPrepared in twophase.c.
Typo: "this functions" should be "this function"
+ if (replorigin_sesssion_origin == InvalidRepOriginId ||
This is not the problem of the patch, but I started wondering what
"sesssion" in the variable name "replorigin_sesssion_origin" means.
Is it just a typo and it should be "session"? Or it's the abbreviation
of something?
This should go in before beta. Is someone updating the patch?
Sorry, missed your reply.
The "sesssion" is typo and it actually affects several variables around
the replication origin, so I attached separate patch (which should be
applied first) which fixes the typo everywhere.
I reworded the comment, hopefully it's better this way.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From e8c68da28dd3f6b785aa23d4cf3c2973d6bca6c5 Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmo...@pjmodos.net>
Date: Mon, 28 Sep 2015 19:43:19 +0200
Subject: [PATCH 2/2] commit_ts-2pcfix
---
src/backend/access/transam/commit_ts.c | 9 +++++----
src/backend/access/transam/twophase.c | 26 +++++++++++++++++++++++++-
src/backend/access/transam/xact.c | 3 +++
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 33136e3..cddde30 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -123,10 +123,11 @@ static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
* decision of storing timestamp info for each subxid.
*
* The do_xlog parameter tells us whether to include an XLog record of this
- * or not. Normal path through RecordTransactionCommit() will be related
- * to a transaction commit XLog record, and so should pass "false" here.
- * Other callers probably want to pass true, so that the given values persist
- * in case of crashes.
+ * or not. Normally, this is called from transaction (both normal and
+ * prepared) commit code and the information will be stored in the transaction
+ * commit XLog record, and so it should pass "false" here. The XLog redo code
+ * should use "false" here as well. Other callers probably want to pass true,
+ * so that the given values persist in case of crashes.
*/
void
TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index d48d101..6e7ede9 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -41,6 +41,7 @@
#include <time.h>
#include <unistd.h>
+#include "access/commit_ts.h"
#include "access/htup_details.h"
#include "access/subtrans.h"
#include "access/transam.h"
@@ -56,6 +57,7 @@
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
+#include "replication/origin.h"
#include "replication/walsender.h"
#include "replication/syncrep.h"
#include "storage/fd.h"
@@ -2087,6 +2089,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
bool initfileinval)
{
XLogRecPtr recptr;
+ TimestampTz committs = GetCurrentTimestamp();
START_CRIT_SECTION();
@@ -2094,13 +2097,34 @@ RecordTransactionCommitPrepared(TransactionId xid,
MyPgXact->delayChkpt = true;
/* Emit the XLOG commit record */
- recptr = XactLogCommitRecord(GetCurrentTimestamp(),
+ recptr = XactLogCommitRecord(committs,
nchildren, children, nrels, rels,
ninvalmsgs, invalmsgs,
initfileinval, false,
xid);
/*
+ * Record plain commit ts if not replaying remote actions, or if no
+ * timestamp is configured.
+ */
+ if (replorigin_session_origin == InvalidRepOriginId ||
+ replorigin_session_origin == DoNotReplicateId ||
+ replorigin_session_origin_timestamp == 0)
+ replorigin_session_origin_timestamp = committs;
+ else
+ replorigin_session_advance(replorigin_session_origin_lsn,
+ XactLastRecEnd);
+
+ /*
+ * We don't need to WAL log origin or timestamp here, the commit
+ * record contains all the necessary information and will redo the SET
+ * action during replay.
+ */
+ TransactionTreeSetCommitTsData(xid, nchildren, children,
+ replorigin_session_origin_timestamp,
+ replorigin_session_origin, false);
+
+ /*
* We don't currently try to sleep before flush here ... nor is there any
* support for async commit of a prepared xact (the very idea is probably
* a contradiction)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 068214d..0238771 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1119,6 +1119,9 @@ AtSubStart_ResourceOwner(void)
*
* Returns latest XID among xact and its children, or InvalidTransactionId
* if the xact has no XID. (We compute that here just because it's easier.)
+ *
+ * Note: if you change this function you should also look at
+ * RecordTransactionCommitPrepared in twophase.c.
*/
static TransactionId
RecordTransactionCommit(void)
--
1.9.1
>From 682a6040fd49027e51c2477adb98d3132073ddef Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmo...@pjmodos.net>
Date: Mon, 28 Sep 2015 19:46:23 +0200
Subject: [PATCH 1/2] Replication origin typo fixes (sesssion -> session)
---
src/backend/access/transam/xact.c | 20 ++++++++++----------
src/backend/access/transam/xloginsert.c | 6 +++---
src/backend/replication/logical/origin.c | 26 +++++++++++++-------------
src/include/replication/origin.h | 6 +++---
4 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 56a1cb4..068214d 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1210,12 +1210,12 @@ RecordTransactionCommit(void)
* Record plain commit ts if not replaying remote actions, or if no
* timestamp is configured.
*/
- if (replorigin_sesssion_origin == InvalidRepOriginId ||
- replorigin_sesssion_origin == DoNotReplicateId ||
- replorigin_sesssion_origin_timestamp == 0)
- replorigin_sesssion_origin_timestamp = xactStopTimestamp;
+ if (replorigin_session_origin == InvalidRepOriginId ||
+ replorigin_session_origin == DoNotReplicateId ||
+ replorigin_session_origin_timestamp == 0)
+ replorigin_session_origin_timestamp = xactStopTimestamp;
else
- replorigin_session_advance(replorigin_sesssion_origin_lsn,
+ replorigin_session_advance(replorigin_session_origin_lsn,
XactLastRecEnd);
/*
@@ -1224,8 +1224,8 @@ RecordTransactionCommit(void)
* action during replay.
*/
TransactionTreeSetCommitTsData(xid, nchildren, children,
- replorigin_sesssion_origin_timestamp,
- replorigin_sesssion_origin, false);
+ replorigin_session_origin_timestamp,
+ replorigin_session_origin, false);
}
/*
@@ -5134,12 +5134,12 @@ XactLogCommitRecord(TimestampTz commit_time,
}
/* dump transaction origin information */
- if (replorigin_sesssion_origin != InvalidRepOriginId)
+ if (replorigin_session_origin != InvalidRepOriginId)
{
xl_xinfo.xinfo |= XACT_XINFO_HAS_ORIGIN;
- xl_origin.origin_lsn = replorigin_sesssion_origin_lsn;
- xl_origin.origin_timestamp = replorigin_sesssion_origin_timestamp;
+ xl_origin.origin_lsn = replorigin_session_origin_lsn;
+ xl_origin.origin_timestamp = replorigin_session_origin_timestamp;
}
if (xl_xinfo.xinfo != 0)
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index abd8420..925255f 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -701,11 +701,11 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
}
/* followed by the record's origin, if any */
- if (include_origin && replorigin_sesssion_origin != InvalidRepOriginId)
+ if (include_origin && replorigin_session_origin != InvalidRepOriginId)
{
*(scratch++) = XLR_BLOCK_ID_ORIGIN;
- memcpy(scratch, &replorigin_sesssion_origin, sizeof(replorigin_sesssion_origin));
- scratch += sizeof(replorigin_sesssion_origin);
+ memcpy(scratch, &replorigin_session_origin, sizeof(replorigin_session_origin));
+ scratch += sizeof(replorigin_session_origin);
}
/* followed by main data, if any */
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index b5fa3d8..f55641b 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -148,9 +148,9 @@ typedef struct ReplicationStateCtl
} ReplicationStateCtl;
/* external variables */
-RepOriginId replorigin_sesssion_origin = InvalidRepOriginId; /* assumed identity */
-XLogRecPtr replorigin_sesssion_origin_lsn = InvalidXLogRecPtr;
-TimestampTz replorigin_sesssion_origin_timestamp = 0;
+RepOriginId replorigin_session_origin = InvalidRepOriginId; /* assumed identity */
+XLogRecPtr replorigin_session_origin_lsn = InvalidXLogRecPtr;
+TimestampTz replorigin_session_origin_timestamp = 0;
/*
* Base address into a shared memory array of replication states of size
@@ -803,7 +803,7 @@ replorigin_redo(XLogReaderState *record)
* Tell the replication origin progress machinery that a commit from 'node'
* that originated at the LSN remote_commit on the remote node was replayed
* successfully and that we don't need to do so again. In combination with
- * setting up replorigin_sesssion_origin_lsn and replorigin_sesssion_origin
+ * setting up replorigin_session_origin_lsn and replorigin_session_origin
* that ensures we won't loose knowledge about that after a crash if the
* transaction had a persistent effect (think of asynchronous commits).
*
@@ -1236,7 +1236,7 @@ pg_replication_origin_session_setup(PG_FUNCTION_ARGS)
origin = replorigin_by_name(name, false);
replorigin_session_setup(origin);
- replorigin_sesssion_origin = origin;
+ replorigin_session_origin = origin;
pfree(name);
@@ -1253,9 +1253,9 @@ pg_replication_origin_session_reset(PG_FUNCTION_ARGS)
replorigin_session_reset();
- replorigin_sesssion_origin = InvalidRepOriginId;
- replorigin_sesssion_origin_lsn = InvalidXLogRecPtr;
- replorigin_sesssion_origin_timestamp = 0;
+ replorigin_session_origin = InvalidRepOriginId;
+ replorigin_session_origin_lsn = InvalidXLogRecPtr;
+ replorigin_session_origin_timestamp = 0;
PG_RETURN_VOID();
}
@@ -1268,7 +1268,7 @@ pg_replication_origin_session_is_setup(PG_FUNCTION_ARGS)
{
replorigin_check_prerequisites(false, false);
- PG_RETURN_BOOL(replorigin_sesssion_origin != InvalidRepOriginId);
+ PG_RETURN_BOOL(replorigin_session_origin != InvalidRepOriginId);
}
@@ -1312,8 +1312,8 @@ pg_replication_origin_xact_setup(PG_FUNCTION_ARGS)
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("no replication origin is configured")));
- replorigin_sesssion_origin_lsn = location;
- replorigin_sesssion_origin_timestamp = PG_GETARG_TIMESTAMPTZ(1);
+ replorigin_session_origin_lsn = location;
+ replorigin_session_origin_timestamp = PG_GETARG_TIMESTAMPTZ(1);
PG_RETURN_VOID();
}
@@ -1323,8 +1323,8 @@ pg_replication_origin_xact_reset(PG_FUNCTION_ARGS)
{
replorigin_check_prerequisites(true, false);
- replorigin_sesssion_origin_lsn = InvalidXLogRecPtr;
- replorigin_sesssion_origin_timestamp = 0;
+ replorigin_session_origin_lsn = InvalidXLogRecPtr;
+ replorigin_session_origin_timestamp = 0;
PG_RETURN_VOID();
}
diff --git a/src/include/replication/origin.h b/src/include/replication/origin.h
index 8cec434..daa3b93 100644
--- a/src/include/replication/origin.h
+++ b/src/include/replication/origin.h
@@ -34,9 +34,9 @@ typedef struct xl_replorigin_drop
#define InvalidRepOriginId 0
#define DoNotReplicateId PG_UINT16_MAX
-extern PGDLLIMPORT RepOriginId replorigin_sesssion_origin;
-extern PGDLLIMPORT XLogRecPtr replorigin_sesssion_origin_lsn;
-extern PGDLLIMPORT TimestampTz replorigin_sesssion_origin_timestamp;
+extern PGDLLIMPORT RepOriginId replorigin_session_origin;
+extern PGDLLIMPORT XLogRecPtr replorigin_session_origin_lsn;
+extern PGDLLIMPORT TimestampTz replorigin_session_origin_timestamp;
/* API for querying & manipulating replication origins */
extern RepOriginId replorigin_by_name(char *name, bool missing_ok);
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
