On Wed, Nov 18, 2020 at 06:32:51AM +0000, Junfeng Yang wrote: > A path is attached co auther by Ashwin Agrawal, the solution is to > fetch the pg_database tuple from disk instead of system cache if > needed.
Yeah, we had better fix and I guess backpatch something here. That's annoying. +DROP DATABASE IF EXISTS vacuum_freeze_test; +NOTICE: database "vacuum_freeze_test" does not exist, skipping +CREATE DATABASE vacuum_freeze_test; This test is costly. Extracting that into a TAP test would be more adapted, still I am not really convinced that this is a case worth spending extra cycles on. + tuple = systable_getnext(sscan); + if (HeapTupleIsValid(tuple)) + tuple = heap_copytuple(tuple); That's an unsafe pattern as the tuple scanned could finish by being invalid. One thing that strikes me as unwise is that we could run into similar problems with vac_update_relstats() in the future, and there have been recent talks about having more toast tables like for pg_class. That is not worth caring about on stable branches because it is not an active problem there, but we could do something better on HEAD. + /* Get the pg_database tuple to scribble on */ + cached_tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + cached_dbform = (Form_pg_database) GETSTRUCT(cached_tuple); Er, shouldn't we *not* use the system cache at all here then? Or am I missing something obvious? Please see the attached, that is more simpleg. -- Michael
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 395e75f768..6cab35126b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1361,6 +1361,7 @@ vac_update_datfrozenxid(void) MultiXactId lastSaneMinMulti; bool bogus = false; bool dirty = false; + ScanKeyData key[1]; /* * Restrict this task to one backend per database. This avoids race @@ -1479,10 +1480,24 @@ vac_update_datfrozenxid(void) /* Now fetch the pg_database tuple we need to update. */ relation = table_open(DatabaseRelationId, RowExclusiveLock); - /* Fetch a copy of the tuple to scribble on */ - tuple = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + /* + * Get the pg_database tuple to scribble on, and do not rely on + * the syscache to avoid problem with toasted values. + */ + ScanKeyInit(&key[0], + Anum_pg_database_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(MyDatabaseId)); + + scan = systable_beginscan(relation, DatabaseOidIndexId, true, + NULL, 1, key); + tuple = systable_getnext(scan); + tuple = heap_copytuple(tuple); + systable_endscan(scan); + if (!HeapTupleIsValid(tuple)) elog(ERROR, "could not find tuple for database %u", MyDatabaseId); + dbform = (Form_pg_database) GETSTRUCT(tuple); /*
signature.asc
Description: PGP signature
