Re: Add pg_file_sync() to adminpack

Hello,

On 2019/12/25 23:12, Julien Rouhaud wrote:
> On Wed, Dec 25, 2019 at 2:01 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> I'd like to propose to add pg_file_sync() function into contrib/adminpack.
>> This function fsyncs the specified file or directory named by its argument.
>> IMO this is useful, for example, when you want to fsync the file that
>> pg_file_write() writes out or that COPY TO exports the data into,
>> for durability. Thought?

+1 too. I have a thought, but maybe it is just a nitpicking.

pg_file_sync() calls fsync_fname() function from fd.c. And I think it
might bring problems because fsync_fname() uses data_sync_elevel() to
get elevel. As a result if data_sync_retry GUC is false fsync_fname()
might raise PANIC message.

It isn't case if a file doesn't exist. But if there are no permissions
on the file:

PANIC: could not open file "testfile": Permissions denied
server closed the connection unexpectedly

It could be fixed by implementing a function like
pg_file_sync_internal() or by making the function fsync_fname_ext()
external.

> +1, that seems like a useful wrapper. Looking at existing functions,
> I see that there's a pg_file_rename() in adminpack, but it doesn't use
> durable_rename nor does it try to perform any fsync. Same for
> pg_file_unlink vs. durable_unlink. It's probably worth fixing that at
> the same time?

I think it might be a different patch.

--
Arthur