Re: Temporary tables versus wraparound... again

On Mon, 28 Mar 2022 at 16:30, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > Make ON COMMIT DELETE ROWS reset relfrozenxmin and other table stats
> > like normal truncate. Otherwise even typical short-lived transactions
> > using temporary tables can easily cause them to reach relfrozenxid.
>
> Might be worth mentioning that ON COMMIT DELETE is implemented as truncating
> tables. If we actually implemented it as deleting rows, it'd not at all be
> correct to reset relfrozenxmin.

In the commit message or are you saying this needs documentation or a comment?

> > Also add warnings when old temporary tables are found to still be in
> > use during autovacuum. Long lived sessions using temporary tables are
> > required to vacuum them themselves.
>
> I'd do that in a separate patch.

Hm, seems a bit small but sure no problem, I'll split it out.

> > + pgcform->relfrozenxid = RecentXmin;
>
> Hm. Is RecentXmin guaranteed to be valid at this point?

I mentioned the same worry. But ok, I just looked into it and it's
definitely not a problem. We only do truncates after either a user
issued TRUNCATE when the table was created in the same transaction or
at commit iff a flag is set indicating temporary tables have been
used. Either way a snapshot has been taken. I've added some comments
and an assertion and I think if assertions are disabled and this
impossible condition is hit we can just skip the stats reset.

> > + pgcform->relminmxid = GetOldestMultiXactId();
>
> Ugh. That's pretty expensive for something now done at a much higher rate than
> before.

This I'm really not sure about. I really don't know much about
multixacts. I've been reading a bit but I'm not sure what to do yet.
I'm wondering if there's something cheaper we can use. We don't need
the oldest mxid that might be visible in a table somewhere, just the
oldest that has a real live uncommitted transaction in it that could
yet create new tuples in the truncated table.

In the case of temporary tables I think we could just set it to the
next mxid since there are no live transactions capable of inserting
into the temporary table. But in the case of a table created in this
transaction then that wouldn't be good enough. I think? I'm not clear
whether existing mxids get reused for new updates if they happen to
have the same set of locks in them as some existing mxid.

> we put else if on a separate line from }. And { also is always on a separate
> line.

Sorry, old habits...

Incidentally.... in doing the above I noticed an actual bug :( The
toast reset had the wrong relid in it. I'll add the toast table to the
test too.

--
greg