Re: shared-memory based stats collector - v67

Hi,

On 2022-03-23 10:42:03 -0700, Andres Freund wrote:
> On 2022-03-23 17:27:50 +0900, Kyotaro Horiguchi wrote:
> > + /*
> > + * When starting with crash recovery, reset pgstat data - it might not be
> > + * valid. Otherwise restore pgstat data. It's safe to do this here,
> > + * because postmaster will not yet have started any other processes
> > + *
> > + * TODO: With a bit of extra work we could just start with a pgstat file
> > + * associated with the checkpoint redo location we're starting from.
> > + */
> > + if (ControlFile->state == DB_SHUTDOWNED ||
> > + ControlFile->state == DB_SHUTDOWNED_IN_RECOVERY)
> > + pgstat_restore_stats();
> > + else
> > + pgstat_discard_stats();
> > +
> >
> > Before there, InitWalRecovery changes the state to
> > DB_IN_ARCHIVE_RECOVERY if it was either DB_SHUTDOWNED or
> > DB_IN_PRODUCTION. So the stat seems like always discarded on standby.
>
> Hm. I though it worked at some point. I guess there's a reason this commit is
> a separate commit marked WIP ;)

FWIW, it had gotten broken by

commit be1c00ab13a7c2c9299d60cb5a9d285c40e2506c
Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
Date: 2022-02-16 09:22:44 +0200

Move code around in StartupXLOG().

because that moved the spot where
ControlFile->state = DB_IN_CRASH_RECOVERY
is set to an earlier location.

Greetings,

Andres Freund