Re: documentation about explicit locking

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: documentation about explicit locking
Date: 2018-07-19 04:17:14
Message-ID: 85dc6464-eb59-ba59-75d3-09b292fa853d@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

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.

Thanks,
Amit

Attachment Content-Type Size
create-collation-locking-doc.patch text/plain 1.2 KB
create-collation-race-fix.patch text/plain 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2018-07-19 04:25:03 Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.
Previous Message Kyotaro HORIGUCHI 2018-07-19 04:04:12 Re: patch to allow disable of WAL recycling