Hi, On 2019-06-08 08:59:37 +0900, Michael Paquier wrote: > On Fri, Jun 07, 2019 at 03:58:43PM -0700, Andres Freund wrote: > > Do we need to move the orphan temp cleanup code into database vacuums or > > such? > > When entering into the vacuum() code path for an autovacuum, only one > relation at a time is processed, and we have prior that extra > processing related to toast relations when selecting the relations to > work on, or potentially delete orphaned temp tables. For a manual > vacuum, we finish by deciding which relation to process in > get_all_vacuum_rels(), so the localized processing is a bit different > than what's done in do_autovacuum() when scanning pg_class for > relations.
Yea, I know. I didn't mean that we should only handle orphan cleanup only within database wide vacuums, just *also* there. ISTM that'd mean that at least some of the code ought to be in vacuum.c, and then also called by autovacuum.c. > Technically, I think that it would work to give up on the gathering of > the orphaned OIDs in a gathering and let them be gathered in the list > of items to vacuum, and then put the deletion logic down to > vacuum_rel(). I don't think it makes much sense to go there. The API would probably look pretty bad. I was more thinking that we'd move the check for orphaned-ness into a separate function (maybe IsOrphanedRelation()), and move the code to drop orphan relations into a separate function (maybe DropOrphanRelations()). That'd limit the amount of code duplication for doing this both in autovacuum and all-database vacuums quite considerably. A more aggressive approach would be to teach vac_update_datfrozenxid() to ignore orphaned temp tables - perhaps even by heap_inplace'ing an orphaned table's relfrozenxid/relminmxid with InvalidTransactionId. We'd not want to do that in do_autovacuum() - otherwise the schema won't get cleaned up, but for database widevacuums that seems like it could be good approach. Random observation while re-reading this code: Having do_autovacuum() and ExecVacuum() both go through vacuum() seems like it adds too much complexity to be worth it. Like half of the file is only concerned with checks related to that. Greetings, Andres Freund