On Thu, Sep 30, 2021 at 12:49:36PM +0900, Michael Paquier wrote:
> On Wed, Sep 29, 2021 at 07:43:41PM -0500, Justin Pryzby wrote:
> > Forking this thread in which Thomas implemented syncfs for the startup process
> > (61752afb2).
> >
https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BSG9jSW3ekwib0cSdC0yD-jReJ21X4bZAmqxoWTLTc2A%40mail.gmail.com
> >
> > Is there any reason that initdb/pg_basebackup/pg_checksums/pg_rewind shouldn't
> > use syncfs() ?
>
> That makes sense.
>
> > do_syncfs() is in src/backend/ so would need to be duplicated^Wimplemented in
> > common.
>
> The fd handling in the backend makes things tricky if trying to plug
> in a common interface, so I'd rather do that as this is frontend-only
> code.
>
> > They can't use the GUC, so need to add an cmdline option or look at an
> > environment variable.
>
> fsync_pgdata() is going to manipulate many inodes anyway, because
> that's a code path designed to do so. If we know that syncfs() is
> just going to be better, I'd rather just call it by default if
> available and not add new switches to all the frontend tools in need
> of flushing the data folder, switches that are not documented in your
> patch.
It is a draft/POC, after all.
The argument against using syncfs by default is that it could be worse than
recursive fsync if a tiny 200MB postgres instance lives on a shared filesystem
along with other, larger applications (maybe a larger postgres instance).
There's also an argument that syncfs might be unreliable in the case of a write
error. (But I agreed with Thomas' earlier assessment: that claim caries little
weight since fsync() itself wasn't reliable for 20some years).
I didn't pursue this patch, as it's easier for me to use /bin/sync -f. Someone
should adopt it if interested.
--
Justin
