On Thu, Apr 30, 2015 at 11:29 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
>> 2. I don't know why it's part of this patch.
>
> In 20150115133245.GG5245@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