Обсуждение: 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
Вложения
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
Вложения
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