From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: removal of dangling temp tables
Date: Wed, 26 Dec 2018 20:51:56 -0300
Message-ID: <20181226235156.sqeoyp6gbfavqolp@alvherre.pgsql> (raw)
In-Reply-To: <0A3221C70F24FB45833433255569204D1FB5DC66@G01JPEXMBYT05>
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
Attachments:autovac-temp-REL9_4_STABLE.patch (text/x-diff, 1.9K) downloaddiff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.cindex 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.cindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, tsunakawa.takay@jp.fujitsu.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: removal of dangling temp tables
In-Reply-To: <20181226235156.sqeoyp6gbfavqolp@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox