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
