Re: Temporary tables prevent autovacuum, leading to XID wraparound - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Temporary tables prevent autovacuum, leading to XID wraparound
Date
Msg-id 20180813165318.w2uizfr4yrfjwfbm@alvherre.pgsql
Whole thread Raw
In response to Re: Temporary tables prevent autovacuum, leading to XID wraparound  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Temporary tables prevent autovacuum, leading to XID wraparound
List pgsql-hackers
From 1e9ba9fa9b172bda1ea54b1f3be1b930973ff45f Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.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


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: ATTACH/DETACH PARTITION CONCURRENTLY
Next
From: Michael Paquier
Date:
Subject: Re: Temporary tables prevent autovacuum, leading to XID wraparound