Re: Temporary tables prevent autovacuum, leading to XID wraparound

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