Re: Checkpoint not retrying failed fsync?

Thomas Munro <thomas.munro@enterprisedb.com>

From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-04-06T00:56:54Z
Lists: pgsql-hackers

Attachments

On Fri, Apr 6, 2018 at 11:36 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Fri, Apr 6, 2018 at 11:34 AM, Andrew Gierth
> <andrew@tao11.riddles.org.uk> wrote:
>> Right.
>>
>> But I don't think just copying the value is sufficient; if a new bit was
>> set while we were processing the old ones, how would we know which to
>> clear? We couldn't just clear all the bits afterwards because then we
>> might lose a request.
>
> Agreed.  The attached draft patch handles that correctly, I think.

After some testing, here is a better one for review.  Changes:

1.  The copy wasn't really needed.
2.  errno needed to be restored (in case bms_union stomped on it),
thanks to Andrew for an off-list complaint about that.
3.  Memory context was wrong.
4.  bms_first_member() eats its input.  Need to use bms_next_member() instead.
5.  Mustn't leak in error path (that was a pre-existing bug).

Also here's a patch to test it.  If the file /tmp/broken_fsync exists,
you'll see a fake EIO when you CHECKPOINT.

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

Commits

  1. Don't forget about failed fsync() requests.