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