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