Re: Support for REINDEX CONCURRENTLY

Hi,

On 2013-02-07 16:45:57 +0900, Michael Paquier wrote:
> Please find attached a patch fixing 3 of the 4 problems reported before
> (the patch does not contain docs).
> 1) Removal of the quadratic dependency with list_append_unique_oid

Afaics you now simply lock objects multiple times, is that right?

> 2) Minimization of the wait phase for parent relations, this is done in a
> single transaction before phase 2

Unfortunately I don't think this did the trick. You currently have the
following:

+    /* Perform a wait on each session lock separate transaction */
+    StartTransactionCommand();
+    foreach(lc, lockTags)
+    {
+        LOCKTAG    *localTag = (LOCKTAG *) lfirst(lc);
+        Assert(localTag && localTag->locktag_field2 != InvalidOid);
+        WaitForVirtualLocks(*localTag, ShareLock);
+    }
+    CommitTransactionCommand();

and

+void
+WaitForVirtualLocks(LOCKTAG heaplocktag, LOCKMODE lockmode)
+{
+    VirtualTransactionId *old_lockholders;
+
+    old_lockholders = GetLockConflicts(&heaplocktag, lockmode);
+
+    while (VirtualTransactionIdIsValid(*old_lockholders))
+    {
+        VirtualXactLock(*old_lockholders, true);
+        old_lockholders++;
+    }
+}

To get rid of the issue you need to batch all the GetLockConflicts calls
together before doing any of the VirtualXactLocks. Otherwise other
backends will produce new conflicts on relation n+1 while you wait for
relation n.

So it would need to be something like:

void
WaitForVirtualLocksList(List heaplocktags, LOCKMODE lockmode)
{   VirtualTransactionId **old_lockholders;   ListCell *lc;   int off = 0;   int i;
   old_lockholders = palloc(sizeof(VirtualTransactionId *) *       list_length(heaplocktags));
   /* collect transactions we need to wait on for all transactions */foreach(lc, heaplocktags)   {       LOCKTAG *tag =
lfirst(lc);      old_lockholders[off++] = GetLockConflicts(tag, lockmode);   }
 
   /* wait on all transactions */   for (i = 0; i < off; i++)   {       VirtualTransactionId *lockholders =
old_lockholders[i];
       while (VirtualTransactionIdIsValid(lockholders[i]))    {               VirtualXactLock(lockholders[i], true);
       lockholders++;    }   }
 
}

Makes sense?

Greetings,

Andres Freund

--Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services