Petr Jelinek wrote:
> On 2015-09-02 16:14, Fujii Masao wrote:
> >On Wed, Aug 5, 2015 at 2:16 AM, Robert Haas <[email protected]> wrote:
> >>On Mon, Aug 3, 2015 at 10:31 AM, Fujii Masao <[email protected]> wrote:
> >>>track_commit_timestamp tracks COMMIT PREPARED as expected in standby
> >>>server,
> >>>but not in master server. Is this intentional? It should track COMMIT
> >>>PREPARED
> >>>even in master? Otherwise, we cannot use commit_timestamp feature to check
> >>>the replication lag properly while we use 2PC.
> >>
> >>That sounds like it must be a bug. I think you should add it to the
> >>open items list.
>
> Attached fixes this. It includes advancement of replication origin as well.
> I didn't feel like doing refactor of commit code this late in 9.5 cycle
> though, so I went with the code duplication + note in xact.c.
Thanks, your proposed behavior looks reasonable. I didn't like the
existing coding nor the fact that with your patch we'd have two copies
of it, so I changed a bit instead to be more understandable. Hopefully I
didn't break too many things. This patch includes the patch for the
other commitTS open item too.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
*** a/src/backend/access/transam/commit_ts.c
--- b/src/backend/access/transam/commit_ts.c
***************
*** 122,150 **** static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
* subtrans implementation changes in the future, we might want to revisit the
* 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.
*/
void
TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
! RepOriginId nodeid, bool do_xlog)
{
int i;
TransactionId headxid;
TransactionId newestXact;
! if (!track_commit_timestamp)
return;
/*
* Comply with the WAL-before-data rule: if caller specified it wants this
* value to be recorded in WAL, do so before touching the data.
*/
! if (do_xlog)
WriteSetTimestampXlogRec(xid, nsubxids, subxids, timestamp, nodeid);
/*
--- 122,160 ----
* subtrans implementation changes in the future, we might want to revisit the
* decision of storing timestamp info for each subxid.
*
! * The replaying_xlog parameter indicates whether the module should execute
! * its write even if the feature is nominally disabled, because we're replaying
! * a record generated from a master where the feature is enabled.
! *
! * The write_xlog parameter tells us whether to include an XLog record of this
! * or not. Normally, this is called from transaction commit routines (both
! * normal and prepared) and the information will be stored in the transaction
! * commit XLog record, and so they should pass "false" for this. 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,
TransactionId *subxids, TimestampTz timestamp,
! RepOriginId nodeid,
! bool replaying_xlog, bool write_xlog)
{
int i;
TransactionId headxid;
TransactionId newestXact;
! /* We'd better not try to write xlog during replay */
! Assert(!(write_xlog && replaying_xlog));
!
! /* No-op if feature not enabled, unless replaying WAL */
! if (!track_commit_timestamp && !replaying_xlog)
return;
/*
* Comply with the WAL-before-data rule: if caller specified it wants this
* value to be recorded in WAL, do so before touching the data.
*/
! if (write_xlog)
WriteSetTimestampXlogRec(xid, nsubxids, subxids, timestamp, nodeid);
/*
***************
*** 906,912 **** commit_ts_redo(XLogReaderState *record)
subxids = NULL;
TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
! setts->timestamp, setts->nodeid, false);
if (subxids)
pfree(subxids);
}
--- 916,923 ----
subxids = NULL;
TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
! setts->timestamp, setts->nodeid, false,
! true);
if (subxids)
pfree(subxids);
}
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
***************
*** 41,46 ****
--- 41,47 ----
#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,63 ****
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
! #include "replication/walsender.h"
#include "replication/syncrep.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/predicate.h"
--- 57,65 ----
#include "miscadmin.h"
#include "pg_trace.h"
#include "pgstat.h"
! #include "replication/origin.h"
#include "replication/syncrep.h"
+ #include "replication/walsender.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/predicate.h"
***************
*** 2070,2077 **** RecoverPreparedTransactions(void)
/*
* RecordTransactionCommitPrepared
*
! * This is basically the same as RecordTransactionCommit: in particular,
! * we must set the delayChkpt flag to avoid a race condition.
*
* We know the transaction made at least one XLOG entry (its PREPARE),
* so it is never possible to optimize out the commit record.
--- 2072,2080 ----
/*
* RecordTransactionCommitPrepared
*
! * This is basically the same as RecordTransactionCommit (q.v. if you change
! * this function): in particular, we must set the delayChkpt flag to avoid a
! * race condition.
*
* We know the transaction made at least one XLOG entry (its PREPARE),
* so it is never possible to optimize out the commit record.
***************
*** 2087,2092 **** RecordTransactionCommitPrepared(TransactionId xid,
--- 2090,2101 ----
bool initfileinval)
{
XLogRecPtr recptr;
+ TimestampTz committs = GetCurrentTimestamp();
+ bool replorigin;
+
+ /* are we using the replication origins feature? */
+ replorigin = (replorigin_session_origin != InvalidRepOriginId &&
+ replorigin_session_origin != DoNotReplicateId);
START_CRIT_SECTION();
***************
*** 2094,2105 **** RecordTransactionCommitPrepared(TransactionId xid,
MyPgXact->delayChkpt = true;
/* Emit the XLOG commit record */
! recptr = XactLogCommitRecord(GetCurrentTimestamp(),
nchildren, children, nrels, rels,
ninvalmsgs, invalmsgs,
initfileinval, false,
xid);
/*
* 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
--- 2103,2135 ----
MyPgXact->delayChkpt = true;
/* Emit the XLOG commit record */
! recptr = XactLogCommitRecord(committs,
nchildren, children, nrels, rels,
ninvalmsgs, invalmsgs,
initfileinval, false,
xid);
+
+ if (replorigin)
+ /* Tell replorigin that this session is moving forward */
+ replorigin_session_advance(replorigin_session_origin_lsn,
+ XactLastRecEnd);
+
+ /*
+ * Record commit timestamp. The value comes from plain commit timestamp if
+ * replorigin is not enabled, or replorigin already set a value for us in
+ * replorigin_session_origin_timestamp otherwise.
+ *
+ * We don't need to WAL-log anything here, as the commit record written
+ * above already contains the data.
+ */
+ if (!replorigin || replorigin_session_origin_timestamp == 0)
+ replorigin_session_origin_timestamp = committs;
+
+ TransactionTreeSetCommitTsData(xid, nchildren, children,
+ replorigin_session_origin_timestamp,
+ replorigin_session_origin, false, 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/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***************
*** 42,50 ****
#include "miscadmin.h"
#include "pgstat.h"
#include "replication/logical.h"
- #include "replication/walsender.h"
- #include "replication/syncrep.h"
#include "replication/origin.h"
#include "storage/fd.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
--- 42,50 ----
#include "miscadmin.h"
#include "pgstat.h"
#include "replication/logical.h"
#include "replication/origin.h"
+ #include "replication/syncrep.h"
+ #include "replication/walsender.h"
#include "storage/fd.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
***************
*** 1119,1124 **** AtSubStart_ResourceOwner(void)
--- 1119,1126 ----
*
* 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.)
+ *
+ * If you change this function, see RecordTransactionCommitPrepared also.
*/
static TransactionId
RecordTransactionCommit(void)
***************
*** 1172,1177 **** RecordTransactionCommit(void)
--- 1174,1188 ----
}
else
{
+ bool replorigin;
+
+ /*
+ * are we using the replication origins feature? Or, in other words,
+ * are we replaying remote actions?
+ */
+ replorigin = (replorigin_session_origin != InvalidRepOriginId &&
+ replorigin_session_origin != DoNotReplicateId);
+
/*
* Begin commit critical section and insert the commit XLOG record.
*/
***************
*** 1206,1231 **** RecordTransactionCommit(void)
RelcacheInitFileInval, forceSyncCommit,
InvalidTransactionId /* plain commit */ );
! /*
! * 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 = xactStopTimestamp;
! 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);
}
/*
--- 1217,1243 ----
RelcacheInitFileInval, forceSyncCommit,
InvalidTransactionId /* plain commit */ );
! if (replorigin)
! /* Tell replorigin that this session is moving forward */
replorigin_session_advance(replorigin_session_origin_lsn,
XactLastRecEnd);
/*
! * Record commit timestamp. The value comes from plain commit
! * timestamp if replorigin is not enabled, or replorigin already set a
! * value for us in replorigin_session_origin_timestamp otherwise.
! *
! * We don't need to WAL-log anything here, as the commit record written
! * above already contains the data.
*/
+
+ if (!replorigin || replorigin_session_origin_timestamp == 0)
+ replorigin_session_origin_timestamp = xactStopTimestamp;
+
TransactionTreeSetCommitTsData(xid, nchildren, children,
replorigin_session_origin_timestamp,
! replorigin_session_origin,
! false, false);
}
/*
***************
*** 5321,5327 **** xact_redo_commit(xl_xact_parsed_commit *parsed,
/* Set the transaction commit timestamp and metadata */
TransactionTreeSetCommitTsData(xid, parsed->nsubxacts, parsed->subxacts,
commit_time, origin_id,
! false);
if (standbyState == STANDBY_DISABLED)
{
--- 5333,5339 ----
/* Set the transaction commit timestamp and metadata */
TransactionTreeSetCommitTsData(xid, parsed->nsubxacts, parsed->subxacts,
commit_time, origin_id,
! false, true);
if (standbyState == STANDBY_DISABLED)
{
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 5826,5844 **** do { \
minValue))); \
} while(0)
- #define RecoveryRequiresBoolParameter(param_name, currValue, masterValue) \
- do { \
- bool _currValue = (currValue); \
- bool _masterValue = (masterValue); \
- if (_currValue != _masterValue) \
- ereport(ERROR, \
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
- errmsg("hot standby is not possible because it requires \"%s\" to be same on master and standby (master has \"%s\", standby has \"%s\")", \
- param_name, \
- _masterValue ? "true" : "false", \
- _currValue ? "true" : "false"))); \
- } while(0)
-
/*
* Check to see if required parameters are set high enough on this server
* for various aspects of recovery operation.
--- 5826,5831 ----
*** a/src/include/access/commit_ts.h
--- b/src/include/access/commit_ts.h
***************
*** 24,30 **** extern bool check_track_commit_timestamp(bool *newval, void **extra,
extern void TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
! RepOriginId nodeid, bool do_xlog);
extern bool TransactionIdGetCommitTsData(TransactionId xid,
TimestampTz *ts, RepOriginId *nodeid);
extern TransactionId GetLatestCommitTsData(TimestampTz *ts,
--- 24,31 ----
extern void TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
! RepOriginId nodeid,
! bool replaying_xlog, bool write_xlog);
extern bool TransactionIdGetCommitTsData(TransactionId xid,
TimestampTz *ts, RepOriginId *nodeid);
extern TransactionId GetLatestCommitTsData(TimestampTz *ts,
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
