Re: shared-memory based stats collector - v67 - Mailing list pgsql-hackers

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.linnakangas@iki.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