Date: Wed, 26 Dec 2018 20:51:56 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
Cc: Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>,
	Robert Haas <robertmhaas@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: removal of dangling temp tables
Message-ID: <20181226235156.sqeoyp6gbfavqolp@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="d7434mtzlytuzog5"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0A3221C70F24FB45833433255569204D1FB5DC66@G01JPEXMBYT05>
User-Agent: NeoMutt/20180716
Precedence: bulk


--d7434mtzlytuzog5
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2018-Dec-26, Tsunakawa, Takayuki wrote:

> From: Alvaro Herrera [mailto:alvherre@2ndquadrant.com]
> > 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

--d7434mtzlytuzog5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="autovac-temp-REL9_4_STABLE.patch"

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

--d7434mtzlytuzog5--