On Fri, Jan 10, 2020 at 05:01:25PM +0900, Michael Paquier wrote: > This makes me wonder how much we should try to outsmart somebody which > puts the catalogs in such a inconsistent state. Hmm. Perhaps at the > end autovacuum should just ignore such entries and just don't help the > user at all as this also comes with its own issues with the storage > level as well as smgr.c uses rd_backend. And if the user plays with > temporary namespaces like that with superuser rights, he likely knows > what he is doing. Perhaps not :D, in which case autovacuum may not be > the best thing to decide that. I still think we should make the log > of autovacuum.c for orphaned relations more careful with its coding > though, and fix it with the previous patch. The documentation of > isTempNamespaceInUse() could gain in clarity, just a nit from me while > looking at the surroundings. And actually I found an issue with its > logic, as the routine would not consider a temp namespace in use for a > session's own MyBackendId. As that's only used for autovacuum, this > has no consequence, but let's be correct in hte long run. > > And this gives the attached after a closer lookup. Thoughts?
Thinking more about it, this has a race condition if a temporary schema is removed after collecting the OIDs in the drop phase. So the updated attached is actually much more conservative and does not need an update of the log message, without giving up on the improvements done in v11~. In 9.4~10, the code of the second phase relies on GetTempNamespaceBackendId() which causes an orphaned relation to not be dropped in the event of a missing namespace. I'll just leave that alone for a couple of days now.. -- Michael
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index c82f9fc4b5..0987986a8f 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3235,10 +3235,14 @@ isTempNamespaceInUse(Oid namespaceId)
backendId = GetTempNamespaceBackendId(namespaceId);
- if (backendId == InvalidBackendId ||
- backendId == MyBackendId)
+ /* No such temporary namespace? */
+ if (backendId == InvalidBackendId)
return false;
+ /* Is the namespace used by this backend? */
+ if (backendId == MyBackendId)
+ return true;
+
/* Is the backend alive? */
proc = BackendIdGetProc(backendId);
if (proc == NULL)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index f0e40e36af..9eb8132a37 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2071,9 +2071,11 @@ do_autovacuum(void)
{
/*
* We just ignore it if the owning backend is still active and
- * using the temporary schema.
+ * using the temporary schema. If the namespace does not exist
+ * ignore the entry.
*/
- if (!isTempNamespaceInUse(classForm->relnamespace))
+ if (!isTempNamespaceInUse(classForm->relnamespace) &&
+ get_namespace_name(classForm->relnamespace) != NULL)
{
/*
* The table seems to be orphaned -- although it might be that
@@ -2202,6 +2204,7 @@ do_autovacuum(void)
Oid relid = lfirst_oid(cell);
Form_pg_class classForm;
ObjectAddress object;
+ char *nspname;
/*
* Check for user-requested abort.
@@ -2243,7 +2246,15 @@ do_autovacuum(void)
continue;
}
- if (isTempNamespaceInUse(classForm->relnamespace))
+ nspname = get_namespace_name(classForm->relnamespace);
+
+ /*
+ * Nothing to do for a relation with a missing namespace. This
+ * check is the same as above when building the list of orphan
+ * relations.
+ */
+ if (isTempNamespaceInUse(classForm->relnamespace) ||
+ nspname == NULL)
{
UnlockRelationOid(relid, AccessExclusiveLock);
continue;
@@ -2253,8 +2264,7 @@ do_autovacuum(void)
ereport(LOG,
(errmsg("autovacuum: dropping orphan temp table \"%s.%s.%s\"",
get_database_name(MyDatabaseId),
- get_namespace_name(classForm->relnamespace),
- NameStr(classForm->relname))));
+ nspname, NameStr(classForm->relname))));
object.classId = RelationRelationId;
object.objectId = relid;
signature.asc
Description: PGP signature
