Thread: [9.4] Make full_page_writes only settable on server start?
There is a significant amount of code supporting the changing of full_page_writes while the server is running, including an XLOG_FPW_CHANGE wal record. But I don't see the use-case; surely almost nobody changes this on a running server, because either your filesystem guarantees atomic page writes for you or not. I'd like to submit a patch to just make it into a PGC_POSTMASTER and remove the code to support changing it. Then, I intend to write another patch to make the full-page writes for checksums honor the full_page_writes setting. That will be easier to write once it's a PGC_POSTMASTER. Any reason to keep the setting as a PGC_SIGHUP? Regards,Jeff Davis
On Mon, Sep 2, 2013 at 7:10 PM, Jeff Davis <pgsql@j-davis.com> wrote: > I'd like to submit a patch to just make it into a PGC_POSTMASTER and > remove the code to support changing it. Makes sense to me. I wonder, is anyone really running in production with full_page_writes off? I talked to someone a while ago who used Postgres on ZFS with data journaling, and was perfectly well aware of the fact that theoretically he could safely turn the setting off, and yet chose not to. Now, I know that Greg Smith's book describes the conditions in which it's acceptable and the precautions that should be taken and so on, but in my (admittedly relatively limited) experience, no one actually does it in production. My sample size for people who at least considered doing it (that both believed that Postgres could write pages atomically on their hardware/FS, and also knew that full_page_writes exists and what it means) is only 1. At least in people's minds, it might be that the knowledge that no one runs with full_page_writes off is enough to put them off. It's like building Postgres with a non-standard BLCKSZ -- even if you had solid evidence that it would help performance in your case, would you really want to do it? -- Peter Geoghegan
On 2013-09-02 19:10:57 -0700, Jeff Davis wrote: > There is a significant amount of code supporting the changing of > full_page_writes while the server is running, including an > XLOG_FPW_CHANGE wal record. But I don't see the use-case; surely almost > nobody changes this on a running server, because either your filesystem > guarantees atomic page writes for you or not. > > I'd like to submit a patch to just make it into a PGC_POSTMASTER and > remove the code to support changing it. I've wondered about that before. But always decided that we wouldn't gain all that much by making it PGC_POSTMASTER since the effective value of full_page_writes still needs to be changeable at runtime since we need to force-enable full page writes during a pg_start/stop_backup(). Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
* Peter Geoghegan (pg@heroku.com) wrote:
> I wonder, is anyone really running in production with full_page_writes
> off?
The simple answer to this is "yes, definitely."
I don't know if they change it on-the-fly, but I doubt it.
Thanks,
Stephen
Peter, > I wonder, is anyone really running in production with full_page_writes > off? We're doing so with clients on Joyent (ZFS). And also for "ephemeral" postgres instances -- ones where we've also turned off fsync. It's also quite reasonable to assume that future filesystems will also be able to make better torn page guarantees. So it should remain a setting. I see no reason why it should be a SIGHUP setting, though, if there's any code maintenance required to support it -- frankly, I only change this setting at first system deployment. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Tue, Sep 3, 2013 at 9:23 AM, Josh Berkus <josh@agliodbs.com> wrote: > So it should remain a setting. Sure. I wasn't suggesting deprecating it or anything. -- Peter Geoghegan
<p dir="ltr"><br /> On Sep 3, 2013 6:23 PM, "Josh Berkus" <<a href="mailto:josh@agliodbs.com">josh@agliodbs.com</a>>wrote:<br /> ><br /> > Peter,<br /> ><br /> ><br />> > I wonder, is anyone really running in production with full_page_writes<br /> > > off?<br /> ><br />> We're doing so with clients on Joyent (ZFS). And also for "ephemeral"<br /> > postgres instances -- ones wherewe've also turned off fsync.<br /> ><br /> > It's also quite reasonable to assume that future filesystems willalso<br /> > be able to make better torn page guarantees.<br /> ><br /> > So it should remain a setting. Isee no reason why it should be a<br /> > SIGHUP setting, though, if there's any code maintenance required to<br /> >support it -- frankly, I only change this setting at first system<br /> > deployment.<br /> ><p dir="ltr">Thismatches my experience as well - people certainly use it, but don't change it during runtime.<p dir="ltr">Nothaving looked at the code, but doesn't pg_start_backup use at least parts of the same code path? That one willdefinitely still be able to modify it at runtime.... <p dir="ltr">/Magnus
On Mon, Sep 2, 2013 at 10:10 PM, Jeff Davis <pgsql@j-davis.com> wrote: > There is a significant amount of code supporting the changing of > full_page_writes while the server is running, including an > XLOG_FPW_CHANGE wal record. But I don't see the use-case; surely almost > nobody changes this on a running server, because either your filesystem > guarantees atomic page writes for you or not. Although this is true, the administrator's estimate of whether that guarantee is or is not provided might not be as consistent as the hardware behavior itself. I am generally of the feeling that having to restart the server to change setting sucks, and while it surely sucks less for this setting than some, mostly because few people change this setting in the first place, I'm still not convinced that this is moving in the right direction. > I'd like to submit a patch to just make it into a PGC_POSTMASTER and > remove the code to support changing it. > > Then, I intend to write another patch to make the full-page writes for > checksums honor the full_page_writes setting. That will be easier to > write once it's a PGC_POSTMASTER. I don't think I know exactly what this means. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Sep 3, 2013 at 07:47:22PM +0200, Magnus Hagander wrote: > > On Sep 3, 2013 6:23 PM, "Josh Berkus" <josh@agliodbs.com> wrote: > > > > Peter, > > > > > > > I wonder, is anyone really running in production with full_page_writes > > > off? > > > > We're doing so with clients on Joyent (ZFS). And also for "ephemeral" > > postgres instances -- ones where we've also turned off fsync. > > > > It's also quite reasonable to assume that future filesystems will also > > be able to make better torn page guarantees. > > > > So it should remain a setting. I see no reason why it should be a > > SIGHUP setting, though, if there's any code maintenance required to > > support it -- frankly, I only change this setting at first system > > deployment. > > > > This matches my experience as well - people certainly use it, but don't change > it during runtime. > > Not having looked at the code, but doesn't pg_start_backup use at least parts > of the same code path? That one will definitely still be able to modify it at > runtime.... Yes, this was already mentioned in the thread, but didn't get noticed by a few folks. In summary, no changes needed here. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Tue, 2013-09-03 at 15:42 -0400, Robert Haas wrote: > Although this is true, the administrator's estimate of whether that > guarantee is or is not provided might not be as consistent as the > hardware behavior itself. I am generally of the feeling that having > to restart the server to change setting sucks, and while it surely > sucks less for this setting than some, mostly because few people > change this setting in the first place, I'm still not convinced that > this is moving in the right direction. I think code complexity matters quite a lot. If we can eliminate some complex code in a complex area, and all we give up is a feature with essentially no use case, that sounds like we're moving in the right direction to me. I suppose some might be using it as a hack when they really just want to temporarily disable WAL during a load or something. Seems like a blunt tool though, and I haven't heard of anyone doing that or suggesting it. And it doesn't store the page hole anyway, so the FPI during a load is ordinarily quite small. > > Then, I intend to write another patch to make the full-page writes for > > checksums honor the full_page_writes setting. That will be easier to > > write once it's a PGC_POSTMASTER. > > I don't think I know exactly what this means. XLogSaveBufferForHint() calls XLogCheckBuffer() but doesn't also look at the full page writes setting (like in XLogInsert()). That means, if checksums are enabled and full_page_writes is off, we'll still write some full page images for checksums. I'd like to remedy that. Regards,Jeff Davis
On 2013-09-04 07:57:15 -0700, Jeff Davis wrote: > XLogSaveBufferForHint() calls XLogCheckBuffer() but doesn't also look at > the full page writes setting (like in XLogInsert()). That means, if > checksums are enabled and full_page_writes is off, we'll still write > some full page images for checksums. I'd like to remedy that. I don't think that's really as easy as it sounds without removing the ability to do base backups with full_page_writes = off. The interlocking that would require makes things complex... Personally I'd rather forbid enabling checkpoints in the combination with full_page_writes = off. That doesn't seem like a good idea to me and I am far from convinced it's actually going to work in all corner cases. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Jeff Davis <pgsql@j-davis.com> writes: > On Tue, 2013-09-03 at 15:42 -0400, Robert Haas wrote: >> Although this is true, the administrator's estimate of whether that >> guarantee is or is not provided might not be as consistent as the >> hardware behavior itself. I am generally of the feeling that having >> to restart the server to change setting sucks, and while it surely >> sucks less for this setting than some, mostly because few people >> change this setting in the first place, I'm still not convinced that >> this is moving in the right direction. > I think code complexity matters quite a lot. If we can eliminate some > complex code in a complex area, and all we give up is a feature with > essentially no use case, that sounds like we're moving in the right > direction to me. Isn't this whole discussion academic in view of Andres' point? http://www.postgresql.org/message-id/20130903121935.GB5783@awork2.anarazel.de regards, tom lane
On Wed, 2013-09-04 at 17:00 +0200, Andres Freund wrote: > On 2013-09-04 07:57:15 -0700, Jeff Davis wrote: > > XLogSaveBufferForHint() calls XLogCheckBuffer() but doesn't also look at > > the full page writes setting (like in XLogInsert()). That means, if > > checksums are enabled and full_page_writes is off, we'll still write > > some full page images for checksums. I'd like to remedy that. > > I don't think that's really as easy as it sounds without removing the > ability to do base backups with full_page_writes = off. The interlocking > that would require makes things complex... I didn't dig into that part yet. I was mostly distracted by the code to support changing full_page_writes with SIGHUP. One option would be to have XLogInsert return early if full_page_writes is off, it's an XLOG_FPI record, and forcePageWrites is off. > Personally I'd rather forbid enabling checkpoints in the combination > with full_page_writes = off. That doesn't seem like a good idea to me > and I am far from convinced it's actually going to work in all corner cases. Hmm. It's good to be cautious when deploying on a less-common configuration. However, I don't think it's a good idea to reject seemingly valid combinations that are supposed to work due to a lack of confidence in the review/testing process. Might be an area warranting some further review and testing; I'll take a look, but feel free to tell me if you can think of specific problem areas. Regards,Jeff Davis
On Wed, 2013-09-04 at 11:32 -0400, Tom Lane wrote: > Jeff Davis <pgsql@j-davis.com> writes: > > I think code complexity matters quite a lot. If we can eliminate some > > complex code in a complex area, and all we give up is a feature with > > essentially no use case, that sounds like we're moving in the right > > direction to me. > > Isn't this whole discussion academic in view of Andres' point? Maybe "complex code" was an overstatement. We'd be able to eliminate the XLOG_FPW_CHANGE, UpdateFullPageWrites(), and one of the members of XLogCtlInsert; and make xlog.c slightly shorter in the process. The first time I looked at doing the patch to honor full_page_writes=off when checksums are on, the fact that fullPageWrites was changeable was a distraction. Since I saw little or no value in what the code offered, my instinct was to see if we could get rid of it. It looks like Simon went to significant effort to maintain the full_page_writes as a PGC_SUGHUP: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=8366c780 Maybe he has the best perspective on the value versus complexity? Regards,Jeff Davis
On 2013-09-04 09:23:20 -0700, Jeff Davis wrote: > On Wed, 2013-09-04 at 11:32 -0400, Tom Lane wrote: > > Jeff Davis <pgsql@j-davis.com> writes: > > > I think code complexity matters quite a lot. If we can eliminate some > > > complex code in a complex area, and all we give up is a feature with > > > essentially no use case, that sounds like we're moving in the right > > > direction to me. > > > > Isn't this whole discussion academic in view of Andres' point? > > Maybe "complex code" was an overstatement. We'd be able to eliminate the > XLOG_FPW_CHANGE, UpdateFullPageWrites(), and one of the members of > XLogCtlInsert; and make xlog.c slightly shorter in the process. That path is also executed during a normal restart and during promotion. Check the invocation of UpdateFullPageWrites() in StartupXLOG(). Note that a standby needs to be able to follow a primaries full_page_writes setting during a promotion. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Sep 5, 2013 at 1:55 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2013-09-04 09:23:20 -0700, Jeff Davis wrote: >> On Wed, 2013-09-04 at 11:32 -0400, Tom Lane wrote: >> > Jeff Davis <pgsql@j-davis.com> writes: >> > > I think code complexity matters quite a lot. If we can eliminate some >> > > complex code in a complex area, and all we give up is a feature with >> > > essentially no use case, that sounds like we're moving in the right >> > > direction to me. >> > >> > Isn't this whole discussion academic in view of Andres' point? >> >> Maybe "complex code" was an overstatement. We'd be able to eliminate the >> XLOG_FPW_CHANGE, UpdateFullPageWrites(), and one of the members of >> XLogCtlInsert; and make xlog.c slightly shorter in the process. > > That path is also executed during a normal restart and during > promotion. Check the invocation of UpdateFullPageWrites() in > StartupXLOG(). Note that a standby needs to be able to follow a > primaries full_page_writes setting during a promotion. Yes, this is required for the backup from the standby. If we make the GUC contect to PGC_POSTMASTER, I think that we can remove XLOG_FPW_CHANGE and treat full_page_writes the same way as wal_level, max_connections, i.e., the parameter which CheckRequiredParameterValues() handles. Regards, -- Fujii Masao