Re: Support for REINDEX CONCURRENTLY

Hi all,

I am resending the patches after Fujii-san noticed a bug allowing to
even drop valid toast indexes with the latest code... While looking at
that, I found a couple of other bugs:
- two bugs, now fixed, with the code path added in tablecmds.c to
allow the manual drop of invalid toast indexes:
-- Even a user having no permission on the parent toast table could
drop an invalid toast index
-- A lock on the parent toast relation was not taken as it is the case
for all the indexes dropped with DROP INDEX
- Trying to reindex concurrently a mapped catalog leads to an error.
As they have no relfilenode, I think it makes sense to block reindex
concurrently in this case, so I modified the core patch in this sense.

Regards,

On Fri, Jul 5, 2013 at 1:47 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Hi all,
>
> Please find attached the patch using MVCC catalogs. I have split the
> previous core patch into 3 pieces to facilitate the review and reduce
> the size of the main patch as the previous core patch contained a lot
> of code refactoring.
> 0) 20130705_0_procarray.patch, this patch adds a set of generic APIs
> in procarray.c that can be used to wait for snapshots older than a
> given xmin, or to wait for some virtual locks. This code has been
> taken from CREATE/DROP INDEX CONCURRENTLY, and I think that this set
> of APIs could be used for the implementation os other concurrent DDLs.
> 1) 20130705_1_index_conc_struct.patch, this patch refactors a bit
> CREATE/DROP INDEX CONCURRENTLY to create 2 generic APIs for the build
> of a concurrent index, and the step where it is set as dead.
> 2) 20130705_2_reindex_concurrently_v28.patch, with the core feature. I
> have added some stuff here:
> - isolation tests, (perhaps it would be better to make the DML actions
> last longer in those tests?)
> - reduction of the lock used at swap phase from AccessExclusiveLock to
> ShareUpdateExclusiveLock, and added a wait before commit of swap phase
> for old snapshots at the end of swap phase to be sure that no
> transactions will use the old relfilenode that has been swapped after
> commit
> - doc update
> - simplified some APIs, like the removal of index_concurrent_clear_valid
> - fixed a bug where it was not possible to reindex concurrently a toast relation
> Patch 1 depends on 0, Patch 2 depends on 1 and 0. Patch 0 can be
> applied directly on master.
>
> The two first patches are pretty simple, patch 0 could even be quickly
> reviewed and approved to provide some more infrastructure that could
> be possibly used by some other patches around, like REFRESH
> CONCURRENTLY...
>
> I have also done some tests with the set of patches:
> - Manual testing, and checked that process went smoothly by taking
> some manual checkpoints during each phase of REINDEX CONCURRENTLY
> - Ran make check for regression and isolation tests
> - Ran make installcheck, and then REINDEX DATABASE CONCURRENTLY on the
> database regression that remained on server
>
> Regards,
> --
> Michael



--
Michael