From 943576bddcb52971041d9f5f806789921fa107ee Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 13 Aug 2018 11:49:12 +0200 Subject: [PATCH] Make autovacuum more aggressive to remove orphaned temp tables Commit dafa084, added in 10, made the removal of temporary orphaned tables more aggressive. This commit makes an extra step into the aggressiveness by adding a flag in each backend's MyProc which tracks down any temporary namespace currently in use. The flag is set when the namespace gets created and can be reset if the temporary namespace has been created in a transaction or sub-transaction which is aborted. The flag value assignment is assumed to be atomic, so this can be done in a lock-less fashion like other flags already present in PGPROC like databaseId or backendId, still the fact that the temporary namespace and table created are still locked until the transaction creating those commits acts as a barrier for other backends. This new flag gets used by autovacuum to discard more aggressively orphaned tables by additionally checking for the database a backend is connected to as well as its temporary namespace in-use, removing orphaned temporary relations even if a backend reuses the same slot as one which created temporary relations in a past session. The base idea of this patch comes from Robert Haas, has been written in its first version by Tsunakawa Takayuki, then heavily reviewed by me. Author: Tsunakawa Takayuki Reviewed-by: Michael Paquier, Kyotaro Horiguchi, Andres Freund Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8A4DC6@G01JPEXMBYT05 Backpatch: 11-, as PGPROC gains a new flag and we don't want silent ABI breakages on already released versions. --- src/backend/access/transam/twophase.c | 1 + src/backend/catalog/namespace.c | 70 ++++++++++++++++++++++++++- src/backend/postmaster/autovacuum.c | 20 +++----- src/backend/storage/lmgr/proc.c | 2 + src/include/catalog/namespace.h | 1 + src/include/storage/proc.h | 3 ++ 6 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index ed3e36c0c2..349d94a5e6 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -471,6 +471,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->backendId = InvalidBackendId; proc->databaseId = databaseid; proc->roleId = owner; + proc->tempNamespaceId = InvalidOid; proc->isBackgroundWorker = false; proc->lwWaiting = false; proc->lwWaitMode = 0; diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 0f67a122ed..3971346e73 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -47,7 +47,7 @@ #include "parser/parse_func.h" #include "storage/ipc.h" #include "storage/lmgr.h" -#include "storage/sinval.h" +#include "storage/sinvaladt.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/catcache.h" @@ -3204,6 +3204,46 @@ isOtherTempNamespace(Oid namespaceId) return isAnyTempNamespace(namespaceId); } +/* + * isTempNamespaceInUse - is the given namespace owned and actively used + * by a backend? + * + * Note: this can be used while scanning relations in pg_class to detect + * orphaned temporary tables or namespaces with a backend connected to a + * given database. The result may be out of date quickly, so the caller + * must be careful how to handle this information. + */ +bool +isTempNamespaceInUse(Oid namespaceId) +{ + PGPROC *proc; + int backendId; + + Assert(OidIsValid(MyDatabaseId)); + + backendId = GetTempNamespaceBackendId(namespaceId); + + if (backendId == InvalidBackendId || + backendId == MyBackendId) + return false; + + /* Is the backend alive? */ + proc = BackendIdGetProc(backendId); + if (proc == NULL) + return false; + + /* Is the backend connected to the same database we are looking at? */ + if (proc->databaseId != MyDatabaseId) + return false; + + /* Does the backend own the temporary namespace? */ + if (proc->tempNamespaceId != namespaceId) + return false; + + /* all good to go */ + return true; +} + /* * GetTempNamespaceBackendId - if the given namespace is a temporary-table * namespace (either my own, or another backend's), return the BackendId @@ -3893,6 +3933,16 @@ InitTempTableNamespace(void) myTempNamespace = namespaceId; myTempToastNamespace = toastspaceId; + /* + * Mark MyProc as owning this namespace which other processes can use to + * decide if a temporary namespace is in use or not. We assume that + * assignment of namespaceId is an atomic operation. Even if it is not, + * the temporary relation which resulted in the creation of this temporary + * namespace is still locked until the current transaction commits, so it + * would not be accessible yet, acting as a barrier. + */ + MyProc->tempNamespaceId = namespaceId; + /* It should not be done already. */ AssertState(myTempNamespaceSubID == InvalidSubTransactionId); myTempNamespaceSubID = GetCurrentSubTransactionId(); @@ -3923,6 +3973,15 @@ AtEOXact_Namespace(bool isCommit, bool parallel) myTempNamespace = InvalidOid; myTempToastNamespace = InvalidOid; baseSearchPathValid = false; /* need to rebuild list */ + + /* + * Reset the temporary namespace flag in MyProc. We assume that + * this operation is atomic. Even if it is not, the temporary + * table which created this namespace is still locked until this + * transaction aborts so it would not be visible yet, acting as a + * barrier. + */ + MyProc->tempNamespaceId = InvalidOid; } myTempNamespaceSubID = InvalidSubTransactionId; } @@ -3975,6 +4034,15 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid, myTempNamespace = InvalidOid; myTempToastNamespace = InvalidOid; baseSearchPathValid = false; /* need to rebuild list */ + + /* + * Reset the temporary namespace flag in MyProc. We assume that + * this operation is atomic. Even if it is not, the temporary + * table which created this namespace is still locked until this + * transaction aborts so it would not be visible yet, acting as a + * barrier. + */ + MyProc->tempNamespaceId = InvalidOid; } } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 02e6d8131e..b9e5b96877 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2084,14 +2084,11 @@ do_autovacuum(void) */ if (classForm->relpersistence == RELPERSISTENCE_TEMP) { - int backendID; - - backendID = GetTempNamespaceBackendId(classForm->relnamespace); - - /* We just ignore it if the owning backend is still active */ - if (backendID != InvalidBackendId && - (backendID == MyBackendId || - BackendIdGetProc(backendID) == NULL)) + /* + * We just ignore it if the owning backend is still active and + * using the temporary schema. + */ + if (!isTempNamespaceInUse(classForm->relnamespace)) { /* * The table seems to be orphaned -- although it might be that @@ -2219,7 +2216,6 @@ do_autovacuum(void) { Oid relid = lfirst_oid(cell); Form_pg_class classForm; - int backendID; ObjectAddress object; /* @@ -2261,10 +2257,8 @@ do_autovacuum(void) UnlockRelationOid(relid, AccessExclusiveLock); continue; } - backendID = GetTempNamespaceBackendId(classForm->relnamespace); - if (!(backendID != InvalidBackendId && - (backendID == MyBackendId || - BackendIdGetProc(backendID) == NULL))) + + if (isTempNamespaceInUse(classForm->relnamespace)) { UnlockRelationOid(relid, AccessExclusiveLock); continue; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 6f30e082b2..6f9aaa52fa 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -371,6 +371,7 @@ InitProcess(void) MyProc->backendId = InvalidBackendId; MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; + MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyPgXact->delayChkpt = false; MyPgXact->vacuumFlags = 0; @@ -552,6 +553,7 @@ InitAuxiliaryProcess(void) MyProc->backendId = InvalidBackendId; MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; + MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyPgXact->delayChkpt = false; MyPgXact->vacuumFlags = 0; diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 7991de5e21..0e202372d5 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -137,6 +137,7 @@ extern bool isTempToastNamespace(Oid namespaceId); extern bool isTempOrTempToastNamespace(Oid namespaceId); extern bool isAnyTempNamespace(Oid namespaceId); extern bool isOtherTempNamespace(Oid namespaceId); +extern bool isTempNamespaceInUse(Oid namespaceId); extern int GetTempNamespaceBackendId(Oid namespaceId); extern Oid GetTempToastNamespace(void); extern void GetTempNamespaceState(Oid *tempNamespaceId, diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 5c19a61dcf..cb613c8076 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -114,6 +114,9 @@ struct PGPROC Oid databaseId; /* OID of database this backend is using */ Oid roleId; /* OID of role using this backend */ + Oid tempNamespaceId; /* OID of temp schema this backend is + * using */ + bool isBackgroundWorker; /* true if background worker. */ /*