Hi, 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. > > > Yeah, I was wondering about the same thing - can't this mean we fail to > > start autovacuum? Let's say we delete a significant fraction of a huge > > table, but then we crash before the next checkpoint. With this patch we > > restore the last stats snapshot, which can easily have > > > > n_dead_tup | 0 > > n_mod_since_analyze | 0 > > > > for the table. And thus we fail to notice the table needs autovacuum. > > AFAIK we run autovacuum on all tables with missing stats (or am I > > wrong?). That's what's happening on replicas after switchover/failover > > too, right? > > That's the opposite, please look at relation_needs_vacanalyze(). If a > table does not have any stats found in pgstats, like on HEAD after a > crash, then autoanalyze is skipped and autovacuum happens only for the > anti-wrap case. > > For the database case, rebuild_database_list() uses > pgstat_fetch_stat_dbentry() three times, discards entirely databases > that have no stats. Again, there should be a stats entry post a > crash upon a reconnection. > > So there's an argument in saying that the situation does not get worse > here and that we actually may improve odds by keeping a trace of the > previous stats, no?
I agree, we still could get autoanalyze/autovacuum skipped due to obsolete/stales stats being restored from the last checkpoint but that's better than having them always skipped (current HEAD). > In most cases, there would be a stats entry > post-crash as an INSERT or a scan would have re-created it, Yeap. > but the > stats would reflect the state registered since the last crash recovery > (even on HEAD, a bulk delete bumping the dead tuple counter would not > be detected post-crash). Right. > The case of spiky workloads may impact the > decision-making, of course, but at least we'd allow autovacuum to take > some decision over giving up entirely based on some previous state of > the stats. That sounds like a win for me with steady workloads, less > for bulby workloads.. I agree and it is not worst (though not ideal) that currently on HEAD. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
