Re: Flush pgstats file during checkpoints

Hi,

On Fri, Jul 12, 2024 at 03:42:21PM +0900, Michael Paquier wrote:
> On Fri, Jul 05, 2024 at 01:52:31PM +0900, Michael Paquier wrote:
> > On Sat, Jun 29, 2024 at 11:13:04PM +0200, Tomas Vondra wrote:
> >> I think those are two independent issues - knowing that the snapshot is
> >> from the last checkpoint, and knowing that it's correct (not corrupted).
> >> And yeah, we should be careful about fsync/durable_rename.
> > 
> > Yeah, that's bugging me as well.  I don't really get why we would not
> > want durability at shutdown for this data.  So how about switching the
> > end of pgstat_write_statsfile() to use durable_rename()?  Sounds like
> > an independent change, worth on its own.
> 
> Please find attached a rebased patch set with the durability point
> addressed in 0001.  There were also some conflicts.

Thanks!

Looking at 0001:

+               /* error logged already */

Maybe mention it's already logged by durable_rename() (like it's done in
InstallXLogFileSegment(), BaseBackup() for example).

Except this nit, 0001 LGTM.

Need to spend more time and thoughts on 0002+.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com