Re: silent data loss with ext4 / all current versions

On Thu, Feb 4, 2016 at 2:34 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Feb 4, 2016 at 12:02 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Tue, Feb 2, 2016 at 4:20 PM, Michael Paquier wrote:
>>> Not wrong, and this leads to the following:
>>> void rename_safe(const char *old, const char *new, bool isdir, int elevel);
>>> Controlling elevel is necessary per the multiple code paths that would
>>> use it. Some use ERROR, most of them FATAL, and a bit of WARNING. Does
>>> that look fine?
>>
>> After really coding it, I finished with the following thing:
>> +int
>> +rename_safe(const char *old, const char *new)
>>
>> There is no need to extend that for directories, well we could of
>> course but all the renames happen on files so I see no need to make
>> that more complicated. More refactoring of the other rename() calls
>> could be done as well by extending rename_safe() with a flag to fsync
>> the data within a critical section, particularly for the replication
>> slot code. I have let that out to not complicate more the patch.
>
> Andres just poked me (2m far from each other now) regarding the fact
> that we should fsync even after the link() calls when
> HAVE_WORKING_LINK is used. So we could lose some meta data here. Mea
> culpa. And the patch misses that.

So, attached is an updated patch that adds a new routine link_safe()
to ensure that a hard link is on-disk persistent. link() is called
twice in timeline.c and once in xlog.c, so those three code paths are
impacted. I noticed as well that my previous patch was sometimes doing
palloc calls in a critical section (oops), I fixed that on the way.

Thoughts welcome.
--
Michael