From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add pg_file_sync() to adminpack |
Date: | 2020-01-09 17:31:27 |
Message-ID: | CAOBaU_b2WTpCrqgfTOUhXy2E43BnPxzCkdtJEtF7yfjL7XvcVw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jan 9, 2020 at 6:16 PM Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>
> Greetings,
>
> * Julien Rouhaud (rjuju123(at)gmail(dot)com) wrote:
> > On Thu, Jan 9, 2020 at 7:43 AM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> > > On Wed, Dec 25, 2019 at 11:11 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> > > > On Wed, Dec 25, 2019 at 2:01 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> > > > > 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, 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 don't think that's a bug. I'm not sure if every users of those functions
> > > need durable rename and unlink at the expense of performance.
> > > So IMO it's better to add new argument like "durable" to those functions
> > > and durable_rename or _unlink is used only if it's true.
> >
> > It's probably a POLA violation. I'm pretty sure that most people
> > using those functions would expect that a successful call to
> > pg_file_unlink() mean that the file cannot raise from the dead even
> > with certain unlikely circumstances, at least I'd expect so. If
> > performance is a problem here, I'd rather have a new wrapper with a
> > sync flag that defaults to true so it's possible to disable it if
> > needed instead of calling a different function. That being said, I
> > agree with Arthur, it should be handled in a different patch.
>
> Why would you expect that when it isn't the case for the filesystem
> itself..?
Just a usual habit with durable property.
> I agree with Fujii on this- you should have to explicitly ask
> for us to do more than the equivilant filesystem-level operation. We
> shouldn't be forcing that on you.
I just checked other somehow related cases and saw that for instance
COPY TO doesn't flush data either, so it's indeed the expected
behavior.
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Verite | 2020-01-09 17:43:14 | Re: [WIP] UNNEST(REFCURSOR): allowing SELECT to consume data from a REFCURSOR |
Previous Message | Robert Haas | 2020-01-09 17:23:46 | Re: Make autovacuum sort tables in descending order of xid_age |