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 <[email protected]> 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