On 2018-Dec-26, Tsunakawa, Takayuki wrote:
> From: Alvaro Herrera [mailto:[email protected]]
> > The more aggressive action is to backpatch 943576bddcb5 ("Make autovacuum
> > more aggressive to remove orphaned temp tables") which is currently only
> > in pg11. We would put the new PGPROC member at the end of the struct, to
> > avoid ABI incompatibilities, but it'd bring trouble for extensions that
> > put PGPROC in arrays. I checked the code of some known extensions; found
> > that pglogical uses PGPROC, but only pointers to it, so it wouldn't be
> > damaged by the proposed change AFAICS.
>
> +1
> I think this is a bug from a user's perspective that garbage is left.
> I want to believe that fixing bugs of PostgreSQL itself are
> prioritized over the ABI compatibility for extensions, if we have to
> choose one of them.
Having been victim of ABI incompatibility myself, I loathe giving too
much priority to other issues that can be resolved in other ways, so I
don't necessarily support your view on bugs.
That said, I think in this case it shouldn't be a problem, so I'm going
to work on that next.
I haven't got around to creating the abidiff reporting system yet ...
> > Another possibly useful change is to make DISCARD ALL and DISCARD TEMP
> > delete
> > everything in what would be the backend's temp namespace, even if it hasn't
> > been initialized yet. This would cover the case where a connection pooler
> > keeps the connection open for a very long time, which I think is a common
> > case.
>
> That sounds good.
Thanks. I just tested that the attached patch does the intended. (I
named it "autovac" but obviously the DISCARD part is not about
autovacuum). I intend to push this patch first, and later backpatch the
other one.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 89df585b87..d6f3fd5436 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3835,12 +3835,31 @@ RemoveTempRelationsCallback(int code, Datum arg)
/*
* Remove all temp tables from the temporary namespace.
+ *
+ * If we haven't set up one yet, but one exists from a previous crashed
+ * backend, clean that one; but only do this once in a session's life.
*/
void
ResetTempTableNamespace(void)
{
+ static bool TempNamespaceCleaned = false;
+
if (OidIsValid(myTempNamespace))
RemoveTempRelations(myTempNamespace);
+ else if (MyBackendId != InvalidBackendId && !RecoveryInProgress() &&
+ !TempNamespaceCleaned)
+ {
+ char namespaceName[NAMEDATALEN];
+ Oid namespaceId;
+
+ snprintf(namespaceName, sizeof(namespaceName), "pg_temp_%d",
+ MyBackendId);
+ namespaceId = get_namespace_oid(namespaceName, true);
+ if (OidIsValid(namespaceId))
+ RemoveTempRelations(namespaceId);
+ }
+
+ TempNamespaceCleaned = true;
}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c18285d690..62a06add56 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2108,11 +2108,18 @@ do_autovacuum(void)
if (classForm->relpersistence == RELPERSISTENCE_TEMP)
{
int backendID;
+ PGPROC *proc;
backendID = GetTempNamespaceBackendId(classForm->relnamespace);
- /* We just ignore it if the owning backend is still active */
- if (backendID == MyBackendId || BackendIdGetProc(backendID) == NULL)
+ /*
+ * We just ignore it if the owning backend is still active in the
+ * same database.
+ */
+ if (backendID != InvalidBackendId &&
+ (backendID == MyBackendId ||
+ (proc = BackendIdGetProc(backendID)) == NULL ||
+ proc->databaseId != MyDatabaseId))
{
/*
* We found an orphan temp table (which was probably left
