Re: PostgreSQL's handling of fsync() errors is unsafe and risks data loss at least on XFS

On Fri, Apr 6, 2018 at 1:27 PM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
> On 6 April 2018 at 07:37, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
>> Note: as I've brought up in another thread, it turns out that PG is not
>> handling fsync errors correctly even when the OS _does_ do the right
>> thing (discovered by testing on FreeBSD).
>
> Yikes. For other readers, the related thread for this is
> https://www.postgresql.org/message-id/87y3i1ia4w.fsf@news-spur.riddles.org.uk

Yeah. That's really embarrassing, especially after beating up on
various operating systems all week. It's also an independent issue --
let's keep that on the other thread and get it fixed.

> I see the failed fync, then the same fd being fsync()d without error on the
> next checkpoint, which succeeds.
>
> postgres 9602 [003] 72380.325817: syscalls:sys_enter_fsync: fd:
> 0x00000005
> postgres 9602 [003] 72380.325931: syscalls:sys_exit_fsync:
> 0xfffffffffffffffb
> ...
> postgres 9602 [000] 72381.336767: syscalls:sys_enter_fsync: fd:
> 0x00000005
> postgres 9602 [000] 72381.336840: syscalls:sys_exit_fsync: 0x0
>
> ... and Pg continues merrily on its way without realising it lost data:
>
> [72379.834872] XFS (dm-0): writeback error on sector 118752
> [72380.324707] XFS (dm-0): writeback error on sector 118688
>
> In this test I set things up so the checkpointer would see the first fsync()
> error. But if I make checkpoints less frequent, the bgwriter aggressive, and
> kernel dirty writeback aggressive, it should be possible to have the failure
> go completely unobserved too. I'll try that next, because we've already
> largely concluded that the solution to the issue above is to PANIC on
> fsync() error. But if we don't see the error at all we're in trouble.

I suppose you only see errors because the file descriptors linger open
in the virtual file descriptor cache, which is a matter of luck
depending on how many relation segment files you touched. One thing
you could try to confirm our understand of the Linux 4.13+ policy
would be to hack PostgreSQL so that it reopens the file descriptor
every time in mdsync(). See attached.

--
Thomas Munro
http://www.enterprisedb.com