On Tue, 11 Dec 2018 at 15:43, Michael Paquier <mich...@paquier.xyz> wrote: > + parentrel = heap_openrv(parent, AccessExclusiveLock); > So, in order to determine which tablespace should be used here, an > exclusive lock is taken on the parent because its partition descriptor > gets updated by the addition of the new partition. This process is > actually done already in MergeAttributes() as well, but it won't get > triggered if a tablespace is defined directly in the CREATE TABLE > statement. I think that we should add a comment to explain the > dependency between both, as well as why an exclusive lock is needed, so > something among those lines perhaps? Here is an idea: > + /* > + * Grab an exclusive lock on the parent because its partition > + * descriptor will be changed by the addition of the new partition. > + * The same lock level is taken as when merging attributes below > + * in MergeAttributes() to protect from lock upgrade deadlocks. > + */ > > The position where the tablespace is chosen is definitely the good one. > > What do you think?
I think a comment in that location is a good idea. There's work being done to reduce the lock level required for attaching a partition so a comment here will help show that it's okay to reduce the lock level for fetching the tablespace too. I've attached an updated patch that includes the new comment. I didn't use your proposed words though. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
v4-0001-Allow-newly-created-partitions-to-inherit-their-p.patch
Description: Binary data
