Re: fdatasync performance problem with large number of DB files

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@gmail.com>, David Steele <david@pgmasters.net>, Fujii Masao <masao.fujii@oss.nttdata.com>, Bruce Momjian <bruce@momjian.us>, Paul Guo <guopa@vmware.com>, Tom Lane <tgl@sss.pgh.pa.us>, Michael Brown <michael.brown@discourse.org>, pgsql-hackers@postgresql.org
Date: 2021-06-04T14:39:04Z
Lists: pgsql-hackers
On Fri, Jun 04, 2021 at 04:24:02PM +0900, Michael Paquier wrote:
> On Sat, May 29, 2021 at 02:23:21PM -0500, Justin Pryzby wrote:
> > On Tue, May 25, 2021 at 07:13:59PM -0500, Justin Pryzby wrote:
> >> On Sat, Mar 20, 2021 at 12:16:27PM +1300, Thomas Munro wrote:
> >> > > > +     {
> >> > > > +             {"recovery_init_sync_method", PGC_POSTMASTER, ERROR_HANDLING_OPTIONS,
> >> > > > +                     gettext_noop("Sets the method for synchronizing the data directory before crash recovery."),
> >> > > > +             },
> >> 
> >> Is there any reason why this can't be PGC_SIGHUP ?
> > 
> > I can't see any reason why this is nontrivial.
> 
> I think that we had better let recovery_init_sync_method as
> PGC_POSTMASTER, to stay on the safe side.  SyncDataDirectory() only
> gets called now in the backend code by the startup process after a
> crash at the beginning of recovery, so switching to PGC_SIGHUP would
> have zero effect to begin with.  Now, let's not forget that
> SyncDataDirectory() is a published API, and if anything exterior were
> to call that, it does not seem right to me to make that its behavior
> reloadable at will.

You said switching to SIGHUP "would have zero effect"; but, actually it allows
an admin who's DB took a long time in recovery/startup to change the parameter
without shutting down the service.  This mitigates the downtime if it crashes
again.  I think that's at least 50% of how this feature might end up being
used.

It might be "safer" if fsync were PGC_POSTMASTER, but it's allowed to change at
runtime that parameter, which is much more widely applicable.  I've already
mentioned restart_after_crash, and remove_temp_files_after_crash.

-- 
Justin



Commits

  1. Change recovery_init_sync_method to PGC_SIGHUP.

  2. Provide recovery_init_sync_method=syncfs.