Re: should frontend tools use syncfs() ?

On Thu, Aug 31, 2023 at 08:48:58AM -0700, Nathan Bossart wrote:
> On Thu, Aug 31, 2023 at 02:30:33PM +0900, Michael Paquier wrote:
> > - Should we have some regression tests? We should only need one test
> > in one of the binaries to be able to stress the new code paths of
> > file_utils.c with syncfs. The cheapest one may be pg_dump with a
> > dump in directory format? Note that we have tests there that depend
> > on lz4 or gzip existing, which are conditional.
>
> I added one for initdb in v8.

+my $supports_syncfs = check_pg_config("#define HAVE_SYNCFS 1");

That should be OK this way. The extra running time is not really
visible, right?

+command_ok([ 'initdb', '-S', $datadir, '--sync-method', 'fsync' ],
+ 'sync method fsync');

Removing this one may be fine, actually, because we test the sync
paths on other places like pg_dump.

> Ha, I was just thinking about this, too. I actually split it into 3
> patches. The first adds DataDirSyncMethod and uses it for
> recovery_init_sync_method. The second adds syncfs() support in
> file_utils.c. And the third adds the ability to specify syncfs in the
> frontend utilities. WDYT?

This split is OK by me, so WFM.
--
Michael