Horton

Re: should frontend tools use syncfs() ?

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Thomas Munro <thomas.munro@gmail.com>, David Steele <david@pgmasters.net>, Fujii Masao <masao.fujii@oss.nttdata.com>, Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>, Michael Brown <michael.brown@discourse.org>, pgsql-hackers@postgresql.org
Date: 2023-09-01T01:40:12Z
Lists: pgsql-hackers 
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

Commits

  1. Adjust documentation for syncfs().

  2. Improve the naming in wal_sync_method code.

  3. Allow using syncfs() in frontend utilities.

  4. Add support for syncfs() in frontend support functions.

  5. Make enum for sync methods available to frontend code.

  6. Move PG_TEMP_FILE* macros to file_utils.h.

  7. Change client-side fsync_fname() to report errors fatally