RecordTransactionCommit() and SharedInvalidationMessages
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases. Am I
missing something?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
On Tue, Aug 10, 2010 at 9:30 AM, Robert Haas <robertmhaas@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.
Perhaps right. During not hot standby, there is no backend which the
startup process should send invalidation message to in the standby.
So, ISTM we don't need to log invalidation message when wal_level is
not hot_standby.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Wed, Aug 11, 2010 at 1:17 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Aug 10, 2010 at 9:30 AM, Robert Haas <robertmhaas@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.Perhaps right. During not hot standby, there is no backend which the
startup process should send invalidation message to in the standby.
So, ISTM we don't need to log invalidation message when wal_level is
not hot_standby.
The fix looks pretty simple (see attached), although I don't have any
clear idea how to test it. I guess the question is whether we should
back-patch this to 9.0. It isn't technically necessary for
correctness, but the whole point of introducing the wal_level GUC was
to insulate people not running Hot Standby from possible bugs in the
Hot Standby code, as well as to avoid unnecessary WAL bloat, so on
balance I'm inclined to think we should go ahead and back-patch it.
Other opinions?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
Attachments:
record_transaction_commmit.patchapplication/octet-stream; name=record_transaction_commmit.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 60e22d0..2d21fdf 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -922,8 +922,11 @@ RecordTransactionCommit(void)
/* Get data needed for commit record */
nrels = smgrGetPendingDeletes(true, &rels, &haveNonTemp);
nchildren = xactGetCommittedChildren(&children);
- nmsgs = xactGetCommittedInvalidationMessages(&invalMessages,
- &RelcacheInitFileInval);
+ if (wal_level == WAL_LEVEL_HOT_STANDBY)
+ nmsgs = xactGetCommittedInvalidationMessages(&invalMessages,
+ &RelcacheInitFileInval);
+ else
+ nmsgs = 0;
/*
* If we haven't been assigned an XID yet, we neither can, nor do we want
On 11/08/10 16:46, Robert Haas wrote:
On Wed, Aug 11, 2010 at 1:17 AM, Fujii Masao<masao.fujii@gmail.com> wrote:
On Tue, Aug 10, 2010 at 9:30 AM, Robert Haas<robertmhaas@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.Perhaps right. During not hot standby, there is no backend which the
startup process should send invalidation message to in the standby.
So, ISTM we don't need to log invalidation message when wal_level is
not hot_standby.The fix looks pretty simple (see attached), although I don't have any
clear idea how to test it.
Should use XLogStandbyInfoActive() macro, for the sake of consistency.
I guess the question is whether we should
back-patch this to 9.0. It isn't technically necessary for
correctness, but the whole point of introducing the wal_level GUC was
to insulate people not running Hot Standby from possible bugs in the
Hot Standby code, as well as to avoid unnecessary WAL bloat, so on
balance I'm inclined to think we should go ahead and back-patch it.
+1 for backpatching. Keeping the branches closer to each other makes
backporting any future fixes easier too.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
On Wed, Aug 11, 2010 at 11:35 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
On 11/08/10 16:46, Robert Haas wrote:
On Wed, Aug 11, 2010 at 1:17 AM, Fujii Masao<masao.fujii@gmail.com>
wrote:On Tue, Aug 10, 2010 at 9:30 AM, Robert Haas<robertmhaas@gmail.com>
wrote:It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.Perhaps right. During not hot standby, there is no backend which the
startup process should send invalidation message to in the standby.
So, ISTM we don't need to log invalidation message when wal_level is
not hot_standby.The fix looks pretty simple (see attached), although I don't have any
clear idea how to test it.Should use XLogStandbyInfoActive() macro, for the sake of consistency.
And, RelcacheInitFileInval should be initialized with false just in case.
I guess the question is whether we should
back-patch this to 9.0. It isn't technically necessary for
correctness, but the whole point of introducing the wal_level GUC was
to insulate people not running Hot Standby from possible bugs in the
Hot Standby code, as well as to avoid unnecessary WAL bloat, so on
balance I'm inclined to think we should go ahead and back-patch it.+1 for backpatching. Keeping the branches closer to each other makes
backporting any future fixes easier too.
+1
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Thu, Aug 12, 2010 at 12:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.
[...]
The fix looks pretty simple (see attached), although I don't have any
clear idea how to test it.Should use XLogStandbyInfoActive() macro, for the sake of consistency.
And, RelcacheInitFileInval should be initialized with false just in case.
How about this?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
Attachments:
record_transaction_commmit-v2.patchapplication/octet-stream; name=record_transaction_commmit-v2.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 511aba2..503c722 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -915,15 +915,16 @@ RecordTransactionCommit(void)
bool haveNonTemp;
int nchildren;
TransactionId *children;
- int nmsgs;
+ int nmsgs = 0;
SharedInvalidationMessage *invalMessages = NULL;
- bool RelcacheInitFileInval;
+ bool RelcacheInitFileInval = false;
/* Get data needed for commit record */
nrels = smgrGetPendingDeletes(true, &rels, &haveNonTemp);
nchildren = xactGetCommittedChildren(&children);
- nmsgs = xactGetCommittedInvalidationMessages(&invalMessages,
- &RelcacheInitFileInval);
+ if (XLogStandbyInfoActive())
+ nmsgs = xactGetCommittedInvalidationMessages(&invalMessages,
+ &RelcacheInitFileInval);
/*
* If we haven't been assigned an XID yet, we neither can, nor do we want
On Fri, Aug 13, 2010 at 10:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Aug 12, 2010 at 12:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.[...]
The fix looks pretty simple (see attached), although I don't have any
clear idea how to test it.Should use XLogStandbyInfoActive() macro, for the sake of consistency.
And, RelcacheInitFileInval should be initialized with false just in case.
How about this?
Looks good to me.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Thu, Aug 12, 2010 at 9:43 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, Aug 13, 2010 at 10:24 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Aug 12, 2010 at 12:11 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
It appears to me that RecordTransactionCommit() only needs to WAL-log
shared invalidation messages when wal_level is hot_standby, but I
don't see a guard to prevent it from doing it in all cases.Should use XLogStandbyInfoActive() macro, for the sake of consistency.
And, RelcacheInitFileInval should be initialized with false just in case.
How about this?
Looks good to me.
Committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company