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
