Re: CREATE INDEX CONCURRENTLY on partitioned index

From: Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>
To: Ilya Gladyshev <ilya(dot)v(dot)gladyshev(at)gmail(dot)com>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, 李杰(慎追) <adger(dot)lj(at)alibaba-inc(dot)com>, 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: CREATE INDEX CONCURRENTLY on partitioned index
Date: 2024-05-24 09:04:33
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Ilya Gladyshev писал(а) 2024-05-24 00:14:
> Hi,


> I think it's well worth the effort to revive the patch, so I rebased it
> on master, updated it and will return it back to the commitfest.
> Alexander, Justin feel free to add yourselves as authors
> On 29.01.2024 12:43, Alexander Pyhalov wrote:
>> Hi.
>> I've rebased patch on master and it'seems to me there's one more issue
>> -
>> when we call DefineIndexConcurrentInternal() in partitioned case, it
>> waits for transactions, locking tableId, not tabrelid - heaprelid
>> LockRelId is constructed for parent index relation, not for child
>> index relation.
>> Attaching fixed version.
>> Also I'm not sure what to do with locking of child relations. If we
>> don't do anything, you can drop one of the partitioned table childs
>> while CIC is in progress, and get error
>> ERROR:  cache lookup failed for index 16399
> I agree that we need to do something about it, in particular, I think
> we should lock all the partitions inside the transaction that builds
> the catalog entries. Fixed this in the new version.
>> If you try to lock all child tables in CIC session, you'll get
>> deadlocks.
> Do you mean the deadlock between the transaction that drops a partition
> and the transaction doing CIC? I think this is unavoidable and can be
> reproduced even without partitioning.

Yes, it seems we trade this error for possible deadlock between
transaction, dropping a partition, and CIC.

> Also not sure why a list of children relation was obtained with
> ShareLock that CIC is supposed to avoid not to block writes, changed
> that to ShareUpdateExclusive.

I expect that it wasn't an issue due to the fact that it's held for a
brief period until DefineIndexConcurrentInternal() commits for the first
time. But it seems, it's more correct to use ShareUpdateExclusive lock

Also I'd like to note that in new patch version there's a strange
wording in documentation:

"This can be very convenient as not only will all existing partitions be
indexed, but any future partitions will be as well.
<command>CREATE INDEX ... CONCURRENTLY</command> can incur long lock
on huge partitioned tables, to avoid that you can
use <command>CREATE INDEX ON ONLY</command> the partitioned table,
creates the new index marked as invalid, preventing automatic
to existing partitions."

All the point of CIC is to avoid long lock times. So it seems this
paragraph should be rewritten in the following way:

"To avoid long lock times, you can use CREATE INDEX CONCURRENTLY or
CREATE INDEX ON ONLY</command> the partitioned table..."

Best regards,
Alexander Pyhalov,
Postgres Professional

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Anthonin Bonnefoy 2024-05-24 09:35:10 Possible incorrect row estimation for Gather paths
Previous Message Ashutosh Bapat 2024-05-24 08:54:52 Re: Addressing SECURITY DEFINER Function Vulnerabilities in PostgreSQL Extensions