Re: Support for REINDEX CONCURRENTLY

On Thu, Feb 14, 2013 at 4:08 PM, Michael Paquier
<michael.paquier@gmail.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:
REINDEXTABLE 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 relationsSchema |           Name            | Type  |  Owner
 |      Table
 
--------+---------------------------+-------+----------+------------------public | pgbench_accounts_pkey     | index |
postgres| pgbench_accountspublic | pgbench_accounts_pkey_cct | index | postgres | pgbench_accounts
 
(2 rows)

Regards,

-- 
Fujii Masao