Re: Postgres, fsync, and OSs (specifically linux)

On 21 May 2018 at 15:50, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:

> On 21 May 2018 at 12:57, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>
>> On 18 May 2018 at 00:44, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>
>>> Hi,
>>>
>>> On 2018-05-10 09:50:03 +0800, Craig Ringer wrote:
>>> > while ((src = (RewriteMappingFile *)
>>> hash_seq_search(&seq_status)) != NULL)
>>> > {
>>> > if (FileSync(src->vfd, WAIT_EVENT_LOGICAL_REWRITE_SYNC)
>>> != 0)
>>> > - ereport(ERROR,
>>> > + ereport(PANIC,
>>> > (errcode_for_file_access(),
>>> > errmsg("could not fsync file
>>> \"%s\": %m", src->path)));
>>>
>>> To me this (and the other callers) doesn't quite look right. First, I
>>> think we should probably be a bit more restrictive about when PANIC
>>> out. It seems like we should PANIC on ENOSPC and EIO, but possibly not
>>> others. Secondly, I think we should centralize the error handling. It
>>> seems likely that we'll acrue some platform specific workarounds, and I
>>> don't want to copy that knowledge everywhere.
>>>
>>> Also, don't we need the same on close()?
>>>
>>>
>> Yes, we do, and that expands the scope a bit.
>>
>> I agree with Robert that some sort of filter/macro is wise, though naming
>> it clearly will be tricky.
>>
>> I'll have a look.
>>
>>
> On the queue for tomorrow.
>
>
Hi all.

I've revised the fsync patch with the cleanups discussed and gone through
the close() calls.

AFAICS either socket closes, temp file closes, or (for WAL) already PANIC
on close. It's mainly fd.c that needs amendment. Which I've done per the
attached revised patch.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services