Re: Temporary tables prevent autovacuum, leading to XID wraparound

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "tgl(at)sss(dot)pgh(dot)pa(dot)us" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Temporary tables prevent autovacuum, leading to XID wraparound
Date: 2018-08-13 16:53:18
Message-ID: 20180813165318.w2uizfr4yrfjwfbm@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


From 1e9ba9fa9b172bda1ea54b1f3be1b930973ff45f Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael(at)paquier(dot)xyz>
Date: Wed, 8 Aug 2018 19:45:31 +0200
Subject: [PATCH] Make autovacuum more aggressive to remove orphaned temp
tables

I was here to complain about this piece:

> @@ -3975,6 +4033,15 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
> myTempNamespace = InvalidOid;
> myTempToastNamespace = InvalidOid;
> baseSearchPathValid = false; /* need to rebuild list */
> +
> + /*
> + * Reset the temporary namespace flag in MyProc. The creation of
> + * the temporary namespace has failed for some reason and should
> + * not be seen by other processes as it has not been committed
> + * yet, hence this would be fine even if not atomic, still we
> + * assume that it is an atomic assignment.
> + */
> + MyProc->tempNamespaceId = InvalidOid;
> }
> }

But after looking carefully, I realized that this only runs if the
subtransaction being aborted is the same that set up the temp namespace
(or one of its children) in the first place; therefore it's correct to
tear it down unconditionally. I was afraid of this clobbering a temp
namespace that had been set up by some other branch of the transaction
tree. However, that's not a concern because we compare the
subTransactionId (and update the value when propagating to parent
subxact.)

I do share Andres' concerns on the wording the comment. I would say
something like

/*
* Reset the temporary namespace flag in MyProc. We assume this to be
* an atomic assignment.
*
* Because this subtransaction is rolling back, the pg_namespace
* row is not visible to anyone else anyway, but that doesn't matter:
* it's not a problem if objects contained in this namespace are removed
* concurrently.
*/

The fact of assignment being atomic and the fact of the pg_namespace row
being visible are separately important. You care about it being atomic
because it means you must not have someone read "16" (0x10) when you
were partway removing the value "65552" (0x10010), thus causing that
someone removing namespace 16. And you care about the visibility of the
pg_namespace row because of whether you're worried about a third party
removing the tables from that namespace or not: since the subxact is
aborting, you are not.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-08-13 17:03:59 Re: Temporary tables prevent autovacuum, leading to XID wraparound
Previous Message Robert Haas 2018-08-13 16:00:34 Re: ATTACH/DETACH PARTITION CONCURRENTLY