Re: Support for REINDEX CONCURRENTLY

On Thu, Feb 14, 2013 at 4:08 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> Hi all,
>
> Please find attached a new set of 3 patches for REINDEX CONCURRENTLY (v11).
> - 20130214_1_remove_reltoastidxid.patch
> - 20130214_2_reindex_concurrently_v11.patch
> - 20130214_3_reindex_concurrently_docs_v11.patch
> Patch 1 needs to be applied before patches 2 and 3.
>
> 20130214_1_remove_reltoastidxid.patch is the patch removing reltoastidxid
> (approach mentioned by Tom) to allow server to manipulate multiple indexes
> of toast relations. Catalog views, system functions and pg_upgrade have been
> updated in consequence by replacing reltoastidxid use by a join on
> pg_index/pg_class. All the functions of tuptoaster.c now use
> RelationGetIndexList to fetch the list of indexes on which depend a given
> toast relation. There are no warnings, regressions are passing (here only an
> update of rules.out and oidjoins has been necessary).
> 20130214_2_reindex_concurrently_v11.patch depends on patch 1. It includes
> the feature with all the fixes requested by Andres in his previous reviews.
> Regressions are passing and I haven't seen any warnings. in this patch
> concurrent rebuild of toast indexes is fully supported thanks to patch 1.
> The kludge used in previous version to change reltoastidxid when swapping
> indexes is not needed anymore, making swap code far cleaner.
> 20130214_3_reindex_concurrently_docs_v11.patch includes the documentation of
> REINDEX CONCURRENTLY. This might need some reshuffling with what is written
> for CREATE INDEX CONCURRENTLY.
>
> I am now pretty happy with the way implementation is done, so I think that
> the basic implementation architecture does not need to be changed.
> Andres, I think that only a single round of review would be necessary now
> before setting this patch as ready for committer. Thoughts?
>
> Comments, as well as reviews are welcome.

When I compiled the HEAD with the patches, I got the following warnings.

index.c:1273: warning: unused variable 'parentRel'
execUtils.c:1199: warning: 'return' with no value, in function
returning non-void

When I ran REINDEX CONCURRENTLY for the same index from two different
sessions, I got the deadlock. The error log is:

ERROR: deadlock detected
DETAIL: Process 37121 waits for ShareLock on virtual transaction
2/196; blocked by process 36413.
Process 36413 waits for ShareUpdateExclusiveLock on relation 16457 of
database 12293; blocked by process 37121.
Process 37121: REINDEX TABLE CONCURRENTLY pgbench_accounts;
Process 36413: REINDEX TABLE CONCURRENTLY pgbench_accounts;
HINT: See server log for query details.
STATEMENT: REINDEX TABLE CONCURRENTLY pgbench_accounts;

And, after the REINDEX CONCURRENTLY that survived the deadlock finished,
I found that new index with another name was created. It was NOT marked as
INVALID. Are these behaviors intentional?

=# \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)

Regards,

--
Fujii Masao