On 2015-10-02 22:02, Robert Haas wrote:
On Fri, Oct 2, 2015 at 2:59 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
Robert Haas wrote:
The standby can have the feature enabled even though the master has it
disabled? That seems like it can only lead to heartache.
Can you elaborate?
Sort of. Our rule up until now has always been that the standby is an
exact copy of the master. I suspect deviating from that behavior will
introduce bugs. I suspect having the standby make data changes that
aren't WAL-logged will introduce bugs; not to be unkind, but that
certainly seems like a lesson to take from what happened with
multixacts.
I agree with that sentiment.
Attached patch adds variable to the shmem which is used for module
activation tracking - set to true in ActiveCommitTs() and false in
DeactivateCommitTs(). All the checks inside the commit_ts code were
changed to use this new variable. I also removed the static variable
Alvaro added in previous commit because it's not needed anymore.
The patch also does full cleanup of the shmem state in
DeactivateCommitTs() so that standby does not have stale last committed
transaction info after enable/disable/enable cycle on primary
I also removed no longer used do_wal parameters in couple of functions.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 24b8291..8af8dbe 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -80,11 +80,17 @@ static SlruCtlData CommitTsCtlData;
/*
* We keep a cache of the last value set in shared memory. This is protected
* by CommitTsLock.
+ *
+ * This is also good place to keep the activation status. We need to keep
+ * the activation status separate from the GUC bellow because the standby needs
+ * to activate the module if the primary has it active independently of what
+ * track_commit_timestamp setting is on standby.
*/
typedef struct CommitTimestampShared
{
TransactionId xidLastCommit;
CommitTimestampEntry dataLastCommit;
+ bool commitTsActive;
} CommitTimestampShared;
CommitTimestampShared *commitTsShared;
@@ -93,14 +99,6 @@ CommitTimestampShared *commitTsShared;
/* GUC variable */
bool track_commit_timestamp;
-/*
- * When this is set, commit_ts is force-enabled during recovery. This is so
- * that a standby can replay WAL records coming from a master with the setting
- * enabled. (Note that this doesn't enable SQL access to the data; it's
- * effectively write-only until the GUC itself is enabled.)
- */
-static bool enable_during_recovery;
-
static void SetXidCommitTsInPage(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz ts,
RepOriginId nodeid, int pageno);
@@ -109,7 +107,7 @@ static void TransactionIdSetCommitTs(TransactionId xid, TimestampTz ts,
static int ZeroCommitTsPage(int pageno, bool writeXlog);
static bool CommitTsPagePrecedes(int page1, int page2);
static void ActivateCommitTs(void);
-static void DeactivateCommitTs(bool do_wal);
+static void DeactivateCommitTs(void);
static void WriteZeroPageXlogRec(int pageno);
static void WriteTruncateXlogRec(int pageno);
static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
@@ -148,11 +146,8 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId headxid;
TransactionId newestXact;
- /*
- * No-op if the module is not enabled, but allow writes in a standby
- * during recovery.
- */
- if (!track_commit_timestamp && !enable_during_recovery)
+ /* No-op if the module is not active. */
+ if (!commitTsShared->commitTsActive)
return;
/*
@@ -284,7 +279,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
TransactionId newestCommitTs;
/* Error if module not enabled */
- if (!track_commit_timestamp)
+ if (!commitTsShared->commitTsActive)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("could not get commit timestamp data"),
@@ -367,7 +362,7 @@ GetLatestCommitTsData(TimestampTz *ts, RepOriginId *nodeid)
TransactionId xid;
/* Error if module not enabled */
- if (!track_commit_timestamp)
+ if (!commitTsShared->commitTsActive)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("could not get commit timestamp data"),
@@ -493,6 +488,7 @@ CommitTsShmemInit(void)
commitTsShared->xidLastCommit = InvalidTransactionId;
TIMESTAMP_NOBEGIN(commitTsShared->dataLastCommit.time);
commitTsShared->dataLastCommit.nodeid = InvalidRepOriginId;
+ commitTsShared->commitTsActive = false;
}
else
Assert(found);
@@ -566,7 +562,7 @@ CompleteCommitTsInitialization(void)
* any leftover data.
*/
if (!track_commit_timestamp)
- DeactivateCommitTs(true);
+ DeactivateCommitTs();
}
/*
@@ -588,11 +584,11 @@ CommitTsParameterChange(bool newvalue, bool oldvalue)
*/
if (newvalue)
{
- if (!track_commit_timestamp && !oldvalue)
+ if (!commitTsShared->commitTsActive)
ActivateCommitTs();
}
- else if (!track_commit_timestamp && oldvalue)
- DeactivateCommitTs(false);
+ else if (commitTsShared->commitTsActive)
+ DeactivateCommitTs();
}
/*
@@ -645,7 +641,7 @@ ActivateCommitTs(void)
}
LWLockRelease(CommitTsLock);
- /* Finally, create the current segment file, if necessary */
+ /* Create the current segment file, if necessary */
if (!SimpleLruDoesPhysicalPageExist(CommitTsCtl, pageno))
{
int slotno;
@@ -657,8 +653,10 @@ ActivateCommitTs(void)
LWLockRelease(CommitTsControlLock);
}
- /* We can now replay xlog records from this module */
- enable_during_recovery = true;
+ /* Change the activation status in shared memory. */
+ LWLockAcquire(CommitTsLock, LW_EXCLUSIVE);
+ commitTsShared->commitTsActive = true;
+ LWLockRelease(CommitTsLock);
}
/*
@@ -672,7 +670,7 @@ ActivateCommitTs(void)
* possibly-invalid data; also removes segments of old data.
*/
static void
-DeactivateCommitTs(bool do_wal)
+DeactivateCommitTs(void)
{
TransactionId xid = ShmemVariableCache->nextXid;
int pageno = TransactionIdToCTsPage(xid);
@@ -684,11 +682,26 @@ DeactivateCommitTs(bool do_wal)
CommitTsCtl->shared->latest_page_number = pageno;
LWLockRelease(CommitTsControlLock);
+ /*
+ * Cleanup the status in the shared memory.
+ *
+ * We reset everything in the commitTsShared record to prevent user from
+ * getting confusing data about last committed transaction on the standby
+ * when the module was activated repeatedly on the primary.
+ */
LWLockAcquire(CommitTsLock, LW_EXCLUSIVE);
+
+ commitTsShared->commitTsActive = false;
+ commitTsShared->xidLastCommit = InvalidTransactionId;
+ TIMESTAMP_NOBEGIN(commitTsShared->dataLastCommit.time);
+ commitTsShared->dataLastCommit.nodeid = InvalidRepOriginId;
+
ShmemVariableCache->oldestCommitTs = InvalidTransactionId;
ShmemVariableCache->newestCommitTs = InvalidTransactionId;
+
LWLockRelease(CommitTsLock);
+
/*
* Remove *all* files. This is necessary so that there are no leftover
* files; in the case where this feature is later enabled after running
@@ -698,9 +711,6 @@ DeactivateCommitTs(bool do_wal)
* tidy.)
*/
(void) SlruScanDirectory(CommitTsCtl, SlruScanDirCbDeleteAll, NULL);
-
- /* No longer enabled on recovery */
- enable_during_recovery = false;
}
/*
@@ -740,7 +750,7 @@ ExtendCommitTs(TransactionId newestXact)
int pageno;
/* nothing to do if module not enabled */
- if (!track_commit_timestamp && !enable_during_recovery)
+ if (!commitTsShared->commitTsActive)
return;
/*
@@ -768,7 +778,7 @@ ExtendCommitTs(TransactionId newestXact)
* Note that we don't need to flush XLOG here.
*/
void
-TruncateCommitTs(TransactionId oldestXact, bool do_wal)
+TruncateCommitTs(TransactionId oldestXact)
{
int cutoffPage;
@@ -784,8 +794,7 @@ TruncateCommitTs(TransactionId oldestXact, bool do_wal)
return; /* nothing to remove */
/* Write XLOG record */
- if (do_wal)
- WriteTruncateXlogRec(cutoffPage);
+ WriteTruncateXlogRec(cutoffPage);
/* Now we can remove the old CommitTs segment(s) */
SimpleLruTruncate(CommitTsCtl, cutoffPage);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 6d55148..7c4ef58 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1140,7 +1140,7 @@ vac_truncate_clog(TransactionId frozenXID,
* Truncate CLOG, multixact and CommitTs to the oldest computed value.
*/
TruncateCLOG(frozenXID);
- TruncateCommitTs(frozenXID, true);
+ TruncateCommitTs(frozenXID);
TruncateMultiXact(minMulti, minmulti_datoid);
/*
diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h
index 1b95b58..3844bb3 100644
--- a/src/include/access/commit_ts.h
+++ b/src/include/access/commit_ts.h
@@ -40,7 +40,7 @@ extern void CompleteCommitTsInitialization(void);
extern void ShutdownCommitTs(void);
extern void CheckPointCommitTs(void);
extern void ExtendCommitTs(TransactionId newestXact);
-extern void TruncateCommitTs(TransactionId oldestXact, bool do_wal);
+extern void TruncateCommitTs(TransactionId oldestXact);
extern void SetCommitTsLimit(TransactionId oldestXact,
TransactionId newestXact);
extern void AdvanceOldestCommitTs(TransactionId oldestXact);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
