Re: Temporary tables prevent autovacuum, leading to XID wraparound

On 2018-08-09 18:50:47 +0200, Michael Paquier wrote:
> On Thu, Aug 09, 2018 at 02:29:54AM -0700, Andres Freund wrote:
> +   /*
> +    * 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 no visible temporary relations associated to it and the
> +    * temporary namespace creation is not committed yet, so none of its
> +    * contents should be seen yet if scanning pg_class or pg_namespace.
> +    */

> I actually have tried to mention what you are willing to see in the
> comments with the last sentence.  So that is awkward :)

I don't know what you're trying to say with this.

> I would propose to reword the last sentence of the patch as follows
> then:
> "Even if it is not atomic, the temporary relation which resulted in the
> creation of this temporary namespace is still locked until the current
> transaction commits, so it would not be accessible yet."
> 
> When resetting the value on abort I have that:
> +   /*
> +    * 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.
> +    */
> 
> Hence I would propose the following wording for this part:
> "Reset the temporary namespace flag in MyProc.  We assume that this
> operation is atomic, however it would be fine even if not atomic as the
> temporary table which created this namespace is still locked until this
> transaction aborts so it would not be visible yet."

I don't think that comment, nor the comment that you ended up
committing:
+
+           /*
+            * Reset the temporary namespace flag in MyProc.  We assume that
+            * this operation is atomic.  Even if it is not, the temporary
+            * table which created this namespace is still locked until this
+            * transaction aborts so it would not be visible yet, acting as a
+            * barrier.
+            */

is actually correct. *Holding* a lock isn't a memory barrier. Acquring
or releasing one is.

Greetings,

Andres Freund