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-08-31T05:30:33Z
Lists: pgsql-hackers 
On Tue, Aug 29, 2023 at 06:14:08PM -0700, Nathan Bossart wrote:
> That seems fair enough.  I did this in v7.  I restructured fsync_pgdata()
> and fsync_dir_recurse() so that any new sync methods should cause compiler
> warnings until they are implemented.

That's pretty cool and easier to maintain in the long term.

After sleeping on it, there are two things that popped up in my mind
that may be worth considering:
- 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.
- Perhaps 0002 should be split into two parts?  The first patch could
introduce DataDirSyncMethod in file_utils.h with the new routines in
file_utils.h (including syncfs support), and the second patch would
plug the new option to all the binaries.  In the first patch, I would
hardcode DATA_DIR_SYNC_METHOD_FSYNC.
--
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