On Thu, Apr 30, 2015 at 11:29 PM, Abhijit Menon-Sen <a...@2ndquadrant.com> wrote: >> 2. I don't know why it's part of this patch. > > In 20150115133245.gg5...@awork2.anarazel.de, Andres explained his > rationale as follows: > > «What I am thinking of is that, currently, if you start the server > for initial loading with fsync=off, and then restart it, you're open > to data loss. So when the current config file setting is changed > from off to on, we should fsync the data directory. Even if there > was no crash restart.»
That's awfully clever, but I'm not sure I like the idea of trying to be that clever. I think if users temporarily disable fsync, they should be responsible for using initdb -S after if that is needed in their situation, and this should be documented. It seems to me that, at a minimum, it would be good to split those controversial and definitely not-back-patchable changes into their own patch. >> Also, it seems awfully unfortunate to me that we're duplicating a >> whole pile of code into xlog.c here. > > I have pointed out and discussed the duplication several times. I did it > this way only because we were considering backporting the changes, and > at the time it seemed better to do this and fix the duplication in a > separate patch. As I've mentioned a few times, I don't mind duplicating the code if we have frontend and backend versions that are materially different. But I do mind putting it into xlog.c instead of some place that's actually appropriate. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
