From 86acd2bda325b05f78489a038ada51268d5563b4 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 1 Sep 2023 10:04:40 +0900 Subject: [PATCH v2] Fix autovacuum scheduler with dropped databases Blah. Reported-by: Will Mortensen, Jacob Speidel Analyzed-by: Will Mortensen, Jacob Speidel Author: Andres Freund Discussion: https://postgr.es/m/17973-bca1f7d5c14f601e@postgresql.org --- src/backend/utils/activity/pgstat_database.c | 13 +++ src/backend/utils/init/postinit.c | 111 +++++++++---------- 2 files changed, 67 insertions(+), 57 deletions(-) diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c index 7149f22f72..d04426f53f 100644 --- a/src/backend/utils/activity/pgstat_database.c +++ b/src/backend/utils/activity/pgstat_database.c @@ -271,6 +271,13 @@ pgstat_update_dbstats(TimestampTz ts) { PgStat_StatDBEntry *dbentry; + /* + * If not connected to a database yet, don't attribute time to "shared + * state" (InvalidOid is used to track stats for shared relations, etc.). + */ + if (!OidIsValid(MyDatabaseId)) + return; + dbentry = pgstat_prep_database_pending(MyDatabaseId); /* @@ -327,6 +334,12 @@ pgstat_prep_database_pending(Oid dboid) { PgStat_EntryRef *entry_ref; + /* + * This should not report stats on database objects before having + * connected to a database. + */ + Assert(!OidIsValid(dboid) || OidIsValid(MyDatabaseId)); + entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_DATABASE, dboid, InvalidOid, NULL); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index f31b85c013..38c8f378d4 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1006,7 +1006,7 @@ InitPostgres(const char *in_dbname, Oid dboid, */ if (bootstrap) { - MyDatabaseId = Template1DbOid; + dboid = Template1DbOid; MyDatabaseTableSpace = DEFAULTTABLESPACE_OID; } else if (in_dbname != NULL) @@ -1020,32 +1020,9 @@ InitPostgres(const char *in_dbname, Oid dboid, (errcode(ERRCODE_UNDEFINED_DATABASE), errmsg("database \"%s\" does not exist", in_dbname))); dbform = (Form_pg_database) GETSTRUCT(tuple); - MyDatabaseId = dbform->oid; - MyDatabaseTableSpace = dbform->dattablespace; - /* take database name from the caller, just for paranoia */ - strlcpy(dbname, in_dbname, sizeof(dbname)); + dboid = dbform->oid; } - else if (OidIsValid(dboid)) - { - /* caller specified database by OID */ - HeapTuple tuple; - Form_pg_database dbform; - - tuple = GetDatabaseTupleByOid(dboid); - if (!HeapTupleIsValid(tuple)) - ereport(FATAL, - (errcode(ERRCODE_UNDEFINED_DATABASE), - errmsg("database %u does not exist", dboid))); - dbform = (Form_pg_database) GETSTRUCT(tuple); - MyDatabaseId = dbform->oid; - MyDatabaseTableSpace = dbform->dattablespace; - Assert(MyDatabaseId == dboid); - strlcpy(dbname, NameStr(dbform->datname), sizeof(dbname)); - /* pass the database name back to the caller */ - if (out_dbname) - strcpy(out_dbname, dbname); - } - else + else if (!OidIsValid(dboid)) { /* * If this is a background worker not bound to any particular @@ -1083,8 +1060,57 @@ InitPostgres(const char *in_dbname, Oid dboid, * CREATE DATABASE. */ if (!bootstrap) - LockSharedObject(DatabaseRelationId, MyDatabaseId, 0, - RowExclusiveLock); + LockSharedObject(DatabaseRelationId, dboid, 0, RowExclusiveLock); + + /* + * Recheck pg_database to make sure the target database hasn't gone away. + * If there was a concurrent DROP DATABASE, this ensures we will die + * cleanly without creating a mess. + */ + if (!bootstrap) + { + HeapTuple tuple; + Form_pg_database datform; + + tuple = GetDatabaseTupleByOid(dboid); + if (HeapTupleIsValid(tuple)) + datform = (Form_pg_database) GETSTRUCT(tuple); + + if (!HeapTupleIsValid(tuple) || + (in_dbname && namestrcmp(&datform->datname, in_dbname))) + { + if (in_dbname) + ereport(FATAL, + (errcode(ERRCODE_UNDEFINED_DATABASE), + errmsg("database \"%s\" does not exist", in_dbname), + errdetail("It seems to have just been dropped or renamed."))); + else + ereport(FATAL, + (errcode(ERRCODE_UNDEFINED_DATABASE), + errmsg("database %u does not exist", dboid))); + } + + strlcpy(dbname, NameStr(datform->datname), sizeof(dbname)); + + if (database_is_invalid_form(datform)) + { + ereport(FATAL, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot connect to invalid database \"%s\"", dbname), + errhint("Use DROP DATABASE to drop invalid databases.")); + } + + MyDatabaseTableSpace = datform->dattablespace; + /* pass the database name back to the caller */ + if (out_dbname) + strcpy(out_dbname, dbname); + } + + /* + * Now that we rechecked, we are certain to be connected to a database and + * thus can set MyDatabaseId. + */ + MyDatabaseId = dboid; /* * Now we can mark our PGPROC entry with the database ID. @@ -1108,35 +1134,6 @@ InitPostgres(const char *in_dbname, Oid dboid, */ InvalidateCatalogSnapshot(); - /* - * Recheck pg_database to make sure the target database hasn't gone away. - * If there was a concurrent DROP DATABASE, this ensures we will die - * cleanly without creating a mess. - */ - if (!bootstrap) - { - HeapTuple tuple; - Form_pg_database datform; - - tuple = GetDatabaseTuple(dbname); - if (!HeapTupleIsValid(tuple) || - MyDatabaseId != ((Form_pg_database) GETSTRUCT(tuple))->oid || - MyDatabaseTableSpace != ((Form_pg_database) GETSTRUCT(tuple))->dattablespace) - ereport(FATAL, - (errcode(ERRCODE_UNDEFINED_DATABASE), - errmsg("database \"%s\" does not exist", dbname), - errdetail("It seems to have just been dropped or renamed."))); - - datform = (Form_pg_database) GETSTRUCT(tuple); - if (database_is_invalid_form(datform)) - { - ereport(FATAL, - errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("cannot connect to invalid database \"%s\"", dbname), - errhint("Use DROP DATABASE to drop invalid databases.")); - } - } - /* * Now we should be able to access the database directory safely. Verify * it's there and looks reasonable. -- 2.40.1