Re: Support for REINDEX CONCURRENTLY

On Sat, Feb 23, 2013 at 2:14 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Thu, Feb 21, 2013 at 11:55 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > A ShareUpdateExclusiveLock is taken on index or table that is going to be
> > rebuilt just before calling ReindexRelationConcurrently. So the solution
> I
> > have here is to make REINDEX CONCURRENTLY fail for session 2. REINDEX
> > CONCURRENTLY is made to allow a table to run DML in parallel to the
> > operation so it doesn't look strange to me to make session 2 fail if
> REINDEX
> > CONCURRENTLY is done in parallel on the same relation.
>
> Thanks for updating the patch!
>
> With updated patch, REINDEX CONCURRENTLY seems to fail even when
> SharedUpdateExclusiveLock is taken by the command other than REINDEX
> CONCURRENTLY, for example, VACUUM. Is this intentional? This behavior
> should be avoided. Otherwise, users might need to disable autovacuum
> whenever they run REINDEX CONCURRENTLY.
>
> With updated patch, unfortunately, I got the similar deadlock error when I
> ran REINDEX CONCURRENTLY in session1 and ANALYZE in session2.
>
Such deadlocks are also possible when running manual VACUUM with CREATE
INDEX CONCURRENTLY. This is because ANALYZE can be included in a
transaction that might do arbitrary operations on the parent table (see
comments in indexcmds.c) between the index build and validation. So the
only problem I see here is that the concurrent index is marked as VALID in
the transaction when a deadlock occurs and REINDEX CONCURRENTLY fails,
right?

> ERROR: deadlock detected
> DETAIL: Process 70551 waits for ShareLock on virtual transaction
> 3/745; blocked by process 70652.
> Process 70652 waits for ShareUpdateExclusiveLock on relation 17460
> of
> database 12293; blocked by process 70551.
> Process 70551: REINDEX TABLE CONCURRENTLY pgbench_accounts;
> Process 70652: ANALYZE pgbench_accounts;
> HINT: See server log for query details.
> STATEMENT: REINDEX TABLE CONCURRENTLY pgbench_accounts;
>
> Like original problem that I reported, temporary index created by REINDEX
> CONCURRENTLY was NOT marked as INVALID.
>
> =# \di pgbench_accounts*
> List of relations
> Schema | Name | Type | Owner | Table
> --------+---------------------------+-------+----------+------------------
> public | pgbench_accounts_pkey | index | postgres | pgbench_accounts
> public | pgbench_accounts_pkey_cct | index | postgres | pgbench_accounts
> (2 rows)
>
Btw, ¥di also prints invalid indexes...

OK, so what you want to see is the index being marked as not valid when a
deadlock occurs with REINDEX CONCURRENTLY when an ANALYZE kicks in (btw,
deadlocks are also possible with CREATE INDEX CONCURRENTLY when ANALYZE is
done on a table, in this case the index is marked as not valid). So indeed
there was a bug in my code for v12 and prior as if a deadlock occurred the
concurrent index was marked as valid.

I have been able to fix that with updated patch attached, which removed the
change done in v12 and checks for deadlock at phase 3 before actually
marking the index as valid (opposite operation was done in v11 and below
making the indexes being seen as valid when the deadlock appeared).
So now here is what heppens with a deadlock:
ioltas=# create table aa (a int);
CREATE TABLE
ioltas=# create index aap on aa (a);
CREATE INDEX
ioltas=# reindex index concurrently aap;
ERROR: deadlock detected
DETAIL: Process 32174 waits for ShareLock on virtual transaction 3/2;
blocked by process 32190.
Process 32190 waits for ShareUpdateExclusiveLock on relation 16385 of
database 16384; blocked by process 32174.
HINT: See server log for query details.
And how the relation remains after the deadlock:
ioltas=# \d aa
Table "public.aa"
Column | Type | Modifiers
--------+---------+-----------
a | integer |
Indexes:
"aap" btree (a)
"aap_cct" btree (a) INVALID
ioltas=# \di aa*
List of relations
Schema | Name | Type | Owner | Table
--------+---------+-------+--------+-------
public | aap | index | ioltas | aa
public | aap_cct | index | ioltas | aa
(2 rows)

The potential *problem* (actually that looks more to be a non-problem) is
the case of REINDEX CONCURRENTLY run on a table with multiple indexes.
For example, let's take the case of a table with 2 indexes.
1) Session 1: Run REINDEX CONCURRENTLY on this table.
2) Session 2: Run ANALYZE on this table after 1st index has been validated
but before the 2nd index is validated
3) Session 1: fails due to a deadlock, the table containing 3 valid
indexes, the former 2 indexes and the 1st concurrent one that has been
validated. The 2nd concurrent index is marked as not valid.
This can happen when REINDEX CONCURRENTLY conflicts with the following
commands: CREATE INDEX CONCURRENTLY, another REINDEX CONCURRENTLY and
ANALYZE. Note that the 1st concurrent index is perfectly valid, so user can
still drop the 1st old index after the deadlock.

So, in the case of a single index being rebuilt with REINDEX CONCURRENTLY
there are no problems, but there is a risk of multiplying the number of
indexes on a table when it is used to rebuild multiple indexes at the same
time with REINDEX TABLE CONCURRENTLY, or even REINDEX DATABASE
CONCURRENTLY. I think that this feature can live with that as long as the
user is aware of the risks when doing a REINDEX CONCURRENTLY that rebuilds
more than 1 index at the same time. Comments?
--
Michael