Re: Temporary tables versus wraparound... again

Hi,

On 2022-12-01 11:13:01 -0500, Greg Stark wrote:
> On Sat, 5 Nov 2022 at 11:34, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Greg Stark <stark(at)mit(dot)edu> writes:
> > > Simple Rebase
> >
> > I took a little bit of a look through these.
> >
> > * I find 0001 a bit scary, specifically that it's decided it's
> > okay to apply extract_autovac_opts, pgstat_fetch_stat_tabentry_ext,
> > and especially relation_needs_vacanalyze to another session's
> > temp table. How safe is that really?
>
> I can look a bit more closely but none of them are doing any thing
> with the table itself, just the catalog entries which afaik have
> always been fair game for other sessions. So I'm not really clear what
> kind of unsafeness you're asking about.

Is that actually true? Don't we skip some locking operations for temporary
tables, which then also means catalog modifications cannot safely be done in
other sessions?

> > Also, skipping that update
> > for non-temp tables immediately falsifies ResetVacStats'
> > claimed charter of "resetting to the same values used when
> > creating tables". Surely GetOldestMultiXactId isn't *that*
> > expensive, especially compared to the costs of file truncation.
> > I think you should just do GetOldestMultiXactId straight up,
> > and maybe submit a separate performance-improvement patch
> > to make it do the other thing (in both places) for temp tables.
>
> Hm. the feedback I got earlier was that it was quite expensive. That
> said, I think the concern was about the temp tables where the truncate
> was happening on every transaction commit when they're used. For
> regular truncates I'm not sure how important optimizing it is.

And it's not called just once, but once for each relation.

> > * I wonder if this is the best place for ResetVacStats --- it
> > doesn't seem to be very close to the code it needs to stay in
> > sync with. If there's no better place, perhaps adding cross-
> > reference comments in both directions would be advisable.
>
> I'll look at that. I think relfrozenxid and relfrozenmxid are touched
> in a lot of places so it may be tilting at windmills to try to
> centralize the code working with them at this point...

Not convinced. Yes, there's plenty of references to relfrozenxid, but most of
them don't modify it.

I find it problematic that ResetVacStats() bypasses tableam. Normal vacuums
etc go through tableam but you put a ResetVacStats() besides each call to
table_relation_nontransactional_truncate(). Seems like this should just be in
heapam_relation_nontransactional_truncate()?

Is it a good idea to use heap_inplace_update() in ResetVacStats()?

Greetings,

Andres Freund