Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 31 Aug 2023 14:30:33 +0900
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
Subject: Re: should frontend tools use syncfs() ?
Message-ID: <ZPAleai+hsW26++B@paquier.xyz>
References: <20230818160111.GA189711@nathanxps13>
 <ZOP5qoUualu5xl2Z@paquier.xyz>
 <20230822014407.GA449131@nathanxps13>
 <ZOQUSbi8bD1nhxb4@paquier.xyz>
 <20230822020632.GA565843@nathanxps13>
 <ZOQxUU7F85hDQpRO@paquier.xyz>
 <20230822171110.GA711171@nathanxps13>
 <20230829154559.GA2136737@nathanxps13>
 <ZO6JB3s4EepXvSBn@paquier.xyz>
 <20230830011408.GA2269049@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="ArDQUYwRnuxAr60f"
Content-Disposition: inline
In-Reply-To: <20230830011408.GA2269049@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/ZPAleai%2BhsW26%2B%2BB%40paquier.xyz>
Precedence: bulk


--ArDQUYwRnuxAr60f
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--ArDQUYwRnuxAr60f
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmTwJXkACgkQnvQgOdby
QH2hIA//UXbRET6H2xjnYSKICNC91owdcNhAjjiDY6swiIu9CZFZCcBDZ5lr3dnI
2KjVRuY8zlv93y43ok/KrPyderkaNeZVHJsg3Ojrkj7Qbc7BBBBHDMQke+vPHrxa
Ud8D1NzV1ThY5l4E+rquDUVEvd2wV7sLPox2NuMPEXREnFqjewEoY+gKkJMLzhQJ
AyRbl3ku7opdsu81K+lcNmi1SCDn7wxPfFA1kqKxToCGZWLndU+XG2YfgCcCblA7
NyKziGWd6GmV+ynCaUTe8L7W6/1in4du0cnQtw4DSR6WuD6XCTbs6C4bhAoj80Uw
l0Obe1wG3SX7GvYFoxcCMI1kOZJNP6v7TuzV7f2ntn7ZNEkb/S2DC5Sltd6lZQ00
ssojhHiEtm8f3LZ72p+iIqJkWWMHYGfMrHTD5bnac6fWABVWs5bkv17E+PRo+74J
CXIwkQu33yX31S/hOZhCikhQCPH7m8w6dAZyDCdgl8mLBCE22iV3EamfP6kNoRnl
eTunLzM/JvY5MyO2v3OZsZeeNSYQmG2+kGqqZCWjAWHSHTB0YlzQ+NhTFcE9mQfO
aO/RhbosgeXKTNrPO7z2OPfHEXJcY9BSarYGpSjP7ZlQevaw3K7ZT+30bj/OH54a
lvP0CdFwtTjYjh8k3iXG7NdUeHLIxBKW2beoqz3WjjDIJw/i4wg=
=FmuV
-----END PGP SIGNATURE-----

--ArDQUYwRnuxAr60f--