Re: Support for REINDEX CONCURRENTLY

On Mon, Jan 28, 2013 at 7:39 PM, Andres Freund <andres@2ndquadrant.com>wrote:

> On 2013-01-27 07:54:43 +0900, Michael Paquier wrote:
> I think you're misunderstanding how this part works a bit. We don't
> acquire locks on the table itself, but we get a list of all transactions
> we would conflict with if we were to acquire a lock of a certain
> strength on the table (GetLockConflicts(locktag, mode)). We then wait
> for each transaction in the resulting list via the VirtualXact mechanism
> (VirtualXactLock(*lockholder)).
> It doesn't matter all that waiting happens in the same transaction the
> initial index build is done in as long as we keep the session locks
> preventing other schema modifications. Nobody can go back and see an
> older index list after we've done the above wait once.
>
Don't worry I got it. I just thought that it was necessary to wait for the
locks taken on the parent relation by other backends just *before* building
the index. It seemed more stable.

So the following should be perfectly fine:
>
> StartTransactionCommand();
> BuildListOfIndexes();
> foreach(index in indexes)
>     DefineNewIndex(index);
> CommitTransactionCommand();
>
> StartTransactionCommand();
> foreach(table in tables)
>     GetLockConflicts()
> foreach(conflict in conflicts)
>     VirtualXactLocks()
> CommitTransactionCommand();
>
> foreach(index in indexes)
>     StartTransactionCommand();
>     InitialIndexBuild(index)
>     CommitTransactionCommand();
>
So you're point is simply to wait for all the locks currently taken on each
table in a different transaction only once and for all, independently from
the build and validation phases. Correct?


> It looks that this feature has still too many disadvantages compared to
> the
> > advantages it could bring in the current infrastructure (SnapshotNow
> > problems, what to do with invalid toast indexes, etc.), so I would tend
> to
> > agree with Tom and postpone this feature once infrastructure is more
> > mature, one of the main things being the non-MVCC'ed catalogs.
>
> I think while catalog mvcc snapshots would make this easier, most
> problems, basically all but the switching of relations, are pretty much
> independent from that fact. All the waiting etc, will still be there.
>
> I can see an argument for pushing it to the next CF because its not
> really there yet...
>
Even if we get this patch in a shape that you think is sufficient to make
it reviewable by a committer within a couple of days, there are still many
doubts from many people regarding this feature, so this is going to take
far more time to put it in a shape that would satisfy a vast majority. So
it is honestly wiser to work on that later.

Another argument that would be enough for a rejection of this patch by a
committer is the problem of invalid toast indexes that cannot be removed up
cleanly by an operator. As long as there is not a clean solution for that...
-- 
Michael Paquier
http://michael.otacoo.com