Thread: Why does WAL_DEBUG macro need to be defined by default?
Hi, I found that by default WAL_DEBUG macro has been defined in 9.2dev and 9.1. I'm very surprised at this. Why does WAL_DEBUG need to be defined by default? The performance overhead introduced by WAL_DEBUG is really vanishingly low? WAL_DEBUG was defined in the following commit: 53dbc27c62d8e1b6c5253feba04a5094cb8fe046 ---------------------- Support unlogged tables. The contents of an unlogged table are WAL-logged; thus, they are not available on standby servers and are truncatedwhenever the database system enters recovery. Indexes on unlogged tables are also unlogged. Unlogged GiST indexesare not currently supported. ---------------------- Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On 07.10.2011 12:19, Fujii Masao wrote: > Hi, > > I found that by default WAL_DEBUG macro has been defined in > 9.2dev and 9.1. I'm very surprised at this. Why does WAL_DEBUG > need to be defined by default? The performance overhead > introduced by WAL_DEBUG is really vanishingly low? > > WAL_DEBUG was defined in the following commit: > 53dbc27c62d8e1b6c5253feba04a5094cb8fe046 > > ---------------------- > Support unlogged tables. > > The contents of an unlogged table are WAL-logged; thus, they are not > available on standby servers and are truncated whenever the database > system enters recovery. Indexes on unlogged tables are also unlogged. > Unlogged GiST indexes are not currently supported. > ---------------------- I'm pretty sure that change was included in the commit by accident. That said, the overhead of WAL_DEBUG probably is insignificant, as long as you don't actually set wal_debug=on. I wonder if we should leave it enabled. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Fri, Oct 7, 2011 at 5:19 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > I found that by default WAL_DEBUG macro has been defined in > 9.2dev and 9.1. I'm very surprised at this. Why does WAL_DEBUG > need to be defined by default? The performance overhead > introduced by WAL_DEBUG is really vanishingly low? > > WAL_DEBUG was defined in the following commit: > 53dbc27c62d8e1b6c5253feba04a5094cb8fe046 > > ---------------------- > Support unlogged tables. > > The contents of an unlogged table are WAL-logged; thus, they are not > available on standby servers and are truncated whenever the database > system enters recovery. Indexes on unlogged tables are also unlogged. > Unlogged GiST indexes are not currently supported. > ---------------------- Oh, dear. That was a mistake on my part. :-( The funny thing is that I've been thinking all of these months about how convenient it is that we defined WAL_DEBUG in debug builds, not realizing that (1) we were defining it all the time, not just in debug builds and (2) I was the one who accidentally did that. Sorry, all. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> wrote: > The funny thing is that I've been thinking all of these months > about how convenient it is that we defined WAL_DEBUG in debug > builds IMO, --enable-debug should not do anything but include debugging symbols. The ability to get a useful stack trace from a production crash, without compromising performance, is just too important by itself to consider conditioning any other behavior on it. -Kevin
On Fri, Oct 7, 2011 at 1:03 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Robert Haas <robertmhaas@gmail.com> wrote: >> The funny thing is that I've been thinking all of these months >> about how convenient it is that we defined WAL_DEBUG in debug >> builds > > IMO, --enable-debug should not do anything but include debugging > symbols. The ability to get a useful stack trace from a production > crash, without compromising performance, is just too important by > itself to consider conditioning any other behavior on it. So, should I go revert this change in head and 9.1, or does anyone else want to argue for Heikki's position that we should just leave it on, on the theory that it's too cheap to matter? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Fri, Oct 7, 2011 at 1:03 PM, Kevin Grittner > <Kevin.Grittner@wicourts.gov> wrote: > > Robert Haas <robertmhaas@gmail.com> wrote: > >> The funny thing is that I've been thinking all of these months > >> about how convenient it is that we defined WAL_DEBUG in debug > >> builds > > > > IMO, --enable-debug should not do anything but include debugging > > symbols. ?The ability to get a useful stack trace from a production > > crash, without compromising performance, is just too important by > > itself to consider conditioning any other behavior on it. > > So, should I go revert this change in head and 9.1, or does anyone > else want to argue for Heikki's position that we should just leave it > on, on the theory that it's too cheap to matter? I would just fix it in head. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Fri, Oct 7, 2011 at 4:06 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Fri, Oct 7, 2011 at 1:03 PM, Kevin Grittner >> <Kevin.Grittner@wicourts.gov> wrote: >> > Robert Haas <robertmhaas@gmail.com> wrote: >> >> The funny thing is that I've been thinking all of these months >> >> about how convenient it is that we defined WAL_DEBUG in debug >> >> builds >> > >> > IMO, --enable-debug should not do anything but include debugging >> > symbols. ?The ability to get a useful stack trace from a production >> > crash, without compromising performance, is just too important by >> > itself to consider conditioning any other behavior on it. >> >> So, should I go revert this change in head and 9.1, or does anyone >> else want to argue for Heikki's position that we should just leave it >> on, on the theory that it's too cheap to matter? > > I would just fix it in head. That just seems weird. Either it's cheap enough not to matter (in which case there's no reason to revert that change at all) or it's expensive enough to matter (in which case presumably we don't want to leave it on in 9.1 for the 5 years or so it remains a supported release). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Fri, Oct 7, 2011 at 4:06 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Robert Haas wrote: > >> On Fri, Oct 7, 2011 at 1:03 PM, Kevin Grittner > >> <Kevin.Grittner@wicourts.gov> wrote: > >> > Robert Haas <robertmhaas@gmail.com> wrote: > >> >> The funny thing is that I've been thinking all of these months > >> >> about how convenient it is that we defined WAL_DEBUG in debug > >> >> builds > >> > > >> > IMO, --enable-debug should not do anything but include debugging > >> > symbols. ?The ability to get a useful stack trace from a production > >> > crash, without compromising performance, is just too important by > >> > itself to consider conditioning any other behavior on it. > >> > >> So, should I go revert this change in head and 9.1, or does anyone > >> else want to argue for Heikki's position that we should just leave it > >> on, on the theory that it's too cheap to matter? > > > > I would just fix it in head. > > That just seems weird. Either it's cheap enough not to matter (in > which case there's no reason to revert that change at all) or it's > expensive enough to matter (in which case presumably we don't want to > leave it on in 9.1 for the 5 years or so it remains a supported > release). I am concerned about changing behavior on people in a minor release --- it is not about risk in this case. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Fri, Oct 7, 2011 at 9:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> On Fri, Oct 7, 2011 at 4:06 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > Robert Haas wrote: >> >> On Fri, Oct 7, 2011 at 1:03 PM, Kevin Grittner >> >> <Kevin.Grittner@wicourts.gov> wrote: >> >> > Robert Haas <robertmhaas@gmail.com> wrote: >> >> >> The funny thing is that I've been thinking all of these months >> >> >> about how convenient it is that we defined WAL_DEBUG in debug >> >> >> builds >> >> > >> >> > IMO, --enable-debug should not do anything but include debugging >> >> > symbols. ?The ability to get a useful stack trace from a production >> >> > crash, without compromising performance, is just too important by >> >> > itself to consider conditioning any other behavior on it. >> >> >> >> So, should I go revert this change in head and 9.1, or does anyone >> >> else want to argue for Heikki's position that we should just leave it >> >> on, on the theory that it's too cheap to matter? >> > >> > I would just fix it in head. >> >> That just seems weird. Either it's cheap enough not to matter (in >> which case there's no reason to revert that change at all) or it's >> expensive enough to matter (in which case presumably we don't want to >> leave it on in 9.1 for the 5 years or so it remains a supported >> release). > > I am concerned about changing behavior on people in a minor release --- > it is not about risk in this case. Well, I still maintain that if the performance impact is low enough that we can get away with that, it's probably not worth fixing in master either. But at any rate, we now have three opinions on what to do about this. Anyone else want to cast a vote (preferably not for an entirely new option)? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Oct 7, 2011 at 4:06 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> I would just fix it in head.
> That just seems weird. Either it's cheap enough not to matter (in
> which case there's no reason to revert that change at all) or it's
> expensive enough to matter (in which case presumably we don't want to
> leave it on in 9.1 for the 5 years or so it remains a supported
> release).
It needs to be reverted. I don't understand why you didn't do that
instantly upon the mistake being pointed out to you.
regards, tom lane