Re: Add pg_file_sync() to adminpack - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Add pg_file_sync() to adminpack
Date
Msg-id 20200113134600.GC41902@paquier.xyz
Whole thread Raw
In response to Re: Add pg_file_sync() to adminpack  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Add pg_file_sync() to adminpack  (Julien Rouhaud <rjuju123@gmail.com>)
Re: Add pg_file_sync() to adminpack  (Fujii Masao <masao.fujii@oss.nttdata.com>)
List pgsql-hackers
On Sat, Jan 11, 2020 at 02:12:15AM +0900, Fujii Masao wrote:
> I'm not sure if returning false with WARNING only in some error cases
> is really good idea or not. At least for me, it's more intuitive to
> return true on success and emit an ERROR otherwise. I'd like to hear
> more opinions about this. Also if returning true on success is rather
> confusing, we can change its return type to void.

An advantage of not issuing an ERROR if that when working on a list of
files (for example a WITH RECURSIVE on the whole data directory?), you
can then know which files could not be synced instead of seeing one
ERROR about one file, while being unsure about the state of the
others.

> Could you elaborate why? But if it's not good to sync the existing directory
> in the regression test, we may need to give up testing the sync of directory.
> Another idea is to add another function like pg_mkdir() into adminpack
> and use the directory that we newly created by using that function,
> for the test. Or better idea?

We should avoid potentially costly tests in any regression scenario if
we have a way to do so.  I like your idea of having a pg_mkdir(), that
feels more natural to have as there is already pg_file_write().
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: isTempNamespaceInUse() is incorrect with its handling ofMyBackendId
Next
From: Julien Rouhaud
Date:
Subject: Re: isTempNamespaceInUse() is incorrect with its handling ofMyBackendId