Re: documentation about explicit locking

Hello.

At Thu, 19 Jul 2018 13:17:14 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <85dc6464-eb59-ba59-75d3-09b292fa853d(at)lab(dot)ntt(dot)co(dot)jp>
> On 2018/07/18 18:30, Peter Eisentraut wrote:
> > On 06.07.18 04:00, Amit Langote wrote:
> >> On 2018/07/05 23:02, Robert Haas wrote:
> >>> On Wed, Jul 4, 2018 at 3:09 AM, Amit Langote
> >>> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> >>>> I wonder why we mention on the following page that CREATE COLLATION
> >>>> requires SHARE ROW EXCLUSIVE lock
> >>>>
> >>>> https://www.postgresql.org/docs/devel/static/explicit-locking.html
> >>>>
> >>>> I know that's the lock taken on the pg_collation catalog, but do we need
> >>>> to mention locks taken by a DDL command on the catalogs it affects? All
> >>>> other commands mentioned on the page require to specify the table name
> >>>> that the lock will be taken on.
> >>>
> >>> Yes, that looks odd.
> >>
> >> OK, here is a patch.
> >>
> >> I see that it was one of Peter E's commits that added that, so cc'd him.
> >
> > The reason this is mentioned is that CREATE COLLATION takes a SHARE ROW
> > EXCLUSIVE lock on pg_collation whereas similar CREATE commands only take
> > a ROW EXCLUSIVE lock on their catalogs. (So you can only have one
> > CREATE COLLATION running at a time. The reasons for this are explained
> > in pg_collation.c.) I think mentioning this was requested during patch
> > review.
>
> I see. Although, which lock we take on the system catalog for
> implementing a particular command seems to be an internal detail. What's
> clearly user-visible in this case is that CREATE COLLATION command cannot
> be used simultaneously by concurrent sessions, so it should be pointed out
> in the CREATE COLLATION command's documentation. On a quick check, it
> doesn't seem to be. So, I have updated my patch to also add a line about
> that on CREATE COLLATION page. What do you think?

I'm not Peter but I have a comment on this.

+ Note that only one of the concurrent sessions can run
+ <command>CREATE COLLATION</command> at a time.

The description seems to me to be failing to give clear idea of
what operation causes what behavior. I agree that the description
in the explicit-locking section is out of the place since it is
not a lock on *specified* tables. I'd like to have a description
with the similar level here instead, like this:

Note that CREATE COLLATION takes a SHARE ROW EXLUCSIVE lock on
pg_collation system calatlog, which blocks other concurrent
CREATE COLLATION.

> When playing with this, I observed that a less user-friendly error message
> is emitted if multiple sessions race to create the same collation.
>
> Session 1:
> begin;
> create collation collname (...);
>
> Session 2:
> create collation collname (...);
> <blocks for lock on pg_collation>
>
> Session 1:
> commit;
>
> Session 2:
> ERROR: duplicate key value violates unique constraint
> "pg_collation_name_enc_nsp_index"
> DETAIL: Key (collname, collencoding, collnamespace)=(collname, 6, 2200)
> already exists.
>
> I figured that's because the order in CollationCreate of locking the
> catalog and checking in syscache whether a duplicate exists. I think we
> should check the syscache for duplicate *after* we have locked the
> catalog, as done in the other patch that's attached.

Such cases in other commands usually have a very narrow window
but the said lock widens the window very much in the case:p So +1
from me to this change.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center