On Sun, Jan 5, 2020 at 8:42 PM Michael Paquier <michael@paquier.xyz> wrote:
> The behavior of the code in 246a6c8 has changed so as a non-existing
> temporary namespace is considered as not in use, in which case
> autovacuum would consider this relation to be orphaned, and it would
> then try to drop it. Anyway, just a revert of the patch is not a good
> idea either, because keeping around the old behavior allows any user
> to create orphaned relations that autovacuum would just ignore in
> 9.4~10, leading to ultimately a forced shutdown of the instance as no
> cleanup can happen if this goes unnoticed. This also puts pg_class
> into an inconsistent state as pg_class entries would include
> references to a namespace that does not exist for sessions still
> holding its own references to myTempNamespace/myTempToastNamespace.
I'm not arguing for a revert of 246a6c8. I think we should just change this:
ereport(LOG,
(errmsg("autovacuum: dropping orphan
temp table \"%s.%s.%s\"",
get_database_name(MyDatabaseId),
get_namespace_name(classForm->relnamespace),
NameStr(classForm->relname))));
To look more like:
char *nspname = get_namespace_name(classForm->relnamespace);
if (nspname != NULL)
ereport(..."autovacuum: dropping orphan temp table \"%s.%s.%s\"...)
else
ereport(..."autovacuum: dropping orphan temp table with OID %u"....)
If we do that, then I think we can just revert
a052f6cbb84e5630d50b68586cecc127e64be639 completely. As a side
benefit, this would also provide some insurance against other
possibly-problematic situations, like a corrupted pg_class row with a
garbage value in the relnamespace field, which is something I've seen
multiple times in the field.
I can't quite understand your comments about why we shouldn't do that,
but the reported bug is just a null pointer reference. Incredibly,
autovacuum.c seems to have been using get_namespace_name() without a
null check since 2006, so it's not really the fault of your patch as I
had originally thought. I wonder how in the world we've managed to get
away with it for as long as we have.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
