Hi, On 2022-03-23 17:27:50 +0900, Kyotaro Horiguchi wrote: > At Mon, 21 Mar 2022 14:30:17 -0700, Andres Freund <and...@anarazel.de> wrote > in > > > Right now we reset stats for replicas, even if we start from a shutdown > > > checkpoint. That seems pretty unnecessary with this patch: > > > - 0021-pgstat-wip-only-reset-pgstat-data-after-crash-re.patch > > > > Might raise this in another thread for higher visibility. > > + /* > + * 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 ;) > In the first place, I'm not sure it is valid that a standby from a > cold backup takes over the stats from the primary. I don't really see a reason not to use the stats in that case - we have a correct stats file after all. But it doesn't seem too important. What I actually find worth addressing is the case of standbys starting in DB_SHUTDOWNED_IN_RECOVERY. Right now we always throw stats away after a *graceful* restart of a standby, which doesn't seem great. Greetings, Andres Freund
