Re: Temporary tables prevent autovacuum, leading to XID wraparound

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, robertmhaas(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Temporary tables prevent autovacuum, leading to XID wraparound
Date: 2018-08-08 08:33:06
Message-ID: 20180808083306.GA10158@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 31, 2018 at 01:39:07PM +0900, Kyotaro HORIGUCHI wrote:
> "int backendID" is left in autovacuum.c as an unused variable.

Fixed.

> "Even if this is *not* an atomic operation" ?

Yeah, I am reworking this comment a bit more to map with the other
PGPROC fields.

> + * Mark the proc entry as owning this namespace which autovacuum uses to
> + * decide if a temporary file entry can be marked as orphaned or not. Even
> + * if this is an atomic operation, this can be safely done lock-less as no
> + * temporary relations associated to it can be seen yet by autovacuum when
> + * scanning pg_class.
> + */
> + MyProc->tempNamespaceId = namespaceId;
>
> The comment looks wrong. After a crash having temp tables,
> pg_class has orphaned temp relations and the namespace can be of
> reconnected backends especially for low-numbered backends

I don't quite understand your comment. InitProcess() initializes the
field, so if a backend reconnects and uses the same proc slot as a
backend which had a temporary namespace, then you would discard orphaned
tables. Anyway, I have reworked it as such:
+ /*
+ * Mark MyProc as owning this namespace which other processes can use to
+ * decide if a temporary namespace is in use or not. We assume that
+ * assignment of namespaceId is an atomic operation. Even if it is not,
+ * there is as no temporary relations associated to it and the temporary
+ * namespace creation is not committed yet, so none of its contents can
+ * be seen yet if scanning pg_class or pg_namespace.
+ */

As new minor versions have been tagged, I am planning to commit this
patch soon with some tweaks for the comments. As this introduces a new
field to PGPROC, so back-patching the thing as-is would cause an ABI
breakage. Are folks here fine with the new field added to the bottom of
the structure for the backpatched versions, including v11? I have found
about commit 13752743 which has also added a new field called
isBackgroundWorker in the middle of PGPROC in a released branch, which
looks to me like an ABI breakage...
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2018-08-08 08:41:27 Re: Temporary tables prevent autovacuum, leading to XID wraparound
Previous Message Kyotaro HORIGUCHI 2018-08-08 08:30:50 Re: Problem while updating a foreign table pointing to a partitioned table on foreign server