Robert Haas <robertmh...@gmail.com> writes:
> This is really two separate changes:
> 1. Teach backends to remove any leftover pg_temp_%d schema on startup.
> 2. Teach autovacuum to remove any leftover tables in a pg_temp_%d
> schema if the backend is active but in some other database (rather
> than only when the backend is not active at all).
> Both of those changes seem broadly reasonable to me, but what do other
> people think?
I just read through this thread for the first time; sorry for not
paying attention sooner.
I'm uncomfortable with all the discussion of changing the autovacuum
launcher's algorithm; all of that seems complicated and unsure of making
anyone's life better. It definitely does nothing for the problem
originally stated, that autovacuum is skipping an orphaned temp table
altogether. That's not relevant to the actually proposed patch;
I'm just saying that I'm not sure anything useful would come of that.
Now as for the problem originally stated, step 1 alone doesn't fix it,
and there's reason not to like that change much. Forcing backends to
clear their temp schemas immediately on connection will slow down
connection times, and for applications that never make any temp tables,
that's just a dead loss (though admittedly it might not be too expensive
in that case). But the problem here is that it doesn't fix the issue.
The reason that a temp table might stay orphaned for a long time, if we're
speaking of an app that does use temp tables, is that it's in a very
high-numbered temp schema and only once in a blue moon does the database
have enough connections for that temp schema to be used at all. Forcing
on-connection cleaning doesn't fix that.
So the correct fix is to improve autovacuum's check to discover whether
an old temp table is orphaned, so that it isn't fooled by putative
owner processes that are connected to some other DB. Step 2 of the
proposed patch tries to do that, but it's only half right: we need a
change near line 2264 not only line 2090. (Evidently, we need either
a comment that the logic is repeated, or else refactor so that there's
only one copy.)
Now, you can argue that autovacuum's check can be fooled by an "owner"
backend that is connected to the current DB but hasn't actually taken
possession of its assigned temp schema (and hence the table in question
really is orphaned after all). This edge case could be taken care of by
having backends clear their temp schema immediately, as in step 1 of the
patch. But I still think that that is an expensive way to catch what
would be a really infrequent case. Perhaps a better idea is to have
backends advertise in PGPROC whether they have taken possession of their
assigned temp schema, and then autovacuum could ignore putative owners
that aren't showing that flag. Or we could just do nothing about that,
on the grounds that nobody has shown the case to be worth worrying about.
Temp schemas that are sufficiently low-numbered to be likely to have
an apparent owner are also likely to get cleaned out by actual temp
table creation reasonably often, so I'm not at all convinced that we
need a mechanism that covers this case. We do need something for the
cross-DB case though, because even a very low-numbered temp schema
can be problematic if it's in a seldom-used DB, which seems to be the
case that was complained of originally.
On the whole, my vote is to fix and apply step 2, and leave it at that.
regards, tom lane
