From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: 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 16:08:34 -0300
Message-ID: <20181226190834.wsk2wzott5yzrjiq@alvherre.pgsql> (raw)
In-Reply-To: <20181216065847.GJ5012@paquier.xyz>
On 2018-Dec-16, Michael Paquier wrote:
> On Sat, Dec 15, 2018 at 09:51:31AM -0500, Tom Lane wrote:
> > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> >> Oh, we already have it! Sorry, I overlooked it. With that, it seems
> >> the patch is fairly simple ... I wonder about the locking implications
> >> in autovacuum, though -- the value is set in backends without acquiring
> >> a lock.
> >
> > I was wondering about that too. But I think it's probably OK. If
> > autovacuum observes that (a) a table is old enough to pose a wraparound
> > hazard and (b) its putatively owning backend hasn't yet set
> > tempNamespaceId, then I think it's safe to conclude that that table is
> > removable, despite the theoretical race condition.
>
> This relies on the fact that the flag gets set by a backend within a
> transaction context, and autovacuum would not see yet temp relations
> associated to it at the moment of the scan of pg_class if the backend
> has not committed yet its namespace creation via the creation of the
> first temp table it uses.
Makes sense, thanks.
I think there are two possible ways forward. The conservative one is to
just apply the attached patch to branches 9.4-10. That will let
autovacuum drop tables when the backend is in another database. It may
not solve the problem for the bunch of users that have only one database
that takes the majority of connections, but I think it's worth doing
nonetheless. I tested the 9.4 instance and it works fine; tables are
deleted as soon as I make the session connection to another database.
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.
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.
--
Á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.2K) download
commit 5aa717be9fe7288cbec043cdea0fb757433f3462[m
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
AuthorDate: Wed Dec 26 15:12:44 2018 -0300
CommitDate: Wed Dec 26 15:12:45 2018 -0300
Improve autovacuum logic about temp tables nearing XID wraparound
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, 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: <20181226190834.wsk2wzott5yzrjiq@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