On Mon, Jan 28, 2013 at 7:39 PM, Andres Freund <and...@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
